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

["Torsten Bögershausen" <tboegi@web.de>]

On Mon, Jul 30, 2018 at 05:27:55PM +0200, Nguyễn Thái Ngọc Duy wrote:
> Paths that only differ in case work fine in a case-sensitive
> filesystems, but if those repos are cloned in a case-insensitive one,
> you'll get problems. The first thing to notice is "git status" will
> never be clean with no indication what's exactly is "dirty".
> 
> This patch helps the situation a bit by pointing out the problem at
> clone time. I have not suggested any way to work around or fix this
> problem. But I guess we could probably have a section in
> Documentation/ dedicated to this problem and point there instead of
> a long advice in this warning.
> 
> Another thing we probably should do is catch in "git checkout" too,
> not just "git clone" since your linux/unix colleage colleague may
> accidentally add some files that your mac/windows machine is not very
> happy with. But then there's another problem, once the problem is
> known, we probably should stop spamming this warning at every
> checkout, but how?
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/clone.c | 41 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/builtin/clone.c b/builtin/clone.c
> index 5c439f1394..32738c2737 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -711,6 +711,33 @@ static void update_head(const struct ref *our, const struct ref *remote,
>  	}
>  }
>  
> +static void find_duplicate_icase_entries(struct index_state *istate,
> +					 struct string_list *dup)
> +{
> +	struct string_list list = STRING_LIST_INIT_NODUP;
> +	int i;
> +
> +	for (i = 0; i < istate->cache_nr; i++)
> +		string_list_append(&list, istate->cache[i]->name);
> +
> +	list.cmp = fspathcmp;
> +	string_list_sort(&list);
> +
> +	for (i = 1; i < list.nr; i++) {
> +		const char *cur = list.items[i].string;
> +		const char *prev = list.items[i - 1].string;
> +
> +		if (dup->nr &&
> +		    !fspathcmp(cur, dup->items[dup->nr - 1].string)) {
> +			string_list_append(dup, cur);
> +		} else if (!fspathcmp(cur, prev)) {
> +			string_list_append(dup, prev);
> +			string_list_append(dup, cur);
> +		}
> +	}
> +	string_list_clear(&list, 0);
> +}
> +
>  static int checkout(int submodule_progress)
>  {
>  	struct object_id oid;
> @@ -761,6 +788,20 @@ static int checkout(int submodule_progress)
>  	if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))
>  		die(_("unable to write new index file"));
>  
> +	if (ignore_case) {
> +		struct string_list dup = STRING_LIST_INIT_DUP;
> +		int i;
> +
> +		find_duplicate_icase_entries(&the_index, &dup);
> +		if (dup.nr) {
> +			warning(_("the following paths in this repository only differ in case and will\n"
> +				  "cause problems because you have cloned it on an case-insensitive filesytem:\n"));

Thanks for the patch.
I wonder if we can tell the users more about the "problems"
and how to avoid them, or to live with them.

This is more loud thinking:

"The following paths only differ in case\n"
"One a case-insensitive file system only one at a time can be present\n"
"You may rename one like this:\n"
"git checkout <file> && git mv <file> <file>.1\n"

> +				fprintf(stderr, "\t%s\n", dup.items[i].string);

Another question:
Do we need any quote_path() here ?
(This may be overkill, since typically the repos with conflicting names
only use ASCII.)

> +		}
> +		string_list_clear(&dup, 0);
> +	}
> +
>  	err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
>  			   oid_to_hex(&oid), "1", NULL);
>  
> -- 
> 2.18.0.656.gda699b98b3
>