ls-files: add pathspec matching for submodules

Pathspecs can be a bit tricky when trying to apply them to submodules. The main challenge is that the pathspecs will be with respect to the superproject and not with respect to paths in the submodule. The approach this patch takes is to pass in the identical pathspec from the superproject to the submodule in addition to the submodule-prefix, which is the path from the root of the superproject to the submodule, and then we can compare an entry in the submodule prepended with the submodule-prefix to the pathspec in order to determine if there is a match. This patch also permits the pathspec logic to perform a prefix match against submodules since a pathspec could refer to a file inside of a submodule. Due to limitations in the wildmatch logic, a prefix match is only done literally. If any wildcard character is encountered we'll simply punt and produce a false positive match. More accurate matching will be done once inside the submodule. This is due to the superproject not knowing what files could exist in the submodule. Signed-off-by: Brandon Williams <bmwill@google.com> Reviewed-by: Stefan Beller <sbeller@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Brandon Williams committed Oct 7, 2016 at 11:18 UTC 75a6315f7416a2fd559d0b0c7352b4f1cd10e186
5 files changed +175 -13
Documentation/git-ls-files.txt
+1 -2
@@ -140,8 +140,7 @@ a space) at the start of each line:
140
141 --recurse-submodules::
142 Recursively calls ls-files on each submodule in the repository.
143 - Currently there is only support for the --cached mode without a
144 - pathspec.
143 + Currently there is only support for the --cached mode.
144
145 --abbrev[=<n>]::
146 Instead of showing the full 40-byte hexadecimal object
builtin/ls-files.c
+21 -6
@@ -195,6 +195,7 @@ static void show_gitlink(const struct cache_entry *ce)
195 {
196 struct child_process cp = CHILD_PROCESS_INIT;
197 int status;
198 + int i;
199
200 argv_array_pushf(&cp.args, "--super-prefix=%s%s/",
201 super_prefix ? super_prefix : "",
@@ -205,6 +206,15 @@ static void show_gitlink(const struct cache_entry *ce)
206 /* add supported options */
207 argv_array_pushv(&cp.args, submodules_options.argv);
208
209 + /*
210 + * Pass in the original pathspec args. The submodule will be
211 + * responsible for prepending the 'submodule_prefix' prior to comparing
212 + * against the pathspec for matches.
213 + */
214 + argv_array_push(&cp.args, "--");
215 + for (i = 0; i < pathspec.nr; i++)
216 + argv_array_push(&cp.args, pathspec.items[i].original);
217 +
218 cp.git_cmd = 1;
219 cp.dir = ce->name;
220 status = run_command(&cp);
@@ -223,7 +233,8 @@ static void show_ce_entry(const char *tag, const struct cache_entry *ce)
233 if (len >= ce_namelen(ce))
234 die("git ls-files: internal error - cache entry not superset of prefix");
235
226 - if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) {
236 + if (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&
237 + submodule_path_match(&pathspec, name.buf, ps_matched)) {
238 show_gitlink(ce);
239 } else if (match_pathspec(&pathspec, name.buf, name.len,
240 len, ps_matched,
@@ -602,16 +613,20 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
613 die("ls-files --recurse-submodules does not support "
614 "--error-unmatch");
615
605 - if (recurse_submodules && argc)
606 - die("ls-files --recurse-submodules does not support pathspec");
607 -
616 parse_pathspec(&pathspec, 0,
617 PATHSPEC_PREFER_CWD |
618 PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
619 prefix, argv);
620
613 - /* Find common prefix for all pathspec's */
614 - max_prefix = common_prefix(&pathspec);
621 + /*
622 + * Find common prefix for all pathspec's
623 + * This is used as a performance optimization which unfortunately cannot
624 + * be done when recursing into submodules
625 + */
626 + if (recurse_submodules)
627 + max_prefix = NULL;
628 + else
629 + max_prefix = common_prefix(&pathspec);
630 max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
631
632 /* Treat unmatching pathspec elements as errors */
dir.c
+44 -2
@@ -207,8 +207,9 @@ int within_depth(const char *name, int namelen,
207 return 1;
208 }
209
210 -#define DO_MATCH_EXCLUDE 1
211 -#define DO_MATCH_DIRECTORY 2
210 +#define DO_MATCH_EXCLUDE (1<<0)
211 +#define DO_MATCH_DIRECTORY (1<<1)
212 +#define DO_MATCH_SUBMODULE (1<<2)
213
214 /*
215 * Does 'match' match the given name?
@@ -283,6 +284,32 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
284 item->nowildcard_len - prefix))
285 return MATCHED_FNMATCH;
286
287 + /* Perform checks to see if "name" is a super set of the pathspec */
288 + if (flags & DO_MATCH_SUBMODULE) {
289 + /* name is a literal prefix of the pathspec */
290 + if ((namelen < matchlen) &&
291 + (match[namelen] == '/') &&
292 + !ps_strncmp(item, match, name, namelen))
293 + return MATCHED_RECURSIVELY;
294 +
295 + /* name" doesn't match up to the first wild character */
296 + if (item->nowildcard_len < item->len &&
297 + ps_strncmp(item, match, name,
298 + item->nowildcard_len - prefix))
299 + return 0;
300 +
301 + /*
302 + * Here is where we would perform a wildmatch to check if
303 + * "name" can be matched as a directory (or a prefix) against
304 + * the pathspec. Since wildmatch doesn't have this capability
305 + * at the present we have to punt and say that it is a match,
306 + * potentially returning a false positive
307 + * The submodules themselves will be able to perform more
308 + * accurate matching to determine if the pathspec matches.
309 + */
310 + return MATCHED_RECURSIVELY;
311 + }
312 +
313 return 0;
314 }
315
@@ -386,6 +413,21 @@ int match_pathspec(const struct pathspec *ps,
413 return negative ? 0 : positive;
414 }
415
416 +/**
417 + * Check if a submodule is a superset of the pathspec
418 + */
419 +int submodule_path_match(const struct pathspec *ps,
420 + const char *submodule_name,
421 + char *seen)
422 +{
423 + int matched = do_match_pathspec(ps, submodule_name,
424 + strlen(submodule_name),
425 + 0, seen,
426 + DO_MATCH_DIRECTORY |
427 + DO_MATCH_SUBMODULE);
428 + return matched;
429 +}
430 +
431 int report_path_error(const char *ps_matched,
432 const struct pathspec *pathspec,
433 const char *prefix)
dir.h
+4
@@ -304,6 +304,10 @@ extern int git_fnmatch(const struct pathspec_item *item,
304 const char *pattern, const char *string,
305 int prefix);
306
307 +extern int submodule_path_match(const struct pathspec *ps,
308 + const char *submodule_name,
309 + char *seen);
310 +
311 static inline int ce_path_match(const struct cache_entry *ce,
312 const struct pathspec *pathspec,
313 char *seen)
t/t3007-ls-files-recurse-submodules.sh
+105 -3
@@ -81,9 +81,111 @@ test_expect_success 'ls-files recurses more than 1 level' '
81 test_cmp expect actual
82 '
83
84 -test_expect_success '--recurse-submodules does not support using path arguments' '
85 - test_must_fail git ls-files --recurse-submodules b 2>actual &&
86 - test_i18ngrep "does not support pathspec" actual
84 +test_expect_success '--recurse-submodules and pathspecs setup' '
85 + echo e >submodule/subsub/e.txt &&
86 + git -C submodule/subsub add e.txt &&
87 + git -C submodule/subsub commit -m "adding e.txt" &&
88 + echo f >submodule/f.TXT &&
89 + echo g >submodule/g.txt &&
90 + git -C submodule add f.TXT g.txt &&
91 + git -C submodule commit -m "add f and g" &&
92 + echo h >h.txt &&
93 + mkdir sib &&
94 + echo sib >sib/file &&
95 + git add h.txt sib/file &&
96 + git commit -m "add h and sib/file" &&
97 + git init sub &&
98 + echo sub >sub/file &&
99 + git -C sub add file &&
100 + git -C sub commit -m "add file" &&
101 + git submodule add ./sub &&
102 + git commit -m "added sub" &&
103 +
104 + cat >expect <<-\EOF &&
105 + .gitmodules
106 + a
107 + b/b
108 + h.txt
109 + sib/file
110 + sub/file
111 + submodule/.gitmodules
112 + submodule/c
113 + submodule/f.TXT
114 + submodule/g.txt
115 + submodule/subsub/d
116 + submodule/subsub/e.txt
117 + EOF
118 +
119 + git ls-files --recurse-submodules >actual &&
120 + test_cmp expect actual &&
121 + cat actual &&
122 + git ls-files --recurse-submodules "*" >actual &&
123 + test_cmp expect actual
124 +'
125 +
126 +test_expect_success '--recurse-submodules and pathspecs' '
127 + cat >expect <<-\EOF &&
128 + h.txt
129 + submodule/g.txt
130 + submodule/subsub/e.txt
131 + EOF
132 +
133 + git ls-files --recurse-submodules "*.txt" >actual &&
134 + test_cmp expect actual
135 +'
136 +
137 +test_expect_success '--recurse-submodules and pathspecs' '
138 + cat >expect <<-\EOF &&
139 + h.txt
140 + submodule/f.TXT
141 + submodule/g.txt
142 + submodule/subsub/e.txt
143 + EOF
144 +
145 + git ls-files --recurse-submodules ":(icase)*.txt" >actual &&
146 + test_cmp expect actual
147 +'
148 +
149 +test_expect_success '--recurse-submodules and pathspecs' '
150 + cat >expect <<-\EOF &&
151 + h.txt
152 + submodule/f.TXT
153 + submodule/g.txt
154 + EOF
155 +
156 + git ls-files --recurse-submodules ":(icase)*.txt" ":(exclude)submodule/subsub/*" >actual &&
157 + test_cmp expect actual
158 +'
159 +
160 +test_expect_success '--recurse-submodules and pathspecs' '
161 + cat >expect <<-\EOF &&
162 + sub/file
163 + EOF
164 +
165 + git ls-files --recurse-submodules "sub" >actual &&
166 + test_cmp expect actual &&
167 + git ls-files --recurse-submodules "sub/" >actual &&
168 + test_cmp expect actual &&
169 + git ls-files --recurse-submodules "sub/file" >actual &&
170 + test_cmp expect actual &&
171 + git ls-files --recurse-submodules "su*/file" >actual &&
172 + test_cmp expect actual &&
173 + git ls-files --recurse-submodules "su?/file" >actual &&
174 + test_cmp expect actual
175 +'
176 +
177 +test_expect_success '--recurse-submodules and pathspecs' '
178 + cat >expect <<-\EOF &&
179 + sib/file
180 + sub/file
181 + EOF
182 +
183 + git ls-files --recurse-submodules "s??/file" >actual &&
184 + test_cmp expect actual &&
185 + git ls-files --recurse-submodules "s???file" >actual &&
186 + test_cmp expect actual &&
187 + git ls-files --recurse-submodules "s*file" >actual &&
188 + test_cmp expect actual
189 '
190
191 test_expect_success '--recurse-submodules does not support --error-unmatch' '