* [PATCH] Allow git mv FileA fILEa when core.ignorecase = true
@ 2011-03-04 21:40 Torsten Bögershausen
2011-03-16 13:05 ` Erik Faye-Lund
0 siblings, 1 reply; 4+ messages in thread
From: Torsten Bögershausen @ 2011-03-04 21:40 UTC (permalink / raw)
To: git; +Cc: tboegi
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Allow git mv FileA fILEa when core.ignorecase = true
2011-03-04 21:40 [PATCH] Allow git mv FileA fILEa when core.ignorecase = true Torsten Bögershausen
@ 2011-03-16 13:05 ` Erik Faye-Lund
2011-03-16 13:18 ` Erik Faye-Lund
0 siblings, 1 reply; 4+ messages in thread
From: Erik Faye-Lund @ 2011-03-16 13:05 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Allow git mv FileA fILEa when core.ignorecase = true
2011-03-16 13:05 ` Erik Faye-Lund
@ 2011-03-16 13:18 ` Erik Faye-Lund
2011-03-19 14:28 ` Torsten Bögershausen
0 siblings, 1 reply; 4+ messages in thread
From: Erik Faye-Lund @ 2011-03-16 13:18 UTC (permalink / raw)
To: Torsten Bögershausen; +Cc: git
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);
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Allow git mv FileA fILEa when core.ignorecase = true
2011-03-16 13:18 ` Erik Faye-Lund
@ 2011-03-19 14:28 ` Torsten Bögershausen
0 siblings, 0 replies; 4+ messages in thread
From: Torsten Bögershausen @ 2011-03-19 14:28 UTC (permalink / raw)
To: kusmabite, git; +Cc: 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2011-03-19 14:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-04 21:40 [PATCH] Allow git mv FileA fILEa when core.ignorecase = true Torsten Bögershausen
2011-03-16 13:05 ` Erik Faye-Lund
2011-03-16 13:18 ` Erik Faye-Lund
2011-03-19 14:28 ` Torsten Bögershausen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).