Thread (1 message) 1 message, 1 author, 2018-08-16

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

From: Junio C Hamano <hidden>
Date: 2018-08-16 16:23:26

Torsten Bögershausen [off-list ref] 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;
		}
	}
}

quoted
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]
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help