clone: fix colliding file detection on APFS

Commit b878579ae7 (clone: report duplicate entries on case-insensitive filesystems - 2018-08-17) adds a warning to user when cloning a repo with case-sensitive file names on a case-insensitive file system. The "find duplicate file" check was doing by comparing inode number (and only fall back to fspathcmp() when inode is known to be unreliable because fspathcmp() can't cover all case folding cases). The inode check is very simple, and wrong. It compares between a 32-bit number (sd_ino) and potentially a 64-bit number (st_ino). When an inode is larger than 2^32 (which seems to be the case for APFS), it will be truncated and stored in sd_ino, but comparing with itself will fail. As a result, instead of showing a pair of files that have the same name, we show just one file (marked before the beginning of the loop). We fail to find the original one. The fix could be just a simple type cast (*) dup->ce_stat_data.sd_ino == (unsigned int)st->st_ino but this is no longer a reliable test, there are 4G possible inodes that can match sd_ino because we only match the lower 32 bits instead of full 64 bits. There are two options to go. Either we ignore inode and go with fspathcmp() on Apple platform. This means we can't do accurate inode check on HFS anymore, or even on APFS when inode numbers are still below 2^32. Or we just to to reduce the odds of matching a wrong file by checking more attributes, counting mostly on st_size because st_xtime is likely the same. This patch goes with this direction, hoping that false positive chances are too small to be seen in practice. While at there, enable the test on Cygwin (verified working by Ramsay Jones) (*) this is also already done inside match_stat_data() Reported-by: Carlo Arenas <carenas@gmail.com> Helped-by: Ramsay Jones <ramsay@ramsayjones.plus.com> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Nguyễn Thái Ngọc Duy committed Nov 20, 2018 at 17:28 UTC e66ceca94b6137dd73cab8b19ba8c890a0055544
2 files changed +3 -3
entry.c
+2 -2
@@ -404,7 +404,7 @@ static void mark_colliding_entries(const struct checkout *state,
404 {
405 int i, trust_ino = check_stat;
406
407 -#if defined(GIT_WINDOWS_NATIVE)
407 +#if defined(GIT_WINDOWS_NATIVE) || defined(__CYGWIN__)
408 trust_ino = 0;
409 #endif
410
@@ -419,7 +419,7 @@ static void mark_colliding_entries(const struct checkout *state,
419 if (dup->ce_flags & (CE_MATCHED | CE_VALID | CE_SKIP_WORKTREE))
420 continue;
421
422 - if ((trust_ino && dup->ce_stat_data.sd_ino == st->st_ino) ||
422 + if ((trust_ino && !match_stat_data(&dup->ce_stat_data, st)) ||
423 (!trust_ino && !fspathcmp(ce->name, dup->name))) {
424 dup->ce_flags |= CE_MATCHED;
425 break;
t/t5601-clone.sh
+1 -1
@@ -628,7 +628,7 @@ test_expect_success 'clone on case-insensitive fs' '
628 )
629 '
630
631 -test_expect_success !MINGW,!CYGWIN,CASE_INSENSITIVE_FS 'colliding file detection' '
631 +test_expect_success !MINGW,CASE_INSENSITIVE_FS 'colliding file detection' '
632 grep X icasefs/warning &&
633 grep x icasefs/warning &&
634 test_i18ngrep "the following paths have collided" icasefs/warning