On Fri, Nov 11, 2016 at 3:51 PM, Brandon Williams <bmwill@xxxxxxxxxx> wrote:
> to:
> HEAD:file
> HEAD:sub/file
Maybe indent this ;)
> static struct argv_array submodule_options = ARGV_ARRAY_INIT;
> +static const char *parent_basename;
>
> static int grep_submodule_launch(struct grep_opt *opt,
> const struct grep_source *gs);
> @@ -535,19 +537,53 @@ static int grep_submodule_launch(struct grep_opt *opt,
> {
> struct child_process cp = CHILD_PROCESS_INIT;
> int status, i;
> + const char *end_of_base;
> + const char *name;
> struct work_item *w = opt->output_priv;
>
> + end_of_base = strchr(gs->name, ':');
> + if (end_of_base)
> + name = end_of_base + 1;
> + else
> + name = gs->name;
> +
> prepare_submodule_repo_env(&cp.env_array);
>
> /* Add super prefix */
> argv_array_pushf(&cp.args, "--super-prefix=%s%s/",
> super_prefix ? super_prefix : "",
> - gs->name);
> + name);
> argv_array_push(&cp.args, "grep");
>
> + /*
> + * Add basename of parent project
> + * When performing grep on a <tree> object the filename is prefixed
> + * with the object's name: '<tree-name>:filename'.
This comment is hard to read as it's unclear what the <angle brackets> mean.
(Are the supposed to indicate a variable? If so why is file name not marked up?)
> In order to
> + * provide uniformity of output we want to pass the name of the
> + * parent project's object name to the submodule so the submodule can
> + * prefix its output with the parent's name and not its own SHA1.
> + */
> + if (end_of_base)
> + argv_array_pushf(&cp.args, "--parent-basename=%.*s",
> + (int) (end_of_base - gs->name),
> + gs->name);
Do we pass this only with the tree-ish?
What if we are grepping the working tree and the file name contains a colon?
> +test_expect_success 'grep tree HEAD^' '
> + cat >expect <<-\EOF &&
> + HEAD^:a:foobar
> + HEAD^:b/b:bar
> + HEAD^:submodule/a:foobar
> + EOF
> +
> + git grep -e "bar" --recurse-submodules HEAD^ > actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'grep tree HEAD^^' '
> + cat >expect <<-\EOF &&
> + HEAD^^:a:foobar
> + HEAD^^:b/b:bar
> + EOF
> +
> + git grep -e "bar" --recurse-submodules HEAD^^ > actual &&
> + test_cmp expect actual
> +'
> +
> +test_expect_success 'grep tree and pathspecs' '
> + cat >expect <<-\EOF &&
> + HEAD:submodule/a:foobar
> + HEAD:submodule/sub/a:foobar
> + EOF
> +
> + git grep -e "bar" --recurse-submodules HEAD -- submodule > actual &&
> + test_cmp expect actual
> +'
Mind to add tests for
* recursive submodules (say 2 levels), preferrably not having the
gitlink at the root each, i.e. root has a sub1 at path subs/sub1 and
sub1 has a sub2
at path subs/sub2, such that recursing would produce a path like
HEAD:subs/sub1/subs/sub2/dir/file ?
* file names with a colon in it
* instead of just HEAD referencing trees, maybe a sha1 referenced test as well
(though it is not immediately clear what the benefit would be)
* what if the submodule doesn't have the commit referenced in the given sha1
Thanks,
Stefan