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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.