Re: [PATCH v4] clone: report duplicate entries on case-insensitive filesystems
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 16, 2018, 16:23 UTC
- Message-ID
- <xmqqpnyiffgo.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20180816140312.GA6102@tor.lan>
Torsten Bögershausen <tboegi@web.de> writes:
Show 6 quoted lines
> check_stat is 0 on Windows, and inum is allways 0 in lstat(). > I was thinking about systems which don't have inodes and inum, > and then generate an inum in memory, sometimes random. > After a reboot or a re-mount of the file systems those ino values > change. > However, for the initial clone we are fine in any case.
Yup.
Show 25 quoted lines
> Now back to the compiler switch:
> Windows always set inum to 0 and I can't think about a situation where
> a file in a working tree gets inum = 0, can we use the following:
>
> static void mark_colliding_entries(const struct checkout *state,
> struct cache_entry *ce, struct stat *st)
> {
> int i;
> ce->ce_flags |= CE_MATCHED;
>
> for (i = 0; i < state->istate->cache_nr; i++) {
> struct cache_entry *dup = state->istate->cache[i];
> int folded = 0;
>
> if (dup == ce)
> break;
>
> if (dup->ce_flags & (CE_MATCHED | CE_VALID | CE_SKIP_WORKTREE))
> continue;
> /*
> * Windows sets ino to 0. On other FS ino = 0 will already be
> * used, so we don't see it for a file in a Git working tree
> */
> if (st->st_ino && (dup->ce_stat_data.sd_ino == st->st_ino))
> folded = 1;Hmm, that is tempting but feels slightly too magical to my taste. Others may easily be able to persuade me to change my mind in this case, though.
Show 31 quoted lines
> /*
> * Fallback for NTFS and other case insenstive FS,
> * which don't use POSIX inums
> */
> if (!fspathcmp(dup->name, ce->name))
> folded = 1;
>
> if (folded) {
> dup->ce_flags |= CE_MATCHED;
> break;
> }
> }
> }
>
>
>>
>> Another thing we maybe want to see is if we can update the caller of
>> this function so that we do not overwrite the earlier checkout with
>> the data for this path. When two paths collide, we check out one of
>> the paths without reporting (because we cannot notice), then attempt
>> to check out the other path and report (because we do notice the
>> previous one with lstat()). The current code then goes on and overwrites
>> the file with the contents from the "other" path.
>>
>> Even if we had false negative in this loop, if we leave the contents
>> for the earlier path while reporting the "other" path, then the user
>> can get curious, inspect what contents the "other" path has on the
>> filesystem, and can notice that it belongs to the (unreported--due
>> to false negative) earlier path.
>>
> [snip]