| Message ID | pull.631.git.1589099162707.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | submodule--helper.c: add only-active to foreach | expand |
Hey Guillaume, > 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: Spelling: achive -> achieve Maybe we can keep the commit message a bit more imperative? Something like: ------------------------- On a repository with some submodules not active, one may need to run a command only for an active submodule or vice-versa. To achieve this, one may use: git submodule foreach 'git -C $toplevel submodule--helper is-active \ $sm_path && pwd || :' Simplify this expression to make it more readable and easy-to-use by adding the flag `--is-active` to subcommand `foreach` of `git submodule`. Thus, simplifying the above command to: git submodule--helper foreach --is-active pwd ------------------------- Yes, maybe renaming the flag to `--is-active` would make it a tad bit simpler? This commit message may not be perfect but it seems like an improvement over the previous one? To me this option seems good. It may have some good utility in the future. Similarly, we may change the struct to: struct foreach_cb { const char *prefix; int quiet; int recursive; int is_active; }; Therefore, the if-statement becomes: if (info->is_active && !is_submodule_active(the_repository, path)) return; BTW what do we return here, could you please be more specific? Again, the change here as well: OPT_BOOL(0, "is-active", &info.is_active, Here, too: N_("git submodule--helper foreach [--quiet] [--recursive] [--is-active] [--] <command>"), And, test_expect_success 'test "submodule--helper foreach --is-active" usage' ' Finally, git submodule--helper foreach --is-active "echo \$toplevel-\$name-\$path-\$sha1" > ../actual What do you think? Regards, Shourya Shukla
Hi Shourya > > > 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: > > Spelling: achive -> achieve agree > Maybe we can keep the commit message a bit more imperative? > Something like: > ------------------------- > On a repository with some submodules not active, one may need to run a > command only for an active submodule or vice-versa. To achieve this, > one may use: > git submodule foreach 'git -C $toplevel submodule--helper is-active \ > $sm_path && pwd || :' > > Simplify this expression to make it more readable and easy-to-use by > adding the flag `--is-active` to subcommand `foreach` of `git > submodule`. Thus, simplifying the above command to: > git submodule--helper foreach --is-active pwd > ------------------------- Agree with the changes except vice-versa. The original patch support only iterate the active submodule. > Yes, maybe renaming the flag to `--is-active` would make it a tad bit > simpler? is-active sound more like a question to me but I can change it. > This commit message may not be perfect but it seems like an > improvement over the previous one? yes definitely > To me this option seems good. It may have some good utility in the > future. Similarly, we may change the struct to: > struct foreach_cb { > const char *prefix; > int quiet; > int recursive; > int is_active; > }; > > Therefore, the if-statement becomes: > if (info->is_active && !is_submodule_active(the_repository, path)) > return; > > BTW what do we return here, could you please be more specific? This is a void function, returning here mean we will not execute the command. Should I add a comment like: return; // skip this submodule and go to next one but maybe it would be more readable to create a intermediate function which handle only the filtering part. Is it what you mean? > Again, the change here as well: > OPT_BOOL(0, "is-active", &info.is_active, > > Here, too: > N_("git submodule--helper foreach [--quiet] [--recursive] [--is-active] [--] <command>"), > > And, > test_expect_success 'test "submodule--helper foreach --is-active" usage' ' > > Finally, > git submodule--helper foreach --is-active "echo \$toplevel-\$name-\$path-\$sha1" > ../actual > > What do you think? > > Regards, > Shourya Shukla Now with the vice-versa idea in mind, I think it is maybe better to change a bit the original patch to add the option to execute command only on inactive submodule as well. Could someone need that in future? Basically this would mean: On struct foreach_cb instead of only_active adding field: int active; Defining some macro to hold possible value: #define FOREACH_ACTIVE 1 #define FOREACH_INACTIVE 0 #define FOREACH_ACTIVE_NOT_SET -1 Changing the FOREACH_CB_INIT to #define FOREACH_CB_INIT { 0, NULL, NULL, 0, 0, FOREACH_ACTIVE_NOT_SET } The filter become: int is_active; if (FOREACH_ACTIVE_NOT_SET != info->active) { is_active = is_submodule_active(the_repository, path); if ((is_active && (FOREACH_ACTIVE != info->active)) || (!is_active && (FOREACH_ACTIVE == info->active))) return; } It need two additionnal function to parse the argument: static int parse_active(const char *arg) { int active = git_parse_maybe_bool(arg); if (active < 0) die(_("invalid --active option: %s"), arg); return active; } static int parse_opt_active_cb(const struct option *opt, const char *arg, int unset) { if (unset) *(int *)opt->value = FOREACH_ACTIVE_NOT_SET; else if (arg) *(int *)opt->value = parse_active(arg); else *(int *)opt->value = FOREACH_ACTIVE; return 0; } And the option OPT_BOOL become a OPT_CALLBACK_F: OPT_CALLBACK_F(0, "active", &info.active, "true|false", N_("Call command depending on submodule active state"), PARSE_OPT_OPTARG | PARSE_OPT_NONEG, parse_opt_active_cb), The help git_submodule_helper_usage: N_("git submodule--helper foreach [--quiet] [--recursive] [--active[=true|false]] [--] <command>"), the added test change to: git submodule--helper foreach --active "echo \$toplevel-\$name-\$path-\$sha1" > ../actual and adding a test for the inactive submodule: cat > expect <<EOF Entering 'sub1' $pwd/clone-foo1-sub1-$sub1sha1 EOF test_expect_success 'test "submodule--helper foreach --active=false" 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 git submodule--helper foreach --active=false "echo \$toplevel-\$name-\$path-\$sha1" > ../actual ) && test_i18ncmp expect actual ' What do you think? Regards, Guillaume
On Sun, May 10, 2020 at 5:53 PM Guillaume Galeazzi <guillaume.galeazzi@gmail.com> wrote: > > Yes, maybe renaming the flag to `--is-active` would make it a tad bit > > simpler? > is-active sound more like a question to me but I can change it. I'm not a submodule user nor have I been following this discussion, but perhaps the name --active-only would be better?
On 10/05 11:51, Guillaume Galeazzi wrote: Before I comment on the patch, I want to apologise for the delay in the reply. I got caught up with some stuff. > Now with the vice-versa idea in mind, I think it is maybe better to > change a bit the original patch > to add the option to execute command only on inactive submodule as > well. Could someone need > that in future? > > Basically this would mean: > > On struct foreach_cb instead of only_active adding field: > int active; Yeah, keeping the option name as `active` would be better if we were to go for the inactive submodules option as well. > Defining some macro to hold possible value: > #define FOREACH_ACTIVE 1 > #define FOREACH_INACTIVE 0 > #define FOREACH_ACTIVE_NOT_SET -1 > > Changing the FOREACH_CB_INIT to > #define FOREACH_CB_INIT { 0, NULL, NULL, 0, 0, FOREACH_ACTIVE_NOT_SET } Do we really need to include the last macro here? > The filter become: > int is_active; > if (FOREACH_ACTIVE_NOT_SET != info->active) { > is_active = is_submodule_active(the_repository, path); > if ((is_active && (FOREACH_ACTIVE != info->active)) || > (!is_active && (FOREACH_ACTIVE == info->active))) > return; > } Is it okay to compare a macro directly? I have not actually seen it happen so I am a bit skeptical. I am tagging along some people who will be able to weigh in a solid opinion regarding this. > It need two additionnal function to parse the argument: > static int parse_active(const char *arg) > { > int active = git_parse_maybe_bool(arg); > > if (active < 0) > die(_("invalid --active option: %s"), arg); > > return active; > } Alright, this one is used for parsing out the active submodules right? > static int parse_opt_active_cb(const struct option *opt, const char *arg, > int unset) > { > if (unset) > *(int *)opt->value = FOREACH_ACTIVE_NOT_SET; > else if (arg) > *(int *)opt->value = parse_active(arg); > else > *(int *)opt->value = FOREACH_ACTIVE; > > return 0; > } > > And the option OPT_BOOL become a OPT_CALLBACK_F: > OPT_CALLBACK_F(0, "active", &info.active, "true|false", > N_("Call command depending on submodule active state"), > PARSE_OPT_OPTARG | PARSE_OPT_NONEG, > parse_opt_active_cb), > > The help git_submodule_helper_usage: > N_("git submodule--helper foreach [--quiet] [--recursive] > [--active[=true|false]] [--] <command>"), What I have inferred right now is that we introduce the `--active` option which will take a T/F value depending on user input. We have 3 macros to check for the value of `active`, but I don't understand the significance of changing the FOREACH_CB_INIT macro to accomodate the third option. And we use a function to parse out the active submodules. Instead of the return statement you wrote, won't it be better to call parse_active() depending on the case? Meaning that we call parse_active() when `active=true`. Regards, Shourya Shukla
"Guillaume G. via GitGitGadget" <gitgitgadget@gmail.com> writes: > 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). > 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. > @@ -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
Hi Junio > 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). The idea that other candidate are possible seem good. But then users will need combination like is-active && !is-populated. We will end up implementing a complex code to filter based on expression which is IMHO overkill. If is-populated is needed, that could be implemented this way: git submodule--helper --is-populated[=true|false] this would allow combination with the is active filter and the previous example would be git submodule--helper --is-active --is-populated=false <cmd> Regards, GG
Guillaume Galeazzi <guillaume.galeazzi@gmail.com> writes: >> 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). > > The idea that other candidate are possible seem good. But then users > will need combination like is-active && !is-populated. ... > ... this would allow combination with the is active filter and the > previous example would be > > git submodule--helper --is-active --is-populated=false <cmd> There is no difference between that and git submodule--helper --trait=is-active --trait=is-populated so I fail to see what new you are proposing, sorry.
>>> 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). >> >> The idea that other candidate are possible seem good. But then users >> will need combination like is-active && !is-populated. ... >> ... this would allow combination with the is active filter and the >> previous example would be >> >> git submodule--helper --is-active --is-populated=false <cmd> > > There is no difference between that and > > git submodule--helper --trait=is-active --trait=is-populated > > so I fail to see what new you are proposing, sorry. > The difference is that you repeat twice the same flag. Sometime repeating a flag overwrite the previous value, but it depend how it is implemented. In current case it should be possible to implement it this way, if this is required. Regarding previous example, it use '!' to negate the value. Not all people know the meaning of it. A new proposal would be: git submodule--helper foreach [--trait=[not-](active|populated)] What do you think?
Guillaume Galeazzi <guillaume.galeazzi@gmail.com> writes: > The difference is that you repeat twice the same flag. Sometime > repeating a flag overwrite the previous value,... I already said I was *not* suggesting a concrete syntax. The more important point was to make us realize that we need to think outside of "active-only" and make sure we can support other kinds of selection criteria for submodules. So whatever syntax you would want to use to specify more than one, combine, and negate, I do not care too deeply, as long as it is in line with what we use in the other parts of the system ;-). > Regarding previous example, it use '!' to negate the value. Not all > people know the meaning of it. We mark the bottom commit by negating with ^ (e.g. "git log ^maint master"), we mark "not ignored" entries by prefixing with '!' in .gitignore, we mark "unset" entries by prefixing with '-' in .gitattributes (the '!' prefix is used to mark "unspecified" entries), and "rev-list --boundary" output uses '~' prefix to mean "this is not part of the range". So there are many one-letter "not" we already use, and there seem to be no rule to pick which one in what context X-<. So spelling out "--no-blah" to mean "not with blah" is probably a good thing to do (especially if readers do not mind being English centric).
Le 13.05.2020 à 22:35, Junio C Hamano a écrit : > Guillaume Galeazzi <guillaume.galeazzi@gmail.com> writes: > > I already said I was *not* suggesting a concrete syntax. The more > important point was to make us realize that we need to think outside > of "active-only" and make sure we can support other kinds of selection > criteria for submodules. Ok get it now. So after looking a bit, another trait that could be interesting filtering on is remote tracking branch. A v2 with filtering based on active (or not), populated (or not), and tracked remote branch is ready on my side. It also include the rename of the struct member only_active to active_only. Let me know when I can /submit. > So spelling out "--no-blah" to mean "not with blah" is probably a > good thing to do (especially if readers do not mind being English > centric). > Great, it make it a bit simpler to code, thanks for the tips.
Le 11.05.2020 à 00:42, Eric Sunshine a écrit : > On Sun, May 10, 2020 at 5:53 PM Guillaume Galeazzi > <guillaume.galeazzi@gmail.com> wrote: >>> Yes, maybe renaming the flag to `--is-active` would make it a tad bit >>> simpler? >> is-active sound more like a question to me but I can change it. > > I'm not a submodule user nor have I been following this discussion, > but perhaps the name --active-only would be better? > To flow up on that topic, the flag can be `--[no-]active`. It simplify code and allow the optional negation. On the source, the variable will be renamed active_only.
Le 12.05.2020 à 16:15, Shourya Shukla a écrit : > On 10/05 11:51, Guillaume Galeazzi wrote: > > >> Defining some macro to hold possible value: >> #define FOREACH_ACTIVE 1 >> #define FOREACH_INACTIVE 0 >> #define FOREACH_ACTIVE_NOT_SET -1 >> >> Changing the FOREACH_CB_INIT to >> #define FOREACH_CB_INIT { 0, NULL, NULL, 0, 0, FOREACH_ACTIVE_NOT_SET } > > Do we really need to include the last macro here? After a cross check, yes it is the correct place to initialise the new active_only member of foreach_cb. But it will be changed to use designated initializers. >> The filter become: >> int is_active; >> if (FOREACH_ACTIVE_NOT_SET != info->active) { >> is_active = is_submodule_active(the_repository, path); >> if ((is_active && (FOREACH_ACTIVE != info->active)) || >> (!is_active && (FOREACH_ACTIVE == info->active))) >> return; >> } > > Is it okay to compare a macro directly? I have not actually seen it > happen so I am a bit skeptical. I am tagging along some people who > will be able to weigh in a solid opinion regarding this. Yes it is okay, a `#define SOMETHING WHATEVER` will just inform the c preprocessor to replace the `SOMETHING` by `WHATEVER`. The only thing the final c compiler will see is `WHATEVER`. In our case a integer value. Goal here was to avoid magic number, but after looking to the code it seem accepted that true is 1 and false is 0. To comply with that, in next version it will be replace it with: if (FOREACH_BOOL_FILTER_NOT_SET != info->active_only) { is_active = is_submodule_active(the_repository, path); if (is_active != info->active_only) return; } > >> It need two additionnal function to parse the argument: >> static int parse_active(const char *arg) >> { >> int active = git_parse_maybe_bool(arg); >> >> if (active < 0) >> die(_("invalid --active option: %s"), arg); >> >> return active; >> } > > Alright, this one is used for parsing out the active submodules right As suggested on other mail of this patch, it will be removed and take the shortcut `--no-active`. >> And the option OPT_BOOL become a OPT_CALLBACK_F: >> OPT_CALLBACK_F(0, "active", &info.active, "true|false", >> N_("Call command depending on submodule active state"), >> PARSE_OPT_OPTARG | PARSE_OPT_NONEG, >> parse_opt_active_cb), >> >> The help git_submodule_helper_usage: >> N_("git submodule--helper foreach [--quiet] [--recursive] >> [--active[=true|false]] [--] <command>"), > > What I have inferred right now is that we introduce the `--active` > option which will take a T/F value depending on user input. We have 3 > macros to check for the value of `active`, but I don't understand the > significance of changing the FOREACH_CB_INIT macro to accomodate the > third option. And we use a function to parse out the active > submodules. The change on `FOREACH_CB_INIT` are to keep original behaviour of the command if new flags are not given. > Instead of the return statement you wrote, won't it be better to call > parse_active() depending on the case? Meaning that we call > parse_active() when `active=true`. > > Regards, > Shourya Shukla > The code to parse command T/F will be removed. Regards, Guillaume
Guillaume Galeazzi <guillaume.galeazzi@gmail.com> writes: > Goal here was to avoid magic number, but after looking to the code it > seem accepted that true is 1 and false is 0. To comply with that, in > next version it will be replace it with: > > if (FOREACH_BOOL_FILTER_NOT_SET != info->active_only) { It still is unusual to have a constant on the left hand side of the "!=" or "==" operator, though. Having a constant on the left hand side of "<" and "<=" is justifiable, but not for "!=" and "==".
Le ven. 15 mai 2020 à 19:03, Junio C Hamano <gitster@pobox.com> a écrit : > > Guillaume Galeazzi <guillaume.galeazzi@gmail.com> writes: > > > Goal here was to avoid magic number, but after looking to the code it > > seem accepted that true is 1 and false is 0. To comply with that, in > > next version it will be replace it with: > > > > if (FOREACH_BOOL_FILTER_NOT_SET != info->active_only) { > > It still is unusual to have a constant on the left hand side of the > "!=" or "==" operator, though. Having a constant on the left hand > side of "<" and "<=" is justifiable, but not for "!=" and "==". It is call Yoda condition. As it compare with a constant, the compiler will throw an error if you write only = instead of != or ==. But after a quick check, this wasn't needed as compiler warn builtin/submodule--helper.c:570:2: error: suggest parentheses around assignment used as truth value [-Werror=parentheses] And I am not a Yoda condition adept at all. So it will be removed.
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; }; #define FOREACH_CB_INIT { 0 } @@ -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