Re: [PATCH] submodule--helper.c: add only-active to foreach
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 12, 2020, 18:53 UTC
- Message-ID
- <xmqq8shxc7ct.fsf@gitster.c.googlers.com>
- In-Reply-To
- <pull.631.git.1589099162707.gitgitgadget@gmail.com>
"Guillaume G. via GitGitGadget" <gitgitgadget@gmail.com> writes:
Show 8 quoted lines
> From: Guillaume Galeazzi <guillaume.galeazzi@gmail.com> > > On repository with some submodule not active, it could be needed to run > a command only for active submodule. Today it can be achived with the > command: > > git submodule foreach 'git -C $toplevel submodule--helper is-active \ > $sm_path && pwd || :'
"it could be needed" is being too modest.
To iterate only on active submodules, we can do ...
... << the above command >> ...
but it is inefficient to ask about each and every submodule.
may be convincing enough.
If iterating over only active ones is useful, surely it would also be useful to be able to iterate over only inactive ones, right?
So, before getting married too much to the use-case of "only active ones" and getting our eyes clouded from seeing a larger picture, let's see what other "traits" of submodules we can use to pick which ones to act on.
Are there attributes other than "is-active" that we may want to and can check about submodules? There is is_submodule_populated() next to is_submodule_active(), which might be a candidate. IOW, what I am wondering is if it makes sense to extend this to
git submodule foreach --trait=is-active ... git submodule foreach --trait=!is-active ... git submodule foreach --trait=is-populated ...
to allow iterating only on submodules with/without given trait (I am not suggesting the actual option name, but merely making sure that 'is-active' is not anything special but one of the possibilities that can be used to limit the iteration using the same mechanism).
Show 13 quoted lines
> builtin/submodule--helper.c | 8 +++++++-
> t/t7407-submodule-foreach.sh | 16 ++++++++++++++++
> 2 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 1a4b391c882..1a275403764 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -450,6 +450,7 @@ struct foreach_cb {
> const char *prefix;
> int quiet;
> int recursive;
> + int only_active;And I tend to agree with Eric downthread that active_only would be a more natural name if we want to have this field.
Show 54 quoted lines
> @@ -464,6 +465,9 @@ static void runcommand_in_submodule_cb(const struct cache_entry *list_item,
> struct child_process cp = CHILD_PROCESS_INIT;
> char *displaypath;
>
> + if (info->only_active && !is_submodule_active(the_repository, path))
> + return;
> +
> displaypath = get_submodule_displaypath(path, info->prefix);
>
> sub = submodule_from_path(the_repository, &null_oid, path);
> @@ -565,11 +569,13 @@ static int module_foreach(int argc, const char **argv, const char *prefix)
> OPT__QUIET(&info.quiet, N_("Suppress output of entering each submodule command")),
> OPT_BOOL(0, "recursive", &info.recursive,
> N_("Recurse into nested submodules")),
> + OPT_BOOL(0, "only-active", &info.only_active,
> + N_("Call command only for active submodules")),
> OPT_END()
> };
>
> const char *const git_submodule_helper_usage[] = {
> - N_("git submodule--helper foreach [--quiet] [--recursive] [--] <command>"),
> + N_("git submodule--helper foreach [--quiet] [--recursive] [--only-active] [--] <command>"),
> NULL
> };
>
> diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh
> index 6b2aa917e11..f90a16e3e67 100755
> --- a/t/t7407-submodule-foreach.sh
> +++ b/t/t7407-submodule-foreach.sh
> @@ -80,6 +80,22 @@ test_expect_success 'test basic "submodule foreach" usage' '
> test_i18ncmp expect actual
> '
>
> +sub3sha1=$(cd super/sub3 && git rev-parse HEAD)
> +cat > expect <<EOF
> +Entering 'sub3'
> +$pwd/clone-foo3-sub3-$sub3sha1
> +EOF
> +
> +test_expect_success 'test "submodule--helper foreach --only-active" usage' '
> + test_when_finished "git -C clone config --unset submodule.foo1.active" &&
> + (
> + cd clone &&
> + git config --bool submodule.foo1.active "false" &&
> + git submodule--helper foreach --only-active "echo \$toplevel-\$name-\$path-\$sha1" > ../actual
> + ) &&
> + test_i18ncmp expect actual
> +'
> +
> cat >expect <<EOF
> Entering '../sub1'
> $pwd/clone-foo1-sub1-../sub1-$sub1sha1
>
> base-commit: b994622632154fc3b17fb40a38819ad954a5fb88