Skip to content

Commit e66ceca

Browse files
pcloudsgitster
authored andcommitted
clone: fix colliding file detection on APFS
Commit b878579 (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 <[email protected]> Helped-by: Ramsay Jones <[email protected]> Signed-off-by: Nguyễn Thái Ngọc Duy <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent b878579 commit e66ceca

File tree

2 files changed

+3
-3
lines changed

2 files changed

+3
-3
lines changed

entry.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ static void mark_colliding_entries(const struct checkout *state,
404404
{
405405
int i, trust_ino = check_stat;
406406

407-
#if defined(GIT_WINDOWS_NATIVE)
407+
#if defined(GIT_WINDOWS_NATIVE) || defined(__CYGWIN__)
408408
trust_ino = 0;
409409
#endif
410410

@@ -419,7 +419,7 @@ static void mark_colliding_entries(const struct checkout *state,
419419
if (dup->ce_flags & (CE_MATCHED | CE_VALID | CE_SKIP_WORKTREE))
420420
continue;
421421

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)) ||
423423
(!trust_ino && !fspathcmp(ce->name, dup->name))) {
424424
dup->ce_flags |= CE_MATCHED;
425425
break;

t/t5601-clone.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -628,7 +628,7 @@ test_expect_success 'clone on case-insensitive fs' '
628628
)
629629
'
630630

631-
test_expect_success !MINGW,!CYGWIN,CASE_INSENSITIVE_FS 'colliding file detection' '
631+
test_expect_success !MINGW,CASE_INSENSITIVE_FS 'colliding file detection' '
632632
grep X icasefs/warning &&
633633
grep x icasefs/warning &&
634634
test_i18ngrep "the following paths have collided" icasefs/warning

0 commit comments

Comments
 (0)