Re: [PATCH v2] pathspec: only match across submodule boundaries when requested
From: Junio C Hamano <hidden>
Date: 2017-12-05 19:19:24
Brandon Williams [off-list ref] writes:
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.
Makes sense.
I initially misread the title
"pathspec: only match across submodule boundaries when requested"
to be saying that the match happens only at the boundary, but that
"only" is not about where the match happens.
"pathspec: match across submodule boundaries only when requested"
would have avoided such a confusion.
quoted hunk
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;
With the current code, recurse_submodules can only be 0 or 1 (the only assignment is from the return value of git_config_bool()), so the force-boolean !! is not strictly needed, but it may be a good future-proofing measure. Will queue; thanks.