Skip to content

Commit b40fcd1

Browse files
bmwillgitster
bmwill
authored and
gitster
committed
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 super module and not with respect to paths in the submodule. The approach this patch takes is to pass in the identical pathspec from the super module to the submodule in addition to the submodule-prefix, which is the path from the root of the super module 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 super module not knowing what files could exist in the submodule. Signed-off-by: Brandon Williams <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 0e91b3f commit b40fcd1

File tree

4 files changed

+231
-62
lines changed

4 files changed

+231
-62
lines changed

‎builtin/ls-files.c

Lines changed: 80 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -177,12 +177,34 @@ static void show_gitlink(const struct cache_entry *ce)
177177
{
178178
struct child_process cp = CHILD_PROCESS_INIT;
179179
int status;
180+
int i;
180181

181182
argv_array_push(&cp.args, "ls-files");
182183
argv_array_push(&cp.args, "--recurse-submodules");
183184
argv_array_pushf(&cp.args, "--submodule-prefix=%s%s/",
184185
submodule_prefix ? submodule_prefix : "",
185186
ce->name);
187+
/* add options */
188+
if (show_eol)
189+
argv_array_push(&cp.args, "--eol");
190+
if (show_valid_bit)
191+
argv_array_push(&cp.args, "-v");
192+
if (show_stage)
193+
argv_array_push(&cp.args, "--stage");
194+
if (show_cached)
195+
argv_array_push(&cp.args, "--cached");
196+
if (debug_mode)
197+
argv_array_push(&cp.args, "--debug");
198+
199+
/*
200+
* Pass in the original pathspec args. The submodule will be
201+
* responsible for prepending the 'submodule_prefix' prior to comparing
202+
* against the pathspec for matches.
203+
*/
204+
argv_array_push(&cp.args, "--");
205+
for (i = 0; i < pathspec.nr; i++)
206+
argv_array_push(&cp.args, pathspec.items[i].original);
207+
186208
cp.git_cmd = 1;
187209
cp.dir = ce->name;
188210
status = run_command(&cp);
@@ -192,57 +214,62 @@ static void show_gitlink(const struct cache_entry *ce)
192214

193215
static void show_ce_entry(const char *tag, const struct cache_entry *ce)
194216
{
217+
struct strbuf name = STRBUF_INIT;
195218
int len = max_prefix_len;
219+
if (submodule_prefix)
220+
strbuf_addstr(&name, submodule_prefix);
221+
strbuf_addstr(&name, ce->name);
196222

197223
if (len >= ce_namelen(ce))
198224
die("git ls-files: internal error - cache entry not superset of prefix");
199225

200-
if (!match_pathspec(&pathspec, ce->name, ce_namelen(ce),
201-
len, ps_matched,
202-
S_ISDIR(ce->ce_mode) || S_ISGITLINK(ce->ce_mode)))
203-
return;
204-
if (recurse_submodules && S_ISGITLINK(ce->ce_mode)) {
226+
if (recurse_submodules && S_ISGITLINK(ce->ce_mode) &&
227+
submodule_path_match(&pathspec, name.buf, ps_matched)) {
205228
show_gitlink(ce);
206-
return;
207-
}
229+
} else if (match_pathspec(&pathspec, name.buf, name.len,
230+
len, ps_matched,
231+
S_ISDIR(ce->ce_mode) ||
232+
S_ISGITLINK(ce->ce_mode))) {
233+
if (tag && *tag && show_valid_bit &&
234+
(ce->ce_flags & CE_VALID)) {
235+
static char alttag[4];
236+
memcpy(alttag, tag, 3);
237+
if (isalpha(tag[0]))
238+
alttag[0] = tolower(tag[0]);
239+
else if (tag[0] == '?')
240+
alttag[0] = '!';
241+
else {
242+
alttag[0] = 'v';
243+
alttag[1] = tag[0];
244+
alttag[2] = ' ';
245+
alttag[3] = 0;
246+
}
247+
tag = alttag;
248+
}
208249

209-
if (tag && *tag && show_valid_bit &&
210-
(ce->ce_flags & CE_VALID)) {
211-
static char alttag[4];
212-
memcpy(alttag, tag, 3);
213-
if (isalpha(tag[0]))
214-
alttag[0] = tolower(tag[0]);
215-
else if (tag[0] == '?')
216-
alttag[0] = '!';
217-
else {
218-
alttag[0] = 'v';
219-
alttag[1] = tag[0];
220-
alttag[2] = ' ';
221-
alttag[3] = 0;
250+
if (!show_stage) {
251+
fputs(tag, stdout);
252+
} else {
253+
printf("%s%06o %s %d\t",
254+
tag,
255+
ce->ce_mode,
256+
find_unique_abbrev(ce->sha1,abbrev),
257+
ce_stage(ce));
258+
}
259+
write_eolinfo(ce, ce->name);
260+
write_name(ce->name);
261+
if (debug_mode) {
262+
const struct stat_data *sd = &ce->ce_stat_data;
263+
264+
printf(" ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec);
265+
printf(" mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec);
266+
printf(" dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino);
267+
printf(" uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid);
268+
printf(" size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags);
222269
}
223-
tag = alttag;
224270
}
225271

226-
if (!show_stage) {
227-
fputs(tag, stdout);
228-
} else {
229-
printf("%s%06o %s %d\t",
230-
tag,
231-
ce->ce_mode,
232-
find_unique_abbrev(ce->sha1,abbrev),
233-
ce_stage(ce));
234-
}
235-
write_eolinfo(ce, ce->name);
236-
write_name(ce->name);
237-
if (debug_mode) {
238-
const struct stat_data *sd = &ce->ce_stat_data;
239-
240-
printf(" ctime: %d:%d\n", sd->sd_ctime.sec, sd->sd_ctime.nsec);
241-
printf(" mtime: %d:%d\n", sd->sd_mtime.sec, sd->sd_mtime.nsec);
242-
printf(" dev: %d\tino: %d\n", sd->sd_dev, sd->sd_ino);
243-
printf(" uid: %d\tgid: %d\n", sd->sd_uid, sd->sd_gid);
244-
printf(" size: %d\tflags: %x\n", sd->sd_size, ce->ce_flags);
245-
}
272+
strbuf_release(&name);
246273
}
247274

248275
static void show_ru_info(void)
@@ -566,27 +593,28 @@ int cmd_ls_files(int argc, const char **argv, const char *cmd_prefix)
566593
setup_work_tree();
567594

568595
if (recurse_submodules &&
569-
(show_stage || show_deleted || show_others || show_unmerged ||
570-
show_killed || show_modified || show_resolve_undo ||
571-
show_valid_bit || show_tag || show_eol))
572-
die("ls-files --recurse-submodules can only be used in "
573-
"--cached mode");
596+
(show_deleted || show_others || show_unmerged ||
597+
show_killed || show_modified || show_resolve_undo))
598+
die("ls-files --recurse-submodules unsupported mode");
574599

575600
if (recurse_submodules && error_unmatch)
576601
die("ls-files --recurse-submodules does not support "
577602
"--error-unmatch");
578603

579-
if (recurse_submodules && argc)
580-
die("ls-files --recurse-submodules does not support path "
581-
"arguments");
582-
583604
parse_pathspec(&pathspec, 0,
584605
PATHSPEC_PREFER_CWD |
585606
PATHSPEC_STRIP_SUBMODULE_SLASH_CHEAP,
586607
prefix, argv);
587608

588-
/* Find common prefix for all pathspec's */
589-
max_prefix = common_prefix(&pathspec);
609+
/*
610+
* Find common prefix for all pathspec's
611+
* This is used as a performance optimization which unfortunately cannot
612+
* be done when recursing into submodules
613+
*/
614+
if (recurse_submodules)
615+
max_prefix = NULL;
616+
else
617+
max_prefix = common_prefix(&pathspec);
590618
max_prefix_len = max_prefix ? strlen(max_prefix) : 0;
591619

592620
/* Treat unmatching pathspec elements as errors */

‎dir.c

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,9 @@ int within_depth(const char *name, int namelen,
207207
return 1;
208208
}
209209

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)
212213

213214
/*
214215
* Does 'match' match the given name?
@@ -283,6 +284,29 @@ static int match_pathspec_item(const struct pathspec_item *item, int prefix,
283284
item->nowildcard_len - prefix))
284285
return MATCHED_FNMATCH;
285286

287+
/* Perform checks to see if "name" is a super set of the pathspec */
288+
if (flags & DO_MATCH_SUBMODULE) {
289+
/* Check if the name is a literal prefix of the pathspec */
290+
if ((item->match[namelen] == '/') &&
291+
!ps_strncmp(item, match, name, namelen))
292+
return MATCHED_RECURSIVELY;
293+
294+
/*
295+
* Here is where we would perform a wildmatch to check if
296+
* "name" can be matched as a directory (or a prefix) against
297+
* the pathspec. Since wildmatch doesn't have this capability
298+
* at the present we have to punt and say that it is a match,
299+
* esentially returning a false positive (as long as "name"
300+
* matches upto the first wild character).
301+
* The submodules themselves will be able to perform more
302+
* accurate matching to determine if the pathspec matches.
303+
*/
304+
if (item->nowildcard_len < item->len &&
305+
!ps_strncmp(item, match, name,
306+
item->nowildcard_len - prefix))
307+
return MATCHED_RECURSIVELY;
308+
}
309+
286310
return 0;
287311
}
288312

@@ -386,6 +410,21 @@ int match_pathspec(const struct pathspec *ps,
386410
return negative ? 0 : positive;
387411
}
388412

413+
/**
414+
* Check if a submodule is a superset of the pathspec
415+
*/
416+
int submodule_path_match(const struct pathspec *ps,
417+
const char *submodule_name,
418+
char *seen)
419+
{
420+
int matched = do_match_pathspec(ps, submodule_name,
421+
strlen(submodule_name),
422+
0, seen,
423+
DO_MATCH_DIRECTORY |
424+
DO_MATCH_SUBMODULE);
425+
return matched;
426+
}
427+
389428
int report_path_error(const char *ps_matched,
390429
const struct pathspec *pathspec,
391430
const char *prefix)

‎dir.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,10 @@ extern int git_fnmatch(const struct pathspec_item *item,
304304
const char *pattern, const char *string,
305305
int prefix);
306306

307+
extern int submodule_path_match(const struct pathspec *ps,
308+
const char *submodule_name,
309+
char *seen);
310+
307311
static inline int ce_path_match(const struct cache_entry *ce,
308312
const struct pathspec *pathspec,
309313
char *seen)

‎t/t3007-ls-files-recurse-submodules.sh

Lines changed: 106 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -69,9 +69,111 @@ test_expect_success 'ls-files recurses more than 1 level' '
6969
test_cmp expect actual
7070
'
7171

72-
test_expect_success '--recurse-submodules does not support using path arguments' '
73-
test_must_fail git ls-files --recurse-submodules b 2>actual &&
74-
test_i18ngrep "does not support path arguments" actual
72+
test_expect_success '--recurse-submodules and pathspecs setup' '
73+
echo e >submodule/subsub/e.txt &&
74+
git -C submodule/subsub add e.txt &&
75+
git -C submodule/subsub commit -m "adding e.txt" &&
76+
echo f >submodule/f.TXT &&
77+
echo g >submodule/g.txt &&
78+
git -C submodule add f.TXT g.txt &&
79+
git -C submodule commit -m "add f and g" &&
80+
echo h >h.txt &&
81+
mkdir sib &&
82+
echo sib >sib/file &&
83+
git add h.txt sib/file &&
84+
git commit -m "add h and sib/file" &&
85+
git init sub &&
86+
echo sub >sub/file &&
87+
git -C sub add file &&
88+
git -C sub commit -m "add file" &&
89+
git submodule add ./sub &&
90+
git commit -m "added sub" &&
91+
92+
cat >expect <<-\EOF &&
93+
.gitmodules
94+
a
95+
b/b
96+
h.txt
97+
sib/file
98+
sub/file
99+
submodule/.gitmodules
100+
submodule/c
101+
submodule/f.TXT
102+
submodule/g.txt
103+
submodule/subsub/d
104+
submodule/subsub/e.txt
105+
EOF
106+
107+
git ls-files --recurse-submodules >actual &&
108+
test_cmp expect actual &&
109+
cat actual &&
110+
git ls-files --recurse-submodules "*" >actual &&
111+
test_cmp expect actual
112+
'
113+
114+
test_expect_success '--recurse-submodules and pathspecs' '
115+
cat >expect <<-\EOF &&
116+
h.txt
117+
submodule/g.txt
118+
submodule/subsub/e.txt
119+
EOF
120+
121+
git ls-files --recurse-submodules "*.txt" >actual &&
122+
test_cmp expect actual
123+
'
124+
125+
test_expect_success '--recurse-submodules and pathspecs' '
126+
cat >expect <<-\EOF &&
127+
h.txt
128+
submodule/f.TXT
129+
submodule/g.txt
130+
submodule/subsub/e.txt
131+
EOF
132+
133+
git ls-files --recurse-submodules ":(icase)*.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+
EOF
143+
144+
git ls-files --recurse-submodules ":(icase)*.txt" ":(exclude)submodule/subsub/*" >actual &&
145+
test_cmp expect actual
146+
'
147+
148+
test_expect_success '--recurse-submodules and pathspecs' '
149+
cat >expect <<-\EOF &&
150+
sub/file
151+
EOF
152+
153+
git ls-files --recurse-submodules "sub" >actual &&
154+
test_cmp expect actual &&
155+
git ls-files --recurse-submodules "sub/" >actual &&
156+
test_cmp expect actual &&
157+
git ls-files --recurse-submodules "sub/file" >actual &&
158+
test_cmp expect actual &&
159+
git ls-files --recurse-submodules "su*/file" >actual &&
160+
test_cmp expect actual &&
161+
git ls-files --recurse-submodules "su?/file" >actual &&
162+
test_cmp expect actual
163+
'
164+
165+
test_expect_success '--recurse-submodules and pathspecs' '
166+
cat >expect <<-\EOF &&
167+
sib/file
168+
sub/file
169+
EOF
170+
171+
git ls-files --recurse-submodules "s??/file" >actual &&
172+
test_cmp expect actual &&
173+
git ls-files --recurse-submodules "s???file" >actual &&
174+
test_cmp expect actual &&
175+
git ls-files --recurse-submodules "s*file" >actual &&
176+
test_cmp expect actual
75177
'
76178

77179
test_expect_success '--recurse-submodules does not support --error-unmatch' '
@@ -82,18 +184,14 @@ test_expect_success '--recurse-submodules does not support --error-unmatch' '
82184
test_incompatible_with_recurse_submodules () {
83185
test_expect_success "--recurse-submodules and $1 are incompatible" "
84186
test_must_fail git ls-files --recurse-submodules $1 2>actual &&
85-
test_i18ngrep 'can only be used in --cached mode' actual
187+
test_i18ngrep 'unsupported mode' actual
86188
"
87189
}
88190

89-
test_incompatible_with_recurse_submodules -v
90-
test_incompatible_with_recurse_submodules -t
91191
test_incompatible_with_recurse_submodules --deleted
92192
test_incompatible_with_recurse_submodules --modified
93193
test_incompatible_with_recurse_submodules --others
94-
test_incompatible_with_recurse_submodules --stage
95194
test_incompatible_with_recurse_submodules --killed
96195
test_incompatible_with_recurse_submodules --unmerged
97-
test_incompatible_with_recurse_submodules --eol
98196

99197
test_done

0 commit comments

Comments
 (0)