[PATCH] Allow git mv FileA fILEa when core.ignorecase = true

[PATCH] Allow git mv FileA fILEa when core.ignorecase = true

[Torsten Bögershausen]

    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) */
+			if (((st_dst.st_ino) && (st_dst.st_ino == st.st_ino)) ||
+					((ignore_case) && !strcasecmp(src, dst) &&
+					 (st.st_size == st_dst.st_size))) {
+				allow_force = 1;
+				bad = NULL;
+			}
+			if (allow_force) {
 				/*
 				 * only files can overwrite each other:
 				 * check both source and destination
 				 */
-				if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode)) {
-					warning("%s; will overwrite!", bad);
+				if (S_ISREG(st_dst.st_mode) || S_ISLNK(st_dst.st_mode)) {
+					if (bad)
+						warning("%s; will overwrite!", bad);
 					bad = NULL;
 				} else
 					bad = "Cannot overwrite";
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
index a845b15..d0e73ee 100755
--- a/t/t7001-mv.sh
+++ b/t/t7001-mv.sh
@@ -255,4 +255,33 @@ test_expect_success SYMLINKS 'git mv should overwrite file with a symlink' '
 
 rm -f moved symlink
 
+touch x
+if ln x y 2>/dev/null; then
+	hardlinks=1
+fi
+rm -f x y
+
+if test "$(git config --bool core.ignorecase)" = true -o "$hardlinks"; then
+	test_expect_success 'git mv FileA fILEa' '
+
+		rm -fr .git * &&
+		git init &&
+		echo FileA > FileA &&
+		git add FileA &&
+		git commit -m add FileA &&
+		{
+			if ! test -f fILEa; then
+				ln FileA fILEa
+			fi
+		} &&
+		git mv FileA fILEa &&
+		git commit -m "mv FileA fILEa" &&
+		rm -f FileA fILEa &&
+		git reset --hard &&
+		test "$(echo *)" = fILEa
+	'
+else
+	say "Neither ignorecase nor hardlinks, skipping git mv FileA fILEa"
+fi
+
 test_done
-- 
1.7.4

Re: [PATCH] Allow git mv FileA fILEa when core.ignorecase = true

[Erik Faye-Lund]

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;
+
+	return hia.dwVolumeSerialNumber == hib.dwVolumeSerialNumber &&
+	    hia.nFileSizeLow == hib.nFileSizeLow &&
+	    hia.nFileSizeHigh == hib.nFileSizeHigh;
+}
+
 #undef mkdir
 int mingw_mkdir(const char *path, int mode)
 {
diff --git a/compat/mingw.h b/compat/mingw.h
index 9c00e75..43c0b5f 100644
--- a/compat/mingw.h
+++ b/compat/mingw.h
@@ -297,6 +297,9 @@ void mingw_open_html(const char *path);
 void mingw_mark_as_git_dir(const char *dir);
 #define mark_as_git_dir mingw_mark_as_git_dir

+int mingw_is_same_file(const char *a, const char *b);
+#define is_same_file mingw_is_same_file
+
 /*
  * helpers
  */
diff --git a/git-compat-util.h b/git-compat-util.h
index 2ca2fad..b95b9e2 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -558,6 +558,17 @@ int remove_or_warn(unsigned int mode, const char *path);
 #define mark_as_git_dir(x) /* noop */
 #endif

+#ifndef is_same_file
+static inline int mingw_is_same_file(const char *a, const char *b)
+{
+	struct stat sta, stb;
+	if (lstat(a, &sta) ||
+	    lstat(b, &stb))
+		return 0;
+	return sta.st_dev == stb.st_deb && sta.st_ino == stb.st_ino;
+}
+#endif
+
 #ifndef get_home_directory
 #define get_home_directory() getenv("HOME")
 #endif

Re: [PATCH] Allow git mv FileA fILEa when core.ignorecase = true

[Erik Faye-Lund]

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);

Re: [PATCH] Allow git mv FileA fILEa when core.ignorecase = true

[Torsten Bögershausen]

On 16.03.11 14:18, Erik Faye-Lund wrote:
> 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);
Good point. I will send a new patch, including your suggestion,
but 50% different ;-)
/Torsten