grep: fix bug when recursing with relative pathspec
When using the --recurse-submodules flag with a relative pathspec which includes "..", an error is produced inside the child process spawned for a submodule. When creating the pathspec struct in the child, the ".." is interpreted to mean "go up a directory" which causes an error stating that the path ".." is outside of the repository. While it is true that ".." is outside the scope of the submodule, it is confusing to a user who originally invoked the command where ".." was indeed still inside the scope of the superproject. Since the child process launched for the submodule has some context that it is operating underneath a superproject, this error could be avoided. This patch fixes the bug by passing the 'prefix' to the child process. Now each child process that works on a submodule has two points of reference to the superproject: (1) the 'super_prefix' which is the path from the root of the superproject down to root of the submodule and (2) the 'prefix' which is the path from the root of the superproject down to the directory where the user invoked the git command. With these two pieces of information a child process can correctly interpret the pathspecs provided by the user as well as being able to properly format its output relative to the directory the user invoked the original command from. Signed-off-by: Brandon Williams <bmwill@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Brandon Williams committed
Mar 17, 2017 at 10:22 UTC
be80a2392f5d10395bf934a066faeefdb6bb380a
2 files changed
+99
-15
builtin/grep.c
+24
-15
@@ -310,10 +310,7 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
310
{
311
struct strbuf pathbuf = STRBUF_INIT;
312
313
- if (opt->relative && opt->prefix_length) {
314
- quote_path_relative(filename + tree_name_len, opt->prefix, &pathbuf);
315
- strbuf_insert(&pathbuf, 0, filename, tree_name_len);
316
- } else if (super_prefix) {
313
+ if (super_prefix) {
314
strbuf_add(&pathbuf, filename, tree_name_len);
315
strbuf_addstr(&pathbuf, super_prefix);
316
strbuf_addstr(&pathbuf, filename + tree_name_len);
@@ -321,6 +318,13 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
318
strbuf_addstr(&pathbuf, filename);
319
}
320
321
+ if (opt->relative && opt->prefix_length) {
322
+ char *name = strbuf_detach(&pathbuf, NULL);
323
+ quote_path_relative(name + tree_name_len, opt->prefix, &pathbuf);
324
+ strbuf_insert(&pathbuf, 0, name, tree_name_len);
325
+ free(name);
326
+ }
327
+
328
#ifndef NO_PTHREADS
329
if (num_threads) {
330
add_work(opt, GREP_SOURCE_SHA1, pathbuf.buf, path, sha1);
@@ -345,12 +349,14 @@ static int grep_file(struct grep_opt *opt, const char *filename)
349
{
350
struct strbuf buf = STRBUF_INIT;
351
352
+ if (super_prefix)
353
+ strbuf_addstr(&buf, super_prefix);
354
+ strbuf_addstr(&buf, filename);
355
+
356
if (opt->relative && opt->prefix_length) {
349
- quote_path_relative(filename, opt->prefix, &buf);
350
- } else {
351
- if (super_prefix)
352
- strbuf_addstr(&buf, super_prefix);
353
- strbuf_addstr(&buf, filename);
357
+ char *name = strbuf_detach(&buf, NULL);
358
+ quote_path_relative(name, opt->prefix, &buf);
359
+ free(name);
360
}
361
362
#ifndef NO_PTHREADS
@@ -399,13 +405,12 @@ static void run_pager(struct grep_opt *opt, const char *prefix)
405
}
406
407
static void compile_submodule_options(const struct grep_opt *opt,
402
- const struct pathspec *pathspec,
408
+ const char **argv,
409
int cached, int untracked,
410
int opt_exclude, int use_index,
411
int pattern_type_arg)
412
{
413
struct grep_pat *pattern;
408
- int i;
414
415
if (recurse_submodules)
416
argv_array_push(&submodule_options, "--recurse-submodules");
@@ -523,9 +528,8 @@ static void compile_submodule_options(const struct grep_opt *opt,
528
529
/* Add Pathspecs */
530
argv_array_push(&submodule_options, "--");
526
- for (i = 0; i < pathspec->nr; i++)
527
- argv_array_push(&submodule_options,
528
- pathspec->items[i].original);
531
+ for (; *argv; argv++)
532
+ argv_array_push(&submodule_options, *argv);
533
}
534
535
/*
@@ -549,6 +553,11 @@ static int grep_submodule_launch(struct grep_opt *opt,
553
prepare_submodule_repo_env(&cp.env_array);
554
argv_array_push(&cp.env_array, GIT_DIR_ENVIRONMENT);
555
556
+ if (opt->relative && opt->prefix_length)
557
+ argv_array_pushf(&cp.env_array, "%s=%s",
558
+ GIT_TOPLEVEL_PREFIX_ENVIRONMENT,
559
+ opt->prefix);
560
+
561
/* Add super prefix */
562
argv_array_pushf(&cp.args, "--super-prefix=%s%s/",
563
super_prefix ? super_prefix : "",
@@ -1206,7 +1215,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
1215
1216
if (recurse_submodules) {
1217
gitmodules_config();
1209
- compile_submodule_options(&opt, &pathspec, cached, untracked,
1218
+ compile_submodule_options(&opt, argv + i, cached, untracked,
1219
opt_exclude, use_index,
1220
pattern_type_arg);
1221
}
t/t7814-grep-recurse-submodules.sh
+75
@@ -227,6 +227,81 @@ test_expect_success 'grep history with moved submoules' '
227
test_cmp expect actual
228
'
229
230
+test_expect_success 'grep using relative path' '
231
+ test_when_finished "rm -rf parent sub" &&
232
+ git init sub &&
233
+ echo "foobar" >sub/file &&
234
+ git -C sub add file &&
235
+ git -C sub commit -m "add file" &&
236
+
237
+ git init parent &&
238
+ echo "foobar" >parent/file &&
239
+ git -C parent add file &&
240
+ mkdir parent/src &&
241
+ echo "foobar" >parent/src/file2 &&
242
+ git -C parent add src/file2 &&
243
+ git -C parent submodule add ../sub &&
244
+ git -C parent commit -m "add files and submodule" &&
245
+
246
+ # From top works
247
+ cat >expect <<-\EOF &&
248
+ file:foobar
249
+ src/file2:foobar
250
+ sub/file:foobar
251
+ EOF
252
+ git -C parent grep --recurse-submodules -e "foobar" >actual &&
253
+ test_cmp expect actual &&
254
+
255
+ # Relative path to top
256
+ cat >expect <<-\EOF &&
257
+ ../file:foobar
258
+ file2:foobar
259
+ ../sub/file:foobar
260
+ EOF
261
+ git -C parent/src grep --recurse-submodules -e "foobar" -- .. >actual &&
262
+ test_cmp expect actual &&
263
+
264
+ # Relative path to submodule
265
+ cat >expect <<-\EOF &&
266
+ ../sub/file:foobar
267
+ EOF
268
+ git -C parent/src grep --recurse-submodules -e "foobar" -- ../sub >actual &&
269
+ test_cmp expect actual
270
+'
271
+
272
+test_expect_success 'grep from a subdir' '
273
+ test_when_finished "rm -rf parent sub" &&
274
+ git init sub &&
275
+ echo "foobar" >sub/file &&
276
+ git -C sub add file &&
277
+ git -C sub commit -m "add file" &&
278
+
279
+ git init parent &&
280
+ mkdir parent/src &&
281
+ echo "foobar" >parent/src/file &&
282
+ git -C parent add src/file &&
283
+ git -C parent submodule add ../sub src/sub &&
284
+ git -C parent submodule add ../sub sub &&
285
+ git -C parent commit -m "add files and submodules" &&
286
+
287
+ # Verify grep from root works
288
+ cat >expect <<-\EOF &&
289
+ src/file:foobar
290
+ src/sub/file:foobar
291
+ sub/file:foobar
292
+ EOF
293
+ git -C parent grep --recurse-submodules -e "foobar" >actual &&
294
+ test_cmp expect actual &&
295
+
296
+ # Verify grep from a subdir works
297
+ cat >expect <<-\EOF &&
298
+ file:foobar
299
+ sub/file:foobar
300
+ EOF
301
+ git -C parent/src grep --recurse-submodules -e "foobar" >actual &&
302
+ test_cmp expect actual
303
+'
304
+
305
test_incompatible_with_recurse_submodules ()
306
{
307
test_expect_success "--recurse-submodules and $1 are incompatible" "