[PATCH] rename: detect and prevent false file deletion on case mangling systems (issue910)
Adrian Buehlmann
adrian at cadifra.com
Fri Apr 18 11:53:43 CDT 2008
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.
More information about the Mercurial-devel
mailing list