git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 0/1] commit-graph: drop top-level --[no-]progress

From
Taylor Blau <me@ttaylorr.com>
Date
Sep 20, 2021, 21:39 UTC
Message-ID
<YUj/h3xucy4JR7B1@nand.local>
In-Reply-To
<xmqqr1dj9c0b.fsf@gitster.g>
On Mon, Sep 20, 2021 at 02:24:04PM -0700, Junio C Hamano wrote:
Show 10 quoted lines
> Taylor Blau <me@ttaylorr.com> writes:
>
> > An open question is whether the same should be done for the multi-pack-index
> > command, whose top-level support for `--[no-]progress` was released in v2.32.0
> > with 60ca94769c (builtin/multi-pack-index.c: split sub-commands, 2021-03-30).
>
> We do not mind too much about "breaking backward compatibility" by
> removing the mistaken "git multi-pack-index --progress cmd", I would
> say.  It's not like people would type it once every day and removing
> the "support" will break their finger-memory.

OK; if we don't mind then we could do something like the following on top. But if we're OK to remove the top-level `--progress` option from the commit-graph and multi-pack-index builtins at any time, then both patches become far less urgent.

Thanks, Taylor

--- >8 ---
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>
---
 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
Previous: Junio C HamanoNext: Ævar Arnfjörð Bjarmason
Message 6 of 10 in “commit-graph: drop top-level --[no-]progress”
  1. 0/1 commit-graph: drop top-level --[no-]progressTaylor Blau, Sep 18, 2021
  2. 1/1 builtin/commit-graph.c: don't accept common --[no-]progressTaylor Blau, Sep 18, 2021
  3. Derrick StoleeSep 20, 2021
  4. Taylor BlauSep 20, 2021
  5. Junio C HamanoSep 20, 2021
  6. Taylor BlauSep 20, 2021
  7. Ævar Arnfjörð BjarmasonSep 21, 2021
  8. Taylor BlauSep 21, 2021
  9. Junio C HamanoSep 22, 2021
  10. Jeff KingSep 21, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.