Re: [PATCH v4] clone: report duplicate entries on case-insensitive filesystems

[Junio C Hamano <gitster@pobox.com>]

Torsten Bögershausen <tboegi@web.de> writes:

> 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.

> 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.

> 		/*
> 		 * 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]