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

[PATCH v2] pathspec: only match across submodule boundaries when requested

From
BWBrandon Williams <bmwill@google.com>
Date
Dec 5, 2017, 00:07 UTC
Message-ID
<20171205000734.69530-1-bmwill@google.com>
In-Reply-To
<20171128232237.54453-1-bmwill@google.com>

Commit 74ed43711fd (grep: enable recurse-submodules to work on <tree> objects, 2016-12-16) taught 'tree_entry_interesting()' to be able to match across submodule boundaries in the presence of wildcards. This is done by performing literal matching up to the first wildcard and then punting to the submodule itself to perform more accurate pattern matching. Instead of introducing a new flag to request this behavior, commit 74ed43711fd overloaded the already existing 'recursive' flag in 'struct pathspec' to request this behavior.

This leads to a bug where whenever any other caller has the 'recursive' flag set as well as a pathspec with wildcards that all submodules will be indicated as matches. One simple example of this is:

	git init repo
	cd repo
	git init submodule
	git -C submodule commit -m initial --allow-empty
	touch "[bracket]"
	git add "[bracket]"
	git commit -m bracket
	git add submodule
	git commit -m submodule
	git rev-list HEAD -- "[bracket]"

Fix this by introducing the new flag 'recurse_submodules' in 'struct pathspec' and using this flag to determine if matches should be allowed to cross submodule boundaries.

This fixes https://github.com/git-for-windows/git/issues/1371.
Signed-off-by: Brandon Williams <bmwill@google.com>
---
 builtin/grep.c                |  1 +
 pathspec.h                    |  1 +
 t/t4208-log-magic-pathspec.sh | 19 +++++++++++++++++++
 tree-walk.c                   |  5 +++--
 4 files changed, 24 insertions(+), 2 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 5a6cfe6b4..3ca4ac80d 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -1015,6 +1015,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		       prefix, argv + i);
 	pathspec.max_depth = opt.max_depth;
 	pathspec.recursive = 1;
+	pathspec.recurse_submodules = !!recurse_submodules;
 
 #ifndef NO_PTHREADS
 	if (list.nr || cached || show_in_pager)
diff --git a/pathspec.h b/pathspec.h
index 6420d1080..099a170c2 100644
--- a/pathspec.h
+++ b/pathspec.h
@@ -24,6 +24,7 @@ struct pathspec {
 	int nr;
 	unsigned int has_wildcard:1;
 	unsigned int recursive:1;
+	unsigned int recurse_submodules:1;
 	unsigned magic;
 	int max_depth;
 	struct pathspec_item {
diff --git a/t/t4208-log-magic-pathspec.sh b/t/t4208-log-magic-pathspec.sh
index 935df6a65..a1705f70c 100755
--- a/t/t4208-log-magic-pathspec.sh
+++ b/t/t4208-log-magic-pathspec.sh
@@ -93,4 +93,23 @@ test_expect_success 'command line pathspec parsing for "git log"' '
 	git log --merge -- a
 '
 
+test_expect_success 'tree_entry_interesting does not match past submodule boundaries' '
+	test_when_finished "rm -rf repo submodule" &&
+	git init submodule &&
+	test_commit -C submodule initial &&
+	git init repo &&
+	>"repo/[bracket]" &&
+	git -C repo add "[bracket]" &&
+	test_tick &&
+	git -C repo commit -m bracket &&
+	git -C repo rev-list HEAD -- "[bracket]" >expect &&
+
+	git -C repo submodule add ../submodule &&
+	test_tick &&
+	git -C repo commit -m submodule &&
+
+	git -C repo rev-list HEAD -- "[bracket]" >actual &&
+	test_cmp expect actual
+'
+
 test_done
diff --git a/tree-walk.c b/tree-walk.c
index 684f0e337..63a87ed66 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -1011,7 +1011,8 @@ static enum interesting do_match(const struct name_entry *entry,
 				 * character.  More accurate matching can then
 				 * be performed in the submodule itself.
 				 */
-				if (ps->recursive && S_ISGITLINK(entry->mode) &&
+				if (ps->recurse_submodules &&
+				    S_ISGITLINK(entry->mode) &&
 				    !ps_strncmp(item, match + baselen,
 						entry->path,
 						item->nowildcard_len - baselen))
@@ -1060,7 +1061,7 @@ static enum interesting do_match(const struct name_entry *entry,
 		 * character.  More accurate matching can then
 		 * be performed in the submodule itself.
 		 */
-		if (ps->recursive && S_ISGITLINK(entry->mode) &&
+		if (ps->recurse_submodules && S_ISGITLINK(entry->mode) &&
 		    !ps_strncmp(item, match, base->buf + base_offset,
 				item->nowildcard_len)) {
 			strbuf_setlen(base, base_offset + baselen);
-- 
2.15.1.424.g9478a66081-goog
Previous: Brandon WilliamsNext: Junio C Hamano
Message 6 of 8 in “Bug in pathspec handling (in conjunction with submodules)”
  1. Johannes SchindelinNov 26, 2017
  2. Brandon WilliamsNov 28, 2017
  3. pathspec: only match across submodule boundaries when requestedBrandon Williams, Nov 28, 2017
  4. Johannes SchindelinNov 29, 2017
  5. Brandon WilliamsDec 5, 2017
  6. pathspec: only match across submodule boundaries when requestedBrandon Williams, Dec 5, 2017
  7. Junio C HamanoDec 5, 2017
  8. Johannes SchindelinDec 6, 2017

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.