From: Junio C Hamano <gitster@pobox.com>
To: Taylor Blau <me@ttaylorr.com>
Cc: git@vger.kernel.org, peff@peff.net, szeder.dev@gmail.com,
avarab@gmail.com, dstolee@microsoft.com
Subject: Re: [PATCH 0/1] commit-graph: drop top-level --[no-]progress
Date: Wed, 22 Sep 2021 09:22:53 -0700 [thread overview]
Message-ID: <xmqqa6k48tr6.fsf@gitster.g> (raw)
In-Reply-To: <YUj/h3xucy4JR7B1@nand.local> (Taylor Blau's message of "Mon, 20 Sep 2021 17:39:19 -0400")
Taylor Blau <me@ttaylorr.com> writes:
> Subject: [PATCH] builtin/multi-pack-index.c: disable top-level --[no-]progress
>
> In a similar spirit as the previous patch, let sub-commands which
> support showing or hiding a progress meter handle parsing the
> `--progress` or `--no-progress` option, but do not expose it as an
> option to the top-level `multi-pack-index` builtin.
>
> Signed-off-by: Taylor Blau <me@ttaylorr.com>
> ---
Hmph, so a few older releases accepts --progress at a wrong point on
the command line but going forward we will correct it?
OK. Then I buy the mention of "as the _previous_ patch" above ;-)
Thanks.
> Documentation/git-multi-pack-index.txt | 6 ++---
> builtin/multi-pack-index.c | 31 +++++++++++++++++++++-----
> t/t5319-multi-pack-index.sh | 12 +++++-----
> 3 files changed, 35 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/git-multi-pack-index.txt b/Documentation/git-multi-pack-index.txt
> index ffd601bc17..5ba4bd5166 100644
> --- a/Documentation/git-multi-pack-index.txt
> +++ b/Documentation/git-multi-pack-index.txt
> @@ -9,8 +9,7 @@ git-multi-pack-index - Write and verify multi-pack-indexes
> SYNOPSIS
> --------
> [verse]
> -'git multi-pack-index' [--object-dir=<dir>] [--[no-]progress]
> - [--preferred-pack=<pack>] <subcommand>
> +'git multi-pack-index' [--object-dir=<dir>] <sub-command>
>
> DESCRIPTION
> -----------
> @@ -26,7 +25,8 @@ OPTIONS
>
> --[no-]progress::
> Turn progress on/off explicitly. If neither is specified, progress is
> - shown if standard error is connected to a terminal.
> + shown if standard error is connected to a terminal. Supported by
> + sub-commands `write`, `verify`, `expire`, and `repack.
>
> The following subcommands are available:
>
> diff --git a/builtin/multi-pack-index.c b/builtin/multi-pack-index.c
> index 649aa5f9ab..5f11a3067d 100644
> --- a/builtin/multi-pack-index.c
> +++ b/builtin/multi-pack-index.c
> @@ -52,7 +52,6 @@ static struct opts_multi_pack_index {
> static struct option common_opts[] = {
> OPT_FILENAME(0, "object-dir", &opts.object_dir,
> N_("object directory containing set of packfile and pack-index pairs")),
> - OPT_BIT(0, "progress", &opts.flags, N_("force progress reporting"), MIDX_PROGRESS),
> OPT_END(),
> };
>
> @@ -68,6 +67,8 @@ static int cmd_multi_pack_index_write(int argc, const char **argv)
> OPT_STRING(0, "preferred-pack", &opts.preferred_pack,
> N_("preferred-pack"),
> N_("pack for reuse when computing a multi-pack bitmap")),
> + OPT_BIT(0, "progress", &opts.flags,
> + N_("force progress reporting"), MIDX_PROGRESS),
> OPT_END(),
> };
>
> @@ -75,6 +76,8 @@ static int cmd_multi_pack_index_write(int argc, const char **argv)
>
> trace2_cmd_mode(argv[0]);
>
> + if (isatty(2))
> + opts.flags |= MIDX_PROGRESS;
> argc = parse_options(argc, argv, NULL,
> options, builtin_multi_pack_index_write_usage,
> PARSE_OPT_KEEP_UNKNOWN);
> @@ -90,10 +93,18 @@ static int cmd_multi_pack_index_write(int argc, const char **argv)
>
> static int cmd_multi_pack_index_verify(int argc, const char **argv)
> {
> - struct option *options = common_opts;
> + struct option *options;
> + static struct option builtin_multi_pack_index_verify_options[] = {
> + OPT_BIT(0, "progress", &opts.flags,
> + N_("force progress reporting"), MIDX_PROGRESS),
> + OPT_END(),
> + };
> + options = add_common_options(builtin_multi_pack_index_verify_options);
>
> trace2_cmd_mode(argv[0]);
>
> + if (isatty(2))
> + opts.flags |= MIDX_PROGRESS;
> argc = parse_options(argc, argv, NULL,
> options, builtin_multi_pack_index_verify_usage,
> PARSE_OPT_KEEP_UNKNOWN);
> @@ -106,10 +117,18 @@ static int cmd_multi_pack_index_verify(int argc, const char **argv)
>
> static int cmd_multi_pack_index_expire(int argc, const char **argv)
> {
> - struct option *options = common_opts;
> + struct option *options;
> + static struct option builtin_multi_pack_index_expire_options[] = {
> + OPT_BIT(0, "progress", &opts.flags,
> + N_("force progress reporting"), MIDX_PROGRESS),
> + OPT_END(),
> + };
> + options = add_common_options(builtin_multi_pack_index_expire_options);
>
> trace2_cmd_mode(argv[0]);
>
> + if (isatty(2))
> + opts.flags |= MIDX_PROGRESS;
> argc = parse_options(argc, argv, NULL,
> options, builtin_multi_pack_index_expire_usage,
> PARSE_OPT_KEEP_UNKNOWN);
> @@ -126,6 +145,8 @@ static int cmd_multi_pack_index_repack(int argc, const char **argv)
> static struct option builtin_multi_pack_index_repack_options[] = {
> OPT_MAGNITUDE(0, "batch-size", &opts.batch_size,
> N_("during repack, collect pack-files of smaller size into a batch that is larger than this size")),
> + OPT_BIT(0, "progress", &opts.flags,
> + N_("force progress reporting"), MIDX_PROGRESS),
> OPT_END(),
> };
>
> @@ -133,6 +154,8 @@ static int cmd_multi_pack_index_repack(int argc, const char **argv)
>
> trace2_cmd_mode(argv[0]);
>
> + if (isatty(2))
> + opts.flags |= MIDX_PROGRESS;
> argc = parse_options(argc, argv, NULL,
> options,
> builtin_multi_pack_index_repack_usage,
> @@ -154,8 +177,6 @@ int cmd_multi_pack_index(int argc, const char **argv,
>
> git_config(git_default_config, NULL);
>
> - if (isatty(2))
> - opts.flags |= MIDX_PROGRESS;
> argc = parse_options(argc, argv, prefix,
> builtin_multi_pack_index_options,
> builtin_multi_pack_index_usage,
> diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh
> index 3d4d9f10c3..86b7de2281 100755
> --- a/t/t5319-multi-pack-index.sh
> +++ b/t/t5319-multi-pack-index.sh
> @@ -174,12 +174,12 @@ test_expect_success 'write progress off for redirected stderr' '
> '
>
> test_expect_success 'write force progress on for stderr' '
> - GIT_PROGRESS_DELAY=0 git multi-pack-index --object-dir=$objdir --progress write 2>err &&
> + GIT_PROGRESS_DELAY=0 git multi-pack-index --object-dir=$objdir write --progress 2>err &&
> test_file_not_empty err
> '
>
> test_expect_success 'write with the --no-progress option' '
> - GIT_PROGRESS_DELAY=0 git multi-pack-index --object-dir=$objdir --no-progress write 2>err &&
> + GIT_PROGRESS_DELAY=0 git multi-pack-index --object-dir=$objdir write --no-progress 2>err &&
> test_line_count = 0 err
> '
>
> @@ -429,12 +429,12 @@ test_expect_success 'repack progress off for redirected stderr' '
> '
>
> test_expect_success 'repack force progress on for stderr' '
> - GIT_PROGRESS_DELAY=0 git multi-pack-index --object-dir=$objdir --progress repack 2>err &&
> + GIT_PROGRESS_DELAY=0 git multi-pack-index --object-dir=$objdir repack --progress 2>err &&
> test_file_not_empty err
> '
>
> test_expect_success 'repack with the --no-progress option' '
> - GIT_PROGRESS_DELAY=0 git multi-pack-index --object-dir=$objdir --no-progress repack 2>err &&
> + GIT_PROGRESS_DELAY=0 git multi-pack-index --object-dir=$objdir repack --no-progress 2>err &&
> test_line_count = 0 err
> '
>
> @@ -618,7 +618,7 @@ test_expect_success 'expire progress off for redirected stderr' '
> test_expect_success 'expire force progress on for stderr' '
> (
> cd dup &&
> - GIT_PROGRESS_DELAY=0 git multi-pack-index --progress expire 2>err &&
> + GIT_PROGRESS_DELAY=0 git multi-pack-index expire --progress 2>err &&
> test_file_not_empty err
> )
> '
> @@ -626,7 +626,7 @@ test_expect_success 'expire force progress on for stderr' '
> test_expect_success 'expire with the --no-progress option' '
> (
> cd dup &&
> - GIT_PROGRESS_DELAY=0 git multi-pack-index --no-progress expire 2>err &&
> + GIT_PROGRESS_DELAY=0 git multi-pack-index expire --no-progress 2>err &&
> test_line_count = 0 err
> )
> '
> --
> 2.33.0.96.g73915697e6
next prev parent reply other threads:[~2021-09-22 16:22 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-18 16:02 [PATCH 0/1] commit-graph: drop top-level --[no-]progress Taylor Blau
2021-09-18 16:02 ` [PATCH 1/1] builtin/commit-graph.c: don't accept common --[no-]progress Taylor Blau
2021-09-20 12:46 ` Derrick Stolee
2021-09-20 15:02 ` Taylor Blau
2021-09-20 21:24 ` [PATCH 0/1] commit-graph: drop top-level --[no-]progress Junio C Hamano
2021-09-20 21:39 ` Taylor Blau
2021-09-21 18:19 ` Ævar Arnfjörð Bjarmason
2021-09-21 20:38 ` Taylor Blau
2021-09-22 16:22 ` Junio C Hamano [this message]
2021-09-21 3:55 ` Jeff King
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=xmqqa6k48tr6.fsf@gitster.g \
--to=gitster@pobox.com \
--cc=avarab@gmail.com \
--cc=dstolee@microsoft.com \
--cc=git@vger.kernel.org \
--cc=me@ttaylorr.com \
--cc=peff@peff.net \
--cc=szeder.dev@gmail.com \
/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).