git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Duy Nguyen <pclouds@gmail.com>
Date
Aug 17, 2018, 18:00 UTC
Message-ID
<20180817180039.GA31789@duynguyen.home>
In-Reply-To
<xmqqh8jsc3kr.fsf@gitster-ct.c.googlers.com>
On Fri, Aug 17, 2018 at 10:20:36AM -0700, Junio C Hamano wrote:
Show 5 quoted lines
> I highly suspect that the above was written in that way to reduce
> the indentation level, but the right way to reduce the indentation
> level, if it bothers readers too much, is to make the whole thing
> inside the above if (o->clone) into a dedicated helper function
> "void report_collided_checkout(void)", I would think.

I read my mind. I thought of separating into a helper function too, but was not happy that the clearing CE_MATCHED in preparation for this test is in check_updates(), but the cleaning up CE_MATCHED() is in the helper function.

So here is the version that separates _both_ phases into helper functions.

-- 8< --
Subject: [PATCH v6] clone: report duplicate entries on case-insensitive filesystems

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 exactly is "dirty".

This patch helps the situation a bit by pointing out the problem at clone time. Even though this patch talks about case sensitivity, the patch makes no assumption about folding rules by the filesystem. It simply observes that if an entry has been already checked out at clone time when we're about to write a new path, some folding rules are behind this.

In the case that we can't rely on filesystem (via inode number) to do this check, fall back to fspathcmp() which is not perfect but should not give false positives.

This patch is tested with vim-colorschemes and Sublime-Gitignore repositories on a JFS partition with case insensitive support on Linux.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 builtin/clone.c  |  1 +
 cache.h          |  1 +
 entry.c          | 31 +++++++++++++++++++++++++++++++
 t/t5601-clone.sh |  8 +++++++-
 unpack-trees.c   | 47 +++++++++++++++++++++++++++++++++++++++++++++++
 unpack-trees.h   |  1 +
 6 files changed, 88 insertions(+), 1 deletion(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 5c439f1394..0702b0e9d0 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -747,6 +747,7 @@ static int checkout(int submodule_progress)
 	memset(&opts, 0, sizeof opts);
 	opts.update = 1;
 	opts.merge = 1;
+	opts.clone = 1;
 	opts.fn = oneway_merge;
 	opts.verbose_update = (option_verbosity >= 0);
 	opts.src_index = &the_index;
diff --git a/cache.h b/cache.h
index 8b447652a7..6d6138f4f1 100644
--- a/cache.h
+++ b/cache.h
@@ -1455,6 +1455,7 @@ struct checkout {
 	unsigned force:1,
 		 quiet:1,
 		 not_new:1,
+		 clone:1,
 		 refresh_cache:1;
 };
 #define CHECKOUT_INIT { NULL, "" }
diff --git a/entry.c b/entry.c
index b5d1d3cf23..8766e27255 100644
--- a/entry.c
+++ b/entry.c
@@ -399,6 +399,34 @@ static int check_path(const char *path, int len, struct stat *st, int skiplen)
 	return lstat(path, st);
 }
 
+static void mark_colliding_entries(const struct checkout *state,
+				   struct cache_entry *ce, struct stat *st)
+{
+	int i, trust_ino = check_stat;
+
+#if defined(GIT_WINDOWS_NATIVE)
+	trust_ino = 0;
+#endif
+
+	ce->ce_flags |= CE_MATCHED;
+
+	for (i = 0; i < state->istate->cache_nr; i++) {
+		struct cache_entry *dup = state->istate->cache[i];
+
+		if (dup == ce)
+			break;
+
+		if (dup->ce_flags & (CE_MATCHED | CE_VALID | CE_SKIP_WORKTREE))
+			continue;
+
+		if ((trust_ino && dup->ce_stat_data.sd_ino == st->st_ino) ||
+		    (!trust_ino && !fspathcmp(ce->name, dup->name))) {
+			dup->ce_flags |= CE_MATCHED;
+			break;
+		}
+	}
+}
+
 /*
  * Write the contents from ce out to the working tree.
  *
@@ -455,6 +483,9 @@ int checkout_entry(struct cache_entry *ce,
 			return -1;
 		}
 
+		if (state->clone)
+			mark_colliding_entries(state, ce, &st);
+
 		/*
 		 * We unlink the old file, to get the new one with the
 		 * right permissions (including umask, which is nasty
diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index 0b62037744..f2eb73bc74 100755
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -624,10 +624,16 @@ test_expect_success 'clone on case-insensitive fs' '
 			git hash-object -w -t tree --stdin) &&
 		c=$(git commit-tree -m bogus $t) &&
 		git update-ref refs/heads/bogus $c &&
-		git clone -b bogus . bogus
+		git clone -b bogus . bogus 2>warning
 	)
 '
 
+test_expect_success !MINGW,!CYGWIN,CASE_INSENSITIVE_FS 'colliding file detection' '
+	grep X icasefs/warning &&
+	grep x icasefs/warning &&
+	test_i18ngrep "the following paths have collided" icasefs/warning
+'
+
 partial_clone () {
 	       SERVER="$1" &&
 	       URL="$2" &&
diff --git a/unpack-trees.c b/unpack-trees.c
index cd0680f11e..213da8bbb4 100644
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -345,6 +345,46 @@ static struct progress *get_progress(struct unpack_trees_options *o)
 	return start_delayed_progress(_("Checking out files"), total);
 }
 
+static void setup_collided_checkout_detection(struct checkout *state,
+					      struct index_state *index)
+{
+	int i;
+
+	state->clone = 1;
+	for (i = 0; i < index->cache_nr; i++)
+		index->cache[i]->ce_flags &= ~CE_MATCHED;
+}
+
+static void report_collided_checkout(struct index_state *index)
+{
+	struct string_list list = STRING_LIST_INIT_NODUP;
+	int i;
+
+	for (i = 0; i < index->cache_nr; i++) {
+		struct cache_entry *ce = index->cache[i];
+
+		if (!(ce->ce_flags & CE_MATCHED))
+			continue;
+
+		string_list_append(&list, ce->name);
+		ce->ce_flags &= ~CE_MATCHED;
+	}
+
+	list.cmp = fspathcmp;
+	string_list_sort(&list);
+
+	if (list.nr) {
+		warning(_("the following paths have collided (e.g. case-sensitive paths\n"
+			  "on a case-insensitive filesystem) and only one from the same\n"
+			  "colliding group is in the working tree:\n"));
+
+		for (i = 0; i < list.nr; i++)
+			fprintf(stderr, "  '%s'\n", list.items[i].string);
+	}
+
+	string_list_clear(&list, 0);
+}
+
 static int check_updates(struct unpack_trees_options *o)
 {
 	unsigned cnt = 0;
@@ -359,6 +399,9 @@ static int check_updates(struct unpack_trees_options *o)
 	state.refresh_cache = 1;
 	state.istate = index;
 
+	if (o->clone)
+		setup_collided_checkout_detection(&state, index);
+
 	progress = get_progress(o);
 
 	if (o->update)
@@ -423,6 +466,10 @@ static int check_updates(struct unpack_trees_options *o)
 	errs |= finish_delayed_checkout(&state);
 	if (o->update)
 		git_attr_set_direction(GIT_ATTR_CHECKIN, NULL);
+
+	if (o->clone)
+		report_collided_checkout(index);
+
 	return errs != 0;
 }
 
diff --git a/unpack-trees.h b/unpack-trees.h
index c2b434c606..d940f1c5c2 100644
--- a/unpack-trees.h
+++ b/unpack-trees.h
@@ -42,6 +42,7 @@ struct unpack_trees_options {
 	unsigned int reset,
 		     merge,
 		     update,
+		     clone,
 		     index_only,
 		     nontrivial_merge,
 		     trivial_merges_only,

-- 
2.18.0.1004.g6639190530

-- 8< --
--
Duy
Previous: Junio C HamanoNext: Torsten Bögershausen
Message 74 of 96 in “Git clone and case sensitivity”
  1. Paweł ParuzelJul 27, 2018
  2. brian m. carlsonJul 27, 2018
  3. Duy NguyenJul 28, 2018
  4. Duy NguyenJul 28, 2018
  5. Jeff KingJul 28, 2018
  6. Duy NguyenJul 28, 2018
  7. Simon RuderichJul 28, 2018
  8. Jeff KingJul 28, 2018
  9. brian m. carlsonJul 28, 2018
  10. Duy NguyenJul 29, 2018
  11. Jeff KingJul 29, 2018
  12. clone: report duplicate entries on case-insensitive filesystemsNguyễn Thái Ngọc Duy, Jul 30, 2018
  13. Torsten BögershausenJul 31, 2018
  14. Duy NguyenAug 1, 2018
  15. Elijah NewrenJul 31, 2018
  16. Junio C HamanoJul 31, 2018
  17. Jeff KingJul 31, 2018
  18. Junio C HamanoJul 31, 2018
  19. Jeff KingJul 31, 2018
  20. Junio C HamanoJul 31, 2018
  21. Junio C HamanoAug 1, 2018
  22. Duy NguyenAug 2, 2018
  23. Junio C HamanoAug 2, 2018
  24. Jeff KingAug 2, 2018
  25. Junio C HamanoAug 2, 2018
  26. Jeff KingAug 2, 2018
  27. Jeff HostetlerAug 3, 2018
  28. Junio C HamanoAug 3, 2018
  29. Jeff KingAug 3, 2018
  30. Jeff HostetlerAug 5, 2018
  31. Torsten BögershausenAug 3, 2018
  32. Duy NguyenAug 1, 2018
  33. Junio C HamanoJul 31, 2018
  34. Duy NguyenAug 1, 2018
  35. clone: report duplicate entries on case-insensitive filesystemsNguyễn Thái Ngọc Duy, Aug 7, 2018
  36. Junio C HamanoAug 7, 2018
  37. Jeff HostetlerAug 8, 2018
  38. Jeff KingAug 8, 2018
  39. Junio C HamanoAug 9, 2018
  40. Jeff KingAug 9, 2018
  41. Jeff HostetlerAug 9, 2018
  42. Jeff KingAug 9, 2018
  43. Elijah NewrenAug 9, 2018
  44. Jeff KingAug 9, 2018
  45. Elijah NewrenAug 9, 2018
  46. Jeff KingAug 9, 2018
  47. Elijah NewrenAug 9, 2018
  48. Junio C HamanoAug 9, 2018
  49. 0/1 clone: warn on colidding entries on checkoutNguyễn Thái Ngọc Duy, Aug 10, 2018
  50. 1/1 clone: report duplicate entries on case-insensitive filesystemsNguyễn Thái Ngọc Duy, Aug 10, 2018
  51. Junio C HamanoAug 10, 2018
  52. SZEDER GáborAug 11, 2018
  53. Duy NguyenAug 11, 2018
  54. Junio C HamanoAug 13, 2018
  55. Duy NguyenAug 13, 2018
  56. Junio C HamanoAug 10, 2018
  57. clone: report duplicate entries on case-insensitive filesystemsNguyễn Thái Ngọc Duy, Aug 12, 2018
  58. Jeff HostetlerAug 13, 2018
  59. Junio C HamanoAug 13, 2018
  60. Torsten BögershausenAug 15, 2018
  61. Duy NguyenAug 15, 2018
  62. config.txt: clarify core.checkStat = minimalNguyễn Thái Ngọc Duy, Aug 16, 2018
  63. Junio C HamanoAug 16, 2018
  64. Duy NguyenAug 16, 2018
  65. Junio C HamanoAug 16, 2018
  66. Junio C HamanoAug 17, 2018
  67. Duy NguyenAug 17, 2018
  68. Junio C HamanoAug 15, 2018
  69. Torsten BögershausenAug 16, 2018
  70. Duy NguyenAug 16, 2018
  71. Junio C HamanoAug 16, 2018
  72. clone: report duplicate entries on case-insensitive filesystemsNguyễn Thái Ngọc Duy, Aug 17, 2018
  73. Junio C HamanoAug 17, 2018
  74. Duy NguyenAug 17, 2018
  75. Torsten BögershausenAug 17, 2018
  76. clone: report duplicate entries on case-insensitive filesystemsCarlo Marcelo Arenas Belón, Nov 19, 2018
  77. Torsten BögershausenNov 19, 2018
  78. Carlo ArenasNov 19, 2018
  79. Duy NguyenNov 19, 2018
  80. Duy NguyenNov 19, 2018
  81. Duy NguyenNov 19, 2018
  82. Duy NguyenNov 19, 2018
  83. Ramsay JonesNov 19, 2018
  84. Ramsay JonesNov 19, 2018
  85. Carlo ArenasNov 20, 2018
  86. Junio C HamanoNov 20, 2018
  87. clone: fix colliding file detection on APFSNguyễn Thái Ngọc Duy, Nov 20, 2018
  88. Ramsay JonesNov 20, 2018
  89. Carlo ArenasNov 20, 2018
  90. Duy NguyenNov 20, 2018
  91. 1/1 t5601-99: Enable colliding file detection for MINGWtboegi@web.de, Nov 22, 2018
  92. 1/1 t5601-99: Enable colliding file detection for MINGWCarlo Marcelo Arenas Belón, Nov 22, 2018
  93. Johannes SchindelinNov 23, 2018
  94. Ramsay JonesNov 19, 2018
  95. Carlo ArenasNov 19, 2018
  96. Jeff HostetlerJul 31, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.