[PATCH] rename: detect and prevent false file deletion on case mangling systems (issue910)

[Adrian Buehlmann]

On 18.04.2008 18:02, Alexis S. L. Carvalho wrote:
> Thus spake Adrian Buehlmann:
>> rename: detect and prevent false file deletion on case mangling systems (issue910)
>>
>> Reject "rename --after a.txt A.txt" on case mangling systems (e.g. Windows).
>> The current implementation of hg rename is unable to handle this case.
>>
>> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
>> --- a/mercurial/cmdutil.py
>> +++ b/mercurial/cmdutil.py
>> @@ -336,6 +336,15 @@
>>          target = repo.wjoin(abstarget)
>>          src = repo.wjoin(abssrc)
>>          state = repo.dirstate[abstarget]
>> +
>> +        if rename and after:
>> +            # reject "rename --after a.txt A.txt" on case mangling systems (issue910)
>> +            nsrc = os.path.normcase(src)
>> +            ntarget = os.path.normcase(target)
>> +            if ntarget == nsrc:
>> +                ui.warn(_('cannot record rename of %s to %s\n') %
>> +                       (repo.pathto(abssrc, cwd), repo.pathto(abstarget, cwd)))
>> +                return
>>
>>          # check for collisions
>>          prevsrc = targets.get(abstarget)
> 
> This wouldn't work on e.g. linux using vfat or other case insensitive
> FS.

Hmm. I haven't tested that (linux using vfat). It's also the first time
in my life I'm writing a call to os.path.normcase(), so I can't comment on
that in detail. But I must assume you are right in so far, as
the rejecting code would not reject in that case -- exactly as
it does now. So the patch would not worsen anything in that regard
(besides adding 8 lines of code).

The detection of this corner case works on Windows (tested myself).
Chances that this detection works on Mac OS X seem to be good as well
(testers welcome).

But I agree that this is not the perfect solution. But quite a
significant improvement over the current situation, without touching
anything of the current program logic.

> What about just not removing the source file for rename --after?
> Something like:
> 
> diff -r 550c53d66949 mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -395,7 +395,7 @@ def copy(ui, repo, pats, opts, rename=Fa
>                  repo.copy(origsrc, abstarget)
>  
>          if rename and not dryrun:
> -            repo.remove([abssrc], True)
> +            repo.remove([abssrc], not after)
>  
>      # pat: ossep
>      # dest ossep
> 
> 

Thanks for that proposal. I will try running the testsuite with that
change and analyze it.