Re: [PATCH] Allow git mv FileA fILEa when core.ignorecase = true
- From
Erik Faye-Lund <kusmabite@gmail.com>
- Date
- Mar 16, 2011, 13:05 UTC
- Message-ID
- <AANLkTin2s_=TVaKx6yv+nDvXB41BxebWHX4iaDYSGd-e@mail.gmail.com>
- In-Reply-To
- <201103042240.38969.tboegi@web.de>
2011/3/4 Torsten Bögershausen <tboegi@web.de>:
Show 44 quoted lines
> 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