From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Sven Strickroth <email@cs-ware.de>
Cc: git <git@vger.kernel.org>, Junio C Hamano <gitster@pobox.com>,
"Robin H. Johnson" <robbat2@gentoo.org>
Subject: Re: [PATCH] Don't pass -v to submodule command
Date: Wed, 30 Nov 2022 20:17:23 +0100 [thread overview]
Message-ID: <221130.868rjsi6bn.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <cad05012-7bf9-5975-3add-253b11c7bcc8@cs-ware.de>
On Wed, Nov 30 2022, Sven Strickroth wrote:
> "git pull -v --recurse-submodules" propagates the "-v" to the submdoule
> command which does not support "-v".
>
> Commit a56771a668dd4963675914bc5da0e1e015952dae introduced this
> regression.
We refer to commits in commit messages like this: a56771a668d
(builtin/pull: respect verbosity settings in submodules, 2018-01-25);
Which also shows that this regression is quite old.
> Signed-off-by: Sven Strickroth <email@cs-ware.de>
> ---
> builtin/pull.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/builtin/pull.c b/builtin/pull.c
> index 1ab4de0005..b67320fa5f 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -256,7 +256,7 @@ static struct option pull_options[] = {
> /**
> * Pushes "-q" or "-v" switches into arr to match the opt_verbosity level.
> */
> -static void argv_push_verbosity(struct strvec *arr)
> +static void argv_push_verbosity(struct strvec *arr, int include_v)
> {
> int verbosity;
>
It looks like you're getting somewhere with this, but you never use this
"include_v", so the bug is still there. We just have the scaffolding
now.
Did you forget to add that part to this commit?
In any case, that serves as a comment on the other thing this patch
really needs: tests, please add some.
I can reproduce this locally by just running the command you noted in a
repo with submodules, so presumably we can use some of the existing
submodule tests, which have already set up such a repo.
> @@ -520,7 +520,7 @@ static int run_fetch(const char *repo, const char **refspecs)
> strvec_pushl(&cmd.args, "fetch", "--update-head-ok", NULL);
>
> /* Shared options */
> - argv_push_verbosity(&cmd.args);
> + argv_push_verbosity(&cmd.args, 1);
> if (opt_progress)
> strvec_push(&cmd.args, opt_progress);
>
> @@ -629,7 +629,7 @@ static int rebase_submodules(void)
> cp.no_stdin = 1;
> strvec_pushl(&cp.args, "submodule", "update",
> "--recursive", "--rebase", NULL);
> - argv_push_verbosity(&cp.args);
> + argv_push_verbosity(&cp.args, 0);
>
> return run_command(&cp);
> }
> @@ -642,7 +642,7 @@ static int update_submodules(void)
> cp.no_stdin = 1;
> strvec_pushl(&cp.args, "submodule", "update",
> "--recursive", "--checkout", NULL);
> - argv_push_verbosity(&cp.args);
> + argv_push_verbosity(&cp.args, 0);
>
> return run_command(&cp);
> }
> @@ -657,7 +657,7 @@ static int run_merge(void)
> strvec_pushl(&cmd.args, "merge", NULL);
>
> /* Shared options */
> - argv_push_verbosity(&cmd.args);
> + argv_push_verbosity(&cmd.args, 1);
> if (opt_progress)
> strvec_push(&cmd.args, opt_progress);
>
> @@ -881,7 +881,7 @@ static int run_rebase(const struct object_id *newbase,
> strvec_push(&cmd.args, "rebase");
>
> /* Shared options */
> - argv_push_verbosity(&cmd.args);
> + argv_push_verbosity(&cmd.args, 1);
>
> /* Options passed to git-rebase */
> if (opt_rebase == REBASE_MERGES)
I think the right longer term fix here is to simply make "git submodule"
support "-v" and "--verbose".
Which, as a funny implementation detail we'd support if we called "git
submodule--helper update", as its OPT__QUIET() adds both variants, but
the git-submodule.sh doesn't support it.
OTOH we've never supported it in "git submodule", so maybe we should
just make the C version stricter, I dunno...
In any case, this is a good fix for now, let's just stop passing the
unsupported flag.
next prev parent reply other threads:[~2022-11-30 19:35 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-11-24 12:47 git pull --verbose with submodules ends in error message Fink, Mike
2022-11-25 15:56 ` Sven Strickroth
2022-11-30 18:30 ` [PATCH] Don't pass -v to submodule command Sven Strickroth
2022-11-30 19:17 ` Ævar Arnfjörð Bjarmason [this message]
2022-12-01 8:32 ` Sven Strickroth
2022-12-01 8:34 ` [PATCH v2] " Sven Strickroth
2022-12-02 0:24 ` [PATCH] " Junio C Hamano
2022-12-10 13:06 ` [PATCH] submodule: Accept -v for update command Sven Strickroth
2022-12-18 1:25 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=221130.868rjsi6bn.gmgdl@evledraar.gmail.com \
--to=avarab@gmail.com \
--cc=email@cs-ware.de \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=robbat2@gentoo.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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 NNTP newsgroup(s).