* regression in output of git-pull --rebase --recurse-submodules=yes --quiet @ 2018-01-20 5:57 Robin H. Johnson 2018-01-25 19:08 ` [PATCH] builtin/pull: respect verbosity settings in submodules Stefan Beller 2019-04-10 6:41 ` regression AGAIN in output of git-pull --rebase --recurse-submodules=yes --quiet Robin H. Johnson 0 siblings, 2 replies; 12+ messages in thread From: Robin H. Johnson @ 2018-01-20 5:57 UTC (permalink / raw) To: Git Mailing List [-- Attachment #1: Type: text/plain, Size: 1155 bytes --] Somewhere between 2.13.6 & 2.14.1 there's an output regression. I haven't done a bisect to trace it down further yet. Specifically, --rebase --recurse-submodules=yes seems to cause --quiet to not be effective anymore. Full commandline: $ git pull --rebase --recurse-submodules --quiet In 2.13.6, there is no output, it's quiet as expect. In 2.14.1, you get: HEAD is up to date. Submodule path '_data/news': rebased into 'a50b763c338161b4621d23e9fa5cd6e11455d6ca' HEAD is up to date. Submodule path 'glep': rebased into 'e1f100ec3ba44ab1672d61cabf4690b355e46158' Steps to reproduction: 1. git clone --recurse-submodules \ https://anongit.gentoo.org/git/sites/www.git 2. cd www 3. git submodule foreach --quiet git pull --quiet origin master 4. git pull --rebase --recurse-submodules=yes --quiet Repeat step 4 for repeated bug output. If you drop the --rebase, then you need to re-run step 3 first. -- Robin Hugh Johnson Gentoo Linux: Dev, Infra Lead, Foundation Treasurer E-Mail : robbat2@gentoo.org GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85 GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136 [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 1113 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] builtin/pull: respect verbosity settings in submodules 2018-01-20 5:57 regression in output of git-pull --rebase --recurse-submodules=yes --quiet Robin H. Johnson @ 2018-01-25 19:08 ` Stefan Beller 2018-01-25 19:18 ` Junio C Hamano 2019-04-10 6:41 ` regression AGAIN in output of git-pull --rebase --recurse-submodules=yes --quiet Robin H. Johnson 1 sibling, 1 reply; 12+ messages in thread From: Stefan Beller @ 2018-01-25 19:08 UTC (permalink / raw) To: robbat2; +Cc: git, Stefan Beller In a6d7eb2c7a (pull: optionally rebase submodules (remote submodule changes only), 2017-06-23), we taught Git how to rebase submodules in a pull. However we missed to pass on the verbosity settings. Reported-by: Robin H. Johnson <robbat2@gentoo.org> Signed-off-by: Stefan Beller <sbeller@google.com> --- builtin/pull.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/pull.c b/builtin/pull.c index 511dbbe0f6..1876271af9 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -574,6 +574,7 @@ static int rebase_submodules(void) cp.no_stdin = 1; argv_array_pushl(&cp.args, "submodule", "update", "--recursive", "--rebase", NULL); + argv_push_verbosity(&cp.args); return run_command(&cp); } @@ -586,6 +587,7 @@ static int update_submodules(void) cp.no_stdin = 1; argv_array_pushl(&cp.args, "submodule", "update", "--recursive", "--checkout", NULL); + argv_push_verbosity(&cp.args); return run_command(&cp); } -- 2.16.0.rc1.238.g530d649a79-goog ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] builtin/pull: respect verbosity settings in submodules 2018-01-25 19:08 ` [PATCH] builtin/pull: respect verbosity settings in submodules Stefan Beller @ 2018-01-25 19:18 ` Junio C Hamano 0 siblings, 0 replies; 12+ messages in thread From: Junio C Hamano @ 2018-01-25 19:18 UTC (permalink / raw) To: Stefan Beller; +Cc: robbat2, git Stefan Beller <sbeller@google.com> writes: > In a6d7eb2c7a (pull: optionally rebase submodules (remote submodule > changes only), 2017-06-23), we taught Git how to rebase submodules in > a pull. However we missed to pass on the verbosity settings. Makes sense. Thanks. > > Reported-by: Robin H. Johnson <robbat2@gentoo.org> > Signed-off-by: Stefan Beller <sbeller@google.com> > --- > builtin/pull.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/builtin/pull.c b/builtin/pull.c > index 511dbbe0f6..1876271af9 100644 > --- a/builtin/pull.c > +++ b/builtin/pull.c > @@ -574,6 +574,7 @@ static int rebase_submodules(void) > cp.no_stdin = 1; > argv_array_pushl(&cp.args, "submodule", "update", > "--recursive", "--rebase", NULL); > + argv_push_verbosity(&cp.args); > > return run_command(&cp); > } > @@ -586,6 +587,7 @@ static int update_submodules(void) > cp.no_stdin = 1; > argv_array_pushl(&cp.args, "submodule", "update", > "--recursive", "--checkout", NULL); > + argv_push_verbosity(&cp.args); > > return run_command(&cp); > } ^ permalink raw reply [flat|nested] 12+ messages in thread
* regression AGAIN in output of git-pull --rebase --recurse-submodules=yes --quiet 2018-01-20 5:57 regression in output of git-pull --rebase --recurse-submodules=yes --quiet Robin H. Johnson 2018-01-25 19:08 ` [PATCH] builtin/pull: respect verbosity settings in submodules Stefan Beller @ 2019-04-10 6:41 ` Robin H. Johnson 2019-04-10 11:18 ` Duy Nguyen 2019-04-12 10:08 ` [PATCH] submodule foreach: fix "<command> --quiet" not being respected Nguyễn Thái Ngọc Duy 1 sibling, 2 replies; 12+ messages in thread From: Robin H. Johnson @ 2019-04-10 6:41 UTC (permalink / raw) To: Git Mailing List [-- Attachment #1: Type: text/plain, Size: 2334 bytes --] A year ago, I raised <robbat2-20190410T062730-540884809Z@orbis-terrarum.net> as an issue, which lead to commit commit a56771a668dd4963675914bc5da0e1e015952dae. The exact same workload somewhere between 2.18.0 and 2.19.0 has caused the message to come back. I noticed it first in a 2.18.0->2.21.0 upgrade, and did a partial bisect based on tags to trace it to 2.19.0. === $ git submodule foreach --quiet git pull origin master --quiet >/dev/null From git://anongit.gentoo.org/data/gentoo-news * branch master -> FETCH_HEAD From git://anongit.gentoo.org/data/glep * branch master -> FETCH_HEAD === I suspect it was a result of: ea27893a65cc41cad2710466aa6a58866ff22f1e Merge branch 'pc/submodule-helper-foreach' But I haven't done a full bisect to prove it yet. On Sat, Jan 20, 2018 at 05:57:29AM +0000, Robin H. Johnson wrote: > Somewhere between 2.13.6 & 2.14.1 there's an output regression. I > haven't done a bisect to trace it down further yet. > > Specifically, --rebase --recurse-submodules=yes seems to cause --quiet > to not be effective anymore. > > Full commandline: > $ git pull --rebase --recurse-submodules --quiet > > In 2.13.6, there is no output, it's quiet as expect. > > In 2.14.1, you get: > HEAD is up to date. > Submodule path '_data/news': rebased into 'a50b763c338161b4621d23e9fa5cd6e11455d6ca' > HEAD is up to date. > Submodule path 'glep': rebased into 'e1f100ec3ba44ab1672d61cabf4690b355e46158' > > Steps to reproduction: > 1. git clone --recurse-submodules \ > https://anongit.gentoo.org/git/sites/www.git > 2. cd www > 3. git submodule foreach --quiet git pull --quiet origin master > 4. git pull --rebase --recurse-submodules=yes --quiet > > Repeat step 4 for repeated bug output. > If you drop the --rebase, then you need to re-run step 3 first. > > -- > Robin Hugh Johnson > Gentoo Linux: Dev, Infra Lead, Foundation Treasurer > E-Mail : robbat2@gentoo.org > GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85 > GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136 -- Robin Hugh Johnson Gentoo Linux: Dev, Infra Lead, Foundation Treasurer E-Mail : robbat2@gentoo.org GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85 GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 1113 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: regression AGAIN in output of git-pull --rebase --recurse-submodules=yes --quiet 2019-04-10 6:41 ` regression AGAIN in output of git-pull --rebase --recurse-submodules=yes --quiet Robin H. Johnson @ 2019-04-10 11:18 ` Duy Nguyen 2019-04-12 7:08 ` Robin H. Johnson [not found] ` <CAODn77oL6sj5zvxgPGw=4TNqmnSeBq4=j2r2nx_51YHooECo7w@mail.gmail.com> 2019-04-12 10:08 ` [PATCH] submodule foreach: fix "<command> --quiet" not being respected Nguyễn Thái Ngọc Duy 1 sibling, 2 replies; 12+ messages in thread From: Duy Nguyen @ 2019-04-10 11:18 UTC (permalink / raw) To: Robin H. Johnson; +Cc: Git Mailing List, Prathamesh Chavan On Wed, Apr 10, 2019 at 06:41:05AM +0000, Robin H. Johnson wrote: > A year ago, I raised <robbat2-20190410T062730-540884809Z@orbis-terrarum.net> as an issue, > which lead to commit commit a56771a668dd4963675914bc5da0e1e015952dae. > > The exact same workload somewhere between 2.18.0 and 2.19.0 has caused > the message to come back. I noticed it first in a 2.18.0->2.21.0 > upgrade, and did a partial bisect based on tags to trace it to 2.19.0. > > === > $ git submodule foreach --quiet git pull origin master --quiet >/dev/null > From git://anongit.gentoo.org/data/gentoo-news > * branch master -> FETCH_HEAD > From git://anongit.gentoo.org/data/glep > * branch master -> FETCH_HEAD > === If you run this with GIT_TRACE=1, you can see that --quiet is passed to submodule--helper correctly. trace: built-in: git submodule--helper foreach --quiet git pull --quiet origin master The problem here is the option parser of this command would try to parse all options, so it considers both --quiet the same thing and are to tell "submodule--foreach" to be quiet, the second --quiet is not part of the "git pull" command anymore. So the fix would be to pass "--" to stop option parsing. submodule--helper should not parse options it does not understand anyway. Something like this should work. -- 8< -- diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 6bcc4f1bd7..6394222628 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -571,7 +571,7 @@ static int module_foreach(int argc, const char **argv, const char *prefix) }; argc = parse_options(argc, argv, prefix, module_foreach_options, - git_submodule_helper_usage, PARSE_OPT_KEEP_UNKNOWN); + git_submodule_helper_usage, 0); if (module_list_compute(0, NULL, prefix, &pathspec, &list) < 0) return 1; diff --git a/git-submodule.sh b/git-submodule.sh index 2c0fb6d723..a967b2890d 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -346,7 +346,7 @@ cmd_foreach() shift done - git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper foreach ${GIT_QUIET:+--quiet} ${recursive:+--recursive} "$@" + git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper foreach ${GIT_QUIET:+--quiet} ${recursive:+--recursive} -- "$@" } # -- 8< -- I'm a bit reluctant to follow up with a proper patch because I can't digest the t5572-submodule-pull.sh tests. And we definitely need to add a test case about --quiet to make sure it won't happen again. -- Duy > > I suspect it was a result of: > ea27893a65cc41cad2710466aa6a58866ff22f1e Merge branch 'pc/submodule-helper-foreach' > > But I haven't done a full bisect to prove it yet. > > On Sat, Jan 20, 2018 at 05:57:29AM +0000, Robin H. Johnson wrote: > > Somewhere between 2.13.6 & 2.14.1 there's an output regression. I > > haven't done a bisect to trace it down further yet. > > > > Specifically, --rebase --recurse-submodules=yes seems to cause --quiet > > to not be effective anymore. > > > > Full commandline: > > $ git pull --rebase --recurse-submodules --quiet > > > > In 2.13.6, there is no output, it's quiet as expect. > > > > In 2.14.1, you get: > > HEAD is up to date. > > Submodule path '_data/news': rebased into 'a50b763c338161b4621d23e9fa5cd6e11455d6ca' > > HEAD is up to date. > > Submodule path 'glep': rebased into 'e1f100ec3ba44ab1672d61cabf4690b355e46158' > > > > Steps to reproduction: > > 1. git clone --recurse-submodules \ > > https://anongit.gentoo.org/git/sites/www.git > > 2. cd www > > 3. git submodule foreach --quiet git pull --quiet origin master > > 4. git pull --rebase --recurse-submodules=yes --quiet > > > > Repeat step 4 for repeated bug output. > > If you drop the --rebase, then you need to re-run step 3 first. > > > > -- > > Robin Hugh Johnson > > Gentoo Linux: Dev, Infra Lead, Foundation Treasurer > > E-Mail : robbat2@gentoo.org > > GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85 > > GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136 > > > > -- > Robin Hugh Johnson > Gentoo Linux: Dev, Infra Lead, Foundation Treasurer > E-Mail : robbat2@gentoo.org > GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85 > GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: regression AGAIN in output of git-pull --rebase --recurse-submodules=yes --quiet 2019-04-10 11:18 ` Duy Nguyen @ 2019-04-12 7:08 ` Robin H. Johnson 2019-04-12 9:25 ` Duy Nguyen 2019-04-15 14:40 ` Johannes Schindelin [not found] ` <CAODn77oL6sj5zvxgPGw=4TNqmnSeBq4=j2r2nx_51YHooECo7w@mail.gmail.com> 1 sibling, 2 replies; 12+ messages in thread From: Robin H. Johnson @ 2019-04-12 7:08 UTC (permalink / raw) To: Duy Nguyen, Git Mailing List; +Cc: Robin H. Johnson, Prathamesh Chavan [-- Attachment #1.1: Type: text/plain, Size: 1260 bytes --] On Wed, Apr 10, 2019 at 06:18:35PM +0700, Duy Nguyen wrote: > ... Thanks, I tested, and had good results in almost all of my tests. Almost all: config setting of 'pull.rebase=preserve' === $ git submodule foreach --quiet git pull --quiet origin master >/dev/null Successfully rebased and updated detached HEAD. Successfully rebased and updated detached HEAD. $ git pull --rebase --recurse-submodules=yes --quiet >/dev/null $ === Looking at git-rebase--preserve-merges.sh for this message, I think that should be a separate patch to make it respect --quiet. > -- 8< -- (snip patch, please add my DCO signed-off-by) Tested-by: Robin H. Johnson <robbat2@gentoo.org> Signed-off-by: Robin H. Johnson <robbat2@gentoo.org> > -- 8< -- > > I'm a bit reluctant to follow up with a proper patch because I can't > digest the t5572-submodule-pull.sh tests. And we definitely need to > add a test case about --quiet to make sure it won't happen again. Find testcase attached. Please submit in a series with your patch -- Robin Hugh Johnson Gentoo Linux: Dev, Infra Lead, Foundation Treasurer E-Mail : robbat2@gentoo.org GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85 GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136 [-- Attachment #1.2: 0001-submodule-foreach-test-foreach-option-swallowing.patch --] [-- Type: text/x-diff, Size: 1271 bytes --] From a57994f2d78134936521375ba9798a1b7418e230 Mon Sep 17 00:00:00 2001 From: "Robin H. Johnson" <robbat2@gentoo.org> Date: Fri, 12 Apr 2019 00:00:07 -0700 Subject: [PATCH] submodule foreach: test foreach option swallowing Add a testcase for submodule foreach option parsing not knowing where to stop taking options, and accidently removing options intended for foreach target commands. CC: Duy Nguyen <pclouds@gmail.com> CC: Prathamesh Chavan <pc44800@gmail.com> Signed-off-by: Robin H. Johnson <robbat2@gentoo.org> --- t/t7407-submodule-foreach.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh index 77729ac4aa..706ae762e0 100755 --- a/t/t7407-submodule-foreach.sh +++ b/t/t7407-submodule-foreach.sh @@ -411,4 +411,14 @@ test_expect_success 'multi-argument command passed to foreach is not shell-evalu test_cmp expected actual ' +test_expect_success 'option-like arguments passed to foreach commands are not lost' ' + ( + cd super && + git submodule foreach "echo be --quiet" > ../expected && + git submodule foreach echo be --quiet > ../actual + ) && + grep -sq -e "--quiet" expected && + test_cmp expected actual +' + test_done -- 2.21.0 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 1113 bytes --] ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: regression AGAIN in output of git-pull --rebase --recurse-submodules=yes --quiet 2019-04-12 7:08 ` Robin H. Johnson @ 2019-04-12 9:25 ` Duy Nguyen 2019-04-15 14:40 ` Johannes Schindelin 1 sibling, 0 replies; 12+ messages in thread From: Duy Nguyen @ 2019-04-12 9:25 UTC (permalink / raw) To: Robin H. Johnson; +Cc: Git Mailing List, Prathamesh Chavan On Fri, Apr 12, 2019 at 2:09 PM Robin H. Johnson <robbat2@gentoo.org> wrote: > > -- 8< -- > (snip patch, please add my DCO signed-off-by) > Tested-by: Robin H. Johnson <robbat2@gentoo.org> > Signed-off-by: Robin H. Johnson <robbat2@gentoo.org> > > -- 8< -- > > > > I'm a bit reluctant to follow up with a proper patch because I can't > > digest the t5572-submodule-pull.sh tests. And we definitely need to > > add a test case about --quiet to make sure it won't happen again. > Find testcase attached. Please submit in a series with your patch Clever. I was stuck thinking about actually pulling things. But yeah "echo --quiet" does the job just as well. Making patches (and maybe trying to fix that pull --rebase --quiet thing as well) -- Duy ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: regression AGAIN in output of git-pull --rebase --recurse-submodules=yes --quiet 2019-04-12 7:08 ` Robin H. Johnson 2019-04-12 9:25 ` Duy Nguyen @ 2019-04-15 14:40 ` Johannes Schindelin 1 sibling, 0 replies; 12+ messages in thread From: Johannes Schindelin @ 2019-04-15 14:40 UTC (permalink / raw) To: Robin H. Johnson; +Cc: Duy Nguyen, Git Mailing List, Prathamesh Chavan Hi Robin, On Fri, 12 Apr 2019, Robin H. Johnson wrote: > Looking at git-rebase--preserve-merges.sh for this message, I think that > should be a separate patch to make it respect --quiet. Please note that `git rebase --preserve-merges` will be deprecated as of the next Git version (see https://github.com/gitgitgadget/git/pull/158 for details). So I don't think it is worth the bother to fix that mode with respect to --quiet. Ciao, Johannes ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <CAODn77oL6sj5zvxgPGw=4TNqmnSeBq4=j2r2nx_51YHooECo7w@mail.gmail.com>]
* Re: regression AGAIN in output of git-pull --rebase --recurse-submodules=yes --quiet [not found] ` <CAODn77oL6sj5zvxgPGw=4TNqmnSeBq4=j2r2nx_51YHooECo7w@mail.gmail.com> @ 2019-04-16 7:48 ` Duy Nguyen 0 siblings, 0 replies; 12+ messages in thread From: Duy Nguyen @ 2019-04-16 7:48 UTC (permalink / raw) To: Paul Morelle Cc: Robin H. Johnson, Git Mailing List, Prathamesh Chavan, Stefan Beller On Tue, Apr 16, 2019 at 1:31 PM Paul Morelle <paul.morelle@gmail.com> wrote: >> The problem here is the option parser of this command would try to >> parse all options, so it considers both --quiet the same thing and are >> to tell "submodule--foreach" to be quiet, the second --quiet is not >> part of the "git pull" command anymore. >> >> So the fix would be to pass "--" to stop option parsing. >> submodule--helper should not parse options it does not understand >> anyway. Something like this should work. > > > My expectation as a user (and probably Robin's too) would be that `git submodule foreach` stops parsing arguments at `--` or at the first not-recognized argument, whichever is encountered first. The rest of the arguments would then be considered as the command. I don't think I change any visible behavior though (or at least trying not to). There are two command line parsers, the "front" one is in git-submodule.sh and should do what you describe (or whatever the current behavior is) and there's an internal one for "git submodule--helper" which is more like internal API than anything. The change here is to stop the internal parser from accidentally interpret options that belong to the foreach's command. The "--" and first non-recognized argument should be handled correctly by the front parser. The exact behavior of this front parser, I can't tell (I'm nowhere near expert level of submodules) but yeah it should stop at either `--` or the first non-option argument (e.g. something that does not start with '-'). An argument that looks like an option (i.e. starts with '-') but not recognized should result in an error. This is pretty much standard behavior for all other commands, but I have not tested this with git-submodule.sh. > This would slightly break the retrocompatibility, but would also avoid similar bugs in the future. -- Duy ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] submodule foreach: fix "<command> --quiet" not being respected 2019-04-10 6:41 ` regression AGAIN in output of git-pull --rebase --recurse-submodules=yes --quiet Robin H. Johnson 2019-04-10 11:18 ` Duy Nguyen @ 2019-04-12 10:08 ` Nguyễn Thái Ngọc Duy 2019-04-12 17:22 ` Robin H. Johnson 1 sibling, 1 reply; 12+ messages in thread From: Nguyễn Thái Ngọc Duy @ 2019-04-12 10:08 UTC (permalink / raw) To: git; +Cc: robbat2, Junio C Hamano, Nguyễn Thái Ngọc Duy Robin reported that git submodule foreach --quiet git pull --quiet origin is not really quiet anymore [1]. "git pull" behaves as if --quiet is not given. This happens because parseopt in submodule--helper will try to parse both --quiet options as if they are foreach's options, not git-pull's. The parsed options are removed from the command line. So when we do pull later, we execute just this git pull origin When calling submodule helper, adding "--" in front of "git pull" will stop parseopt for parsing options that do not really belong to submodule--helper foreach. PARSE_OPT_KEEP_UNKNOWN is removed as a safety measure. parseopt should never see unknown options or something has gone wrong. There are also a couple usage string update while I'm looking at them. While at it, I also add "--" to other subcommands that pass "$@" to submodule--helper. "$@" in these cases are paths and less likely to be --something-like-this. But the point still stands, git-submodule has parsed and classified what are options, what are paths. submodule--helper should never consider paths passed by git-submodule to be options even if they look like one. The test case is also contributed by Robin. [1] it should be quiet before fc1b9243cd (submodule: port submodule subcommand 'foreach' from shell to C, 2018-05-10) because parseopt can't accidentally eat options then. Reported-by: Robin H. Johnson <robbat2@gentoo.org> Tested-by: Robin H. Johnson <robbat2@gentoo.org> Signed-off-by: Robin H. Johnson <robbat2@gentoo.org> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> --- I'm not trying to fix "git pull --rebase --quiet" (or "git rebase --quiet" in general) in the end, since that looks like a whole other can of worms. Not only git-rebase--preserve-merges.sh needs to respect --quiet (but which case? I don't have enough experience to say) but sequencer.c may need to be scanned too. builtin/submodule--helper.c | 8 ++++---- git-submodule.sh | 11 ++++++----- t/t7407-submodule-foreach.sh | 10 ++++++++++ 3 files changed, 20 insertions(+), 9 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 6bcc4f1bd7..59570b5e87 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -566,12 +566,12 @@ static int module_foreach(int argc, const char **argv, const char *prefix) }; const char *const git_submodule_helper_usage[] = { - N_("git submodule--helper foreach [--quiet] [--recursive] <command>"), + N_("git submodule--helper foreach [--quiet] [--recursive] [--] <command>"), NULL }; argc = parse_options(argc, argv, prefix, module_foreach_options, - git_submodule_helper_usage, PARSE_OPT_KEEP_UNKNOWN); + git_submodule_helper_usage, 0); if (module_list_compute(0, NULL, prefix, &pathspec, &list) < 0) return 1; @@ -709,7 +709,7 @@ static int module_init(int argc, const char **argv, const char *prefix) }; const char *const git_submodule_helper_usage[] = { - N_("git submodule--helper init [<path>]"), + N_("git submodule--helper init [<options>] [<path>]"), NULL }; @@ -2096,7 +2096,7 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix) }; const char *const git_submodule_helper_usage[] = { - N_("git submodule--helper embed-git-dir [<path>...]"), + N_("git submodule--helper asorb-git-dirs [<options>] [<path>...]"), NULL }; diff --git a/git-submodule.sh b/git-submodule.sh index 2c0fb6d723..d33f5d8bb4 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -346,7 +346,7 @@ cmd_foreach() shift done - git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper foreach ${GIT_QUIET:+--quiet} ${recursive:+--recursive} "$@" + git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper foreach ${GIT_QUIET:+--quiet} ${recursive:+--recursive} -- "$@" } # @@ -377,7 +377,7 @@ cmd_init() shift done - git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper init ${GIT_QUIET:+--quiet} "$@" + git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper init ${GIT_QUIET:+--quiet} -- "$@" } # @@ -413,7 +413,7 @@ cmd_deinit() shift done - git ${wt_prefix:+-C "$wt_prefix"} submodule--helper deinit ${GIT_QUIET:+--quiet} ${prefix:+--prefix "$prefix"} ${force:+--force} ${deinit_all:+--all} "$@" + git ${wt_prefix:+-C "$wt_prefix"} submodule--helper deinit ${GIT_QUIET:+--quiet} ${prefix:+--prefix "$prefix"} ${force:+--force} ${deinit_all:+--all} -- "$@" } is_tip_reachable () ( @@ -542,6 +542,7 @@ cmd_update() ${depth:+--depth "$depth"} \ $recommend_shallow \ $jobs \ + -- \ "$@" || echo "#unmatched" $? } | { err= @@ -934,7 +935,7 @@ cmd_status() shift done - git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper status ${GIT_QUIET:+--quiet} ${cached:+--cached} ${recursive:+--recursive} "$@" + git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper status ${GIT_QUIET:+--quiet} ${cached:+--cached} ${recursive:+--recursive} -- "$@" } # # Sync remote urls for submodules @@ -967,7 +968,7 @@ cmd_sync() esac done - git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper sync ${GIT_QUIET:+--quiet} ${recursive:+--recursive} "$@" + git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper sync ${GIT_QUIET:+--quiet} ${recursive:+--recursive} -- "$@" } cmd_absorbgitdirs() diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh index 77729ac4aa..706ae762e0 100755 --- a/t/t7407-submodule-foreach.sh +++ b/t/t7407-submodule-foreach.sh @@ -411,4 +411,14 @@ test_expect_success 'multi-argument command passed to foreach is not shell-evalu test_cmp expected actual ' +test_expect_success 'option-like arguments passed to foreach commands are not lost' ' + ( + cd super && + git submodule foreach "echo be --quiet" > ../expected && + git submodule foreach echo be --quiet > ../actual + ) && + grep -sq -e "--quiet" expected && + test_cmp expected actual +' + test_done -- 2.21.0.682.g30d2204636 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] submodule foreach: fix "<command> --quiet" not being respected 2019-04-12 10:08 ` [PATCH] submodule foreach: fix "<command> --quiet" not being respected Nguyễn Thái Ngọc Duy @ 2019-04-12 17:22 ` Robin H. Johnson 2019-04-15 2:59 ` Junio C Hamano 0 siblings, 1 reply; 12+ messages in thread From: Robin H. Johnson @ 2019-04-12 17:22 UTC (permalink / raw) To: Nguyễn Thái Ngọc Duy; +Cc: git, robbat2, Junio C Hamano [-- Attachment #1: Type: text/plain, Size: 638 bytes --] On Fri, Apr 12, 2019 at 05:08:19PM +0700, Nguyễn Thái Ngọc Duy wrote: > @@ -2096,7 +2096,7 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix) > }; > > const char *const git_submodule_helper_usage[] = { > - N_("git submodule--helper embed-git-dir [<path>...]"), > + N_("git submodule--helper asorb-git-dirs [<options>] [<path>...]"), Nit typo here: s/asorb/absorb/ -- Robin Hugh Johnson Gentoo Linux: Dev, Infra Lead, Foundation Treasurer E-Mail : robbat2@gentoo.org GnuPG FP : 11ACBA4F 4778E3F6 E4EDF38E B27B944E 34884E85 GnuPG FP : 7D0B3CEB E9B85B1F 825BCECF EE05E6F6 A48F6136 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 1113 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] submodule foreach: fix "<command> --quiet" not being respected 2019-04-12 17:22 ` Robin H. Johnson @ 2019-04-15 2:59 ` Junio C Hamano 0 siblings, 0 replies; 12+ messages in thread From: Junio C Hamano @ 2019-04-15 2:59 UTC (permalink / raw) To: Robin H. Johnson; +Cc: Nguyễn Thái Ngọc Duy, git "Robin H. Johnson" <robbat2@gentoo.org> writes: > On Fri, Apr 12, 2019 at 05:08:19PM +0700, Nguyễn Thái Ngọc Duy wrote: >> @@ -2096,7 +2096,7 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix) >> }; >> >> const char *const git_submodule_helper_usage[] = { >> - N_("git submodule--helper embed-git-dir [<path>...]"), >> + N_("git submodule--helper asorb-git-dirs [<options>] [<path>...]"), > Nit typo here: s/asorb/absorb/ Will locally tweak while queueing. Thanks, both. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-04-16 7:48 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-01-20 5:57 regression in output of git-pull --rebase --recurse-submodules=yes --quiet Robin H. Johnson 2018-01-25 19:08 ` [PATCH] builtin/pull: respect verbosity settings in submodules Stefan Beller 2018-01-25 19:18 ` Junio C Hamano 2019-04-10 6:41 ` regression AGAIN in output of git-pull --rebase --recurse-submodules=yes --quiet Robin H. Johnson 2019-04-10 11:18 ` Duy Nguyen 2019-04-12 7:08 ` Robin H. Johnson 2019-04-12 9:25 ` Duy Nguyen 2019-04-15 14:40 ` Johannes Schindelin [not found] ` <CAODn77oL6sj5zvxgPGw=4TNqmnSeBq4=j2r2nx_51YHooECo7w@mail.gmail.com> 2019-04-16 7:48 ` Duy Nguyen 2019-04-12 10:08 ` [PATCH] submodule foreach: fix "<command> --quiet" not being respected Nguyễn Thái Ngọc Duy 2019-04-12 17:22 ` Robin H. Johnson 2019-04-15 2:59 ` Junio C Hamano
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).