From: Erik Faye-Lund <kusmabite@gmail.com>
Date: Wed, 16 Mar 2011 13:18:28 GMT
Subject: Re: [PATCH] Allow git mv FileA fILEa when core.ignorecase = true
Message-ID: <AANLkTikvkzUKBCqygGJoUG3qXNDArXpEX0RM5_5dbnBD@mail.gmail.com>
In-Reply-To: <AANLkTin2s_=TVaKx6yv+nDvXB41BxebWHX4iaDYSGd-e@mail.gmail.com>

2011/3/16 Erik Faye-Lund <kusmabite@gmail.com>:
> 2011/3/4 Torsten Bögershausen <tboegi@web.de>:
>>    The typical use case is when a file "FileA" should be renamed into fILEa
>>    and we are on a case insenstive file system (system core.ignorecase = true).
>>    Source and destination are the same file, it can be accessed under both names.
>>    This makes git think that the destination file exists.
>>    Unless used with --forced, git will refuse the "git mv FileA fILEa".
>>    This change will allow "git mv FileA fILEa", when core.ignorecase = true
>>    and source and destination filenames only differ in case and the file length
>>    is identical.
>>    On Linux/Unix/Mac OS X the mv is allowed when the inode of the source and
>>    destination are equal.
>>    On  this allows renames of MÄRCHEN into Märchen on Mac OS X.
>>    (As a side effect, a file can be renamed to a name which is already
>>    hard-linked to the same inode)
>>
>> Signed-off-by: Torsten Bögershausen <tboegi@web.de>
>> ---
>>  builtin/mv.c  |   20 +++++++++++++++-----
>>  t/t7001-mv.sh |   29 +++++++++++++++++++++++++++++
>>  2 files changed, 44 insertions(+), 5 deletions(-)
>>
>> diff --git a/builtin/mv.c b/builtin/mv.c
>> index 93e8995..e0aad62 100644
>> --- a/builtin/mv.c
>> +++ b/builtin/mv.c
>> @@ -62,7 +62,7 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
>>        };
>>        const char **source, **destination, **dest_path;
>>        enum update_mode { BOTH = 0, WORKING_DIRECTORY, INDEX } *modes;
>> -       struct stat st;
>> +       struct stat st, st_dst;
>>        struct string_list src_for_dst = STRING_LIST_INIT_NODUP;
>>
>>        git_config(git_default_config, NULL);
>> @@ -164,15 +164,25 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
>>                        }
>>                } else if (cache_name_pos(src, length) < 0)
>>                        bad = "not under version control";
>> -               else if (lstat(dst, &st) == 0) {
>> +               else if (lstat(dst, &st_dst) == 0) {
>> +                       int allow_force = force;
>>                        bad = "destination exists";
>> -                       if (force) {
>> +                       /* Allow when src and dst have the same inode (Mac OS X) */
>> +                       /* Allow when ignore_case and same file length (Windows) */
>
> Wait, what? Same file length is sufficient to trigger overwriting
> without -f? I find this to be a very dubious heuristic...
>
> Shouldn't you be checking something like nFileIndexLow/High from
> BY_HANDLE_FILE_INFORMATION instead? (ref:
> http://msdn.microsoft.com/en-us/library/aa363788(v=VS.85).aspx)
>
> Sure, we'd need some API to check that, but if we assume that this
> code path is rare-ish we could do something like this (note,
> untested):
>
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 6750e67..fee4113 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -299,6 +299,21 @@ void mingw_mark_as_git_dir(const char *dir)
>                 "dotGitOnly" : "true"));
>  }
>
> +int is_same_file(const char *a, const char *b)
> +{
> +       BY_HANDLE_FILE_INFORMATION hia, hib;
> +       HANDLE ha = OpenFileA(a, NULL, OF_READ),
> +              hb = OpenFileA(b, NULL, OF_READ);
> +       if (!ha || !hb ||
> +           !GetFileInformationByHandle(ha) ||
> +           !GetFileInformationByHandle(hb))
> +               return 0;
> +

And if couse:
CloseHandle(ha);
CloseHandle(hb);