git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* option -q not passed from "git commit" to "git gc --auto"
@ 2020-05-06  9:43 Pierrick Gaudry
  2020-05-06 17:28 ` Taylor Blau
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Pierrick Gaudry @ 2020-05-06  9:43 UTC (permalink / raw)
  To: git

Hello,

It seems that when "git commit" is run with the "-q" option, there are
still, from time to time, messages that get printed. With the French
locale the message is:
  Compression automatique du dépôt en tâche de fond pour optimiser les performances.
  Voir "git help gc" pour toute information sur le nettoyage manuel.

From what I could guess, this is due to the fact that "git commit" calls
"git gc --auto", but does not propagate the "-q" option if present.

A similar problem was present some time ago with "git fetch" and was
solved in the 2-line patch 6fceed3b . I guess that the same should be
done for "git commit".

Regards,
Pierrick

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: option -q not passed from "git commit" to "git gc --auto"
  2020-05-06  9:43 option -q not passed from "git commit" to "git gc --auto" Pierrick Gaudry
@ 2020-05-06 17:28 ` Taylor Blau
  2020-05-06 17:33   ` Taylor Blau
  2020-05-06 17:31 ` Junio C Hamano
  2020-05-06 20:18 ` [PATCH 0/2] Pass down "--quiet" to "gc --auto" Junio C Hamano
  2 siblings, 1 reply; 13+ messages in thread
From: Taylor Blau @ 2020-05-06 17:28 UTC (permalink / raw)
  To: Pierrick Gaudry; +Cc: git

On Wed, May 06, 2020 at 11:43:27AM +0200, Pierrick Gaudry wrote:
> Hello,
>
> It seems that when "git commit" is run with the "-q" option, there are
> still, from time to time, messages that get printed. With the French
> locale the message is:
>   Compression automatique du dépôt en tâche de fond pour optimiser les performances.
>   Voir "git help gc" pour toute information sur le nettoyage manuel.
>
> From what I could guess, this is due to the fact that "git commit" calls
> "git gc --auto", but does not propagate the "-q" option if present.
>
> A similar problem was present some time ago with "git fetch" and was
> solved in the 2-line patch 6fceed3b . I guess that the same should be
> done for "git commit".

Yes, I think so. A quick search through the list archive turns up [1],
which identifies and provides a patch for this issue. If I were
reviewing that patch today, I'd suggest the following:

  * break the change from a 'char **' to a 'struct argv_array' into a
    separate, preparatory patch.

  * adjust the commit message of the second commit (which will only pass
    '-q' to the 'git-gc' sub-process) to indicate that there may be
    other locations

  * fix those other locations that spawn 'git gc', if they exist, in a
    similar fashion

  * in each of the previous two steps, add tests in the appropriate
    files in 't' to demonstrate that '-q' propagation works as expected.

I'm happy to do any and all of this, if you want, but you are also
welcome to submit the patches yourself.

> Regards,
> Pierrick

Thanks,
Taylor

[1]: https://lore.kernel.org/git/20200506140138.650455-1-abhishekkumar8222@gmail.com/

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: option -q not passed from "git commit" to "git gc --auto"
  2020-05-06  9:43 option -q not passed from "git commit" to "git gc --auto" Pierrick Gaudry
  2020-05-06 17:28 ` Taylor Blau
@ 2020-05-06 17:31 ` Junio C Hamano
  2020-05-06 18:56   ` Re* " Junio C Hamano
  2020-05-06 20:18 ` [PATCH 0/2] Pass down "--quiet" to "gc --auto" Junio C Hamano
  2 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2020-05-06 17:31 UTC (permalink / raw)
  To: Pierrick Gaudry; +Cc: git

Pierrick Gaudry <pierrick.gaudry@loria.fr> writes:

> Hello,
>
> It seems that when "git commit" is run with the "-q" option, there are
> still, from time to time, messages that get printed. With the French
> locale the message is:
>   Compression automatique du dépôt en tâche de fond pour optimiser les performances.
>   Voir "git help gc" pour toute information sur le nettoyage manuel.
>
> From what I could guess, this is due to the fact that "git commit" calls
> "git gc --auto", but does not propagate the "-q" option if present.
>
> A similar problem was present some time ago with "git fetch" and was
> solved in the 2-line patch 6fceed3b . I guess that the same should be
> done for "git commit".

Hmph, once our minds are on this, we probably should see how
widespread the issues would be before just "fixing" commit.

$ git grep -e 'gc.*,.*--auto' \*.c
builtin/am.c:	const char *argv_gc_auto[] = {"gc", "--auto", NULL};
builtin/commit.c:	const char *argv_gc_auto[] = {"gc", "--auto", NULL};
builtin/fetch.c:		argv_array_pushl(&argv_gc_auto, "gc", "--auto", NULL);
builtin/merge.c:			const char *argv_gc_auto[] = { "gc", "--auto", NULL };
builtin/rebase.c:	const char *argv_gc_auto[] = { "gc", "--auto", NULL };
builtin/receive-pack.c:				"gc", "--auto", "--quiet", NULL,

It is quite clear that "git am --quiet" will ignore "--quiet" (there
is no room to pass an extra argument to "gc --auto").  "merge" and
"rebase" are in the same boat.

I'd rather not see a fix that mimicks 6fceed3b (fetch: silence
git-gc if --quiet is given, 2014-08-16).  Instead why not introduce
a helper "int run_auto_gc(int quiet)" and use it consistently when
we run the auto-gc?

An illustration to cover these (not even compile tested) to show
what I mean and help anybody get started.

 builtin/am.c     | 3 +--
 builtin/commit.c | 3 +--
 builtin/merge.c  | 3 +--
 builtin/rebase.c | 3 +--
 4 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index e3dfd93c25..955f91f076 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1691,7 +1691,6 @@ static int do_interactive(struct am_state *state)
  */
 static void am_run(struct am_state *state, int resume)
 {
-	const char *argv_gc_auto[] = {"gc", "--auto", NULL};
 	struct strbuf sb = STRBUF_INIT;
 
 	unlink(am_path(state, "dirtyindex"));
@@ -1796,7 +1795,7 @@ static void am_run(struct am_state *state, int resume)
 	if (!state->rebasing) {
 		am_destroy(state);
 		close_object_store(the_repository->objects);
-		run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+		run_gc_auto(state->quiet);
 	}
 }
 
diff --git a/builtin/commit.c b/builtin/commit.c
index a73de0a4c5..15755a52e1 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1494,7 +1494,6 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 
 int cmd_commit(int argc, const char **argv, const char *prefix)
 {
-	const char *argv_gc_auto[] = {"gc", "--auto", NULL};
 	static struct wt_status s;
 	static struct option builtin_commit_options[] = {
 		OPT__QUIET(&quiet, N_("suppress summary after successful commit")),
@@ -1703,7 +1702,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 	git_test_write_commit_graph_or_die();
 
 	repo_rerere(the_repository, 0);
-	run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+	run_gc_auto(quiet);
 	run_commit_hook(use_editor, get_index_file(), "post-commit", NULL);
 	if (amend && !no_post_rewrite) {
 		commit_post_rewrite(the_repository, current_head, &oid);
diff --git a/builtin/merge.c b/builtin/merge.c
index 923e32acf1..2361e3604a 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -450,7 +450,6 @@ static void finish(struct commit *head_commit,
 		if (verbosity >= 0 && !merge_msg.len)
 			printf(_("No merge message -- not updating HEAD\n"));
 		else {
-			const char *argv_gc_auto[] = { "gc", "--auto", NULL };
 			update_ref(reflog_message.buf, "HEAD", new_head, head,
 				   0, UPDATE_REFS_DIE_ON_ERR);
 			/*
@@ -458,7 +457,7 @@ static void finish(struct commit *head_commit,
 			 * user should see them.
 			 */
 			close_object_store(the_repository->objects);
-			run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+			run_gc_auto(verbosity < 0);
 		}
 	}
 	if (new_head && show_diffstat) {
diff --git a/builtin/rebase.c b/builtin/rebase.c
index fb1227de45..2edd2ea596 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -737,7 +737,6 @@ static int rebase_write_basic_state(struct rebase_options *opts)
 static int finish_rebase(struct rebase_options *opts)
 {
 	struct strbuf dir = STRBUF_INIT;
-	const char *argv_gc_auto[] = { "gc", "--auto", NULL };
 	int ret = 0;
 
 	delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
@@ -747,7 +746,7 @@ static int finish_rebase(struct rebase_options *opts)
 	 * We ignore errors in 'gc --auto', since the
 	 * user should see them.
 	 */
-	run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+	run_auto_gc(!(opts->flags & (REBASE_NO_QUIET|REBASE_VERBOSE)));
 	if (opts->type == REBASE_MERGE) {
 		struct replay_opts replay = REPLAY_OPTS_INIT;
 

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: option -q not passed from "git commit" to "git gc --auto"
  2020-05-06 17:28 ` Taylor Blau
@ 2020-05-06 17:33   ` Taylor Blau
  2020-05-06 17:38     ` Taylor Blau
  0 siblings, 1 reply; 13+ messages in thread
From: Taylor Blau @ 2020-05-06 17:33 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Pierrick Gaudry, git

On Wed, May 06, 2020 at 11:28:23AM -0600, Taylor Blau wrote:
> On Wed, May 06, 2020 at 11:43:27AM +0200, Pierrick Gaudry wrote:
> > Hello,
> >
> > It seems that when "git commit" is run with the "-q" option, there are
> > still, from time to time, messages that get printed. With the French
> > locale the message is:
> >   Compression automatique du dépôt en tâche de fond pour optimiser les performances.
> >   Voir "git help gc" pour toute information sur le nettoyage manuel.
> >
> > From what I could guess, this is due to the fact that "git commit" calls
> > "git gc --auto", but does not propagate the "-q" option if present.
> >
> > A similar problem was present some time ago with "git fetch" and was
> > solved in the 2-line patch 6fceed3b . I guess that the same should be
> > done for "git commit".
>
> Yes, I think so. A quick search through the list archive turns up [1],
> which identifies and provides a patch for this issue. If I were
> reviewing that patch today, I'd suggest the following:
>
>   * break the change from a 'char **' to a 'struct argv_array' into a
>     separate, preparatory patch.
>
>   * adjust the commit message of the second commit (which will only pass
>     '-q' to the 'git-gc' sub-process) to indicate that there may be
>     other locations
>
>   * fix those other locations that spawn 'git gc', if they exist, in a
>     similar fashion
>
>   * in each of the previous two steps, add tests in the appropriate
>     files in 't' to demonstrate that '-q' propagation works as expected.

Junio identified a much better way to do this in the email below this
one. Since it appears that there are multiple places that ignore
'--quiet' when running 'git gc' as a sub-process, a helper function is
certainly you want to be using.

> I'm happy to do any and all of this, if you want, but you are also
> welcome to submit the patches yourself.
>
> > Regards,
> > Pierrick
>
> Thanks,
> Taylor
>
> [1]: https://lore.kernel.org/git/20200506140138.650455-1-abhishekkumar8222@gmail.com/
Thanks,
Taylor

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: option -q not passed from "git commit" to "git gc --auto"
  2020-05-06 17:33   ` Taylor Blau
@ 2020-05-06 17:38     ` Taylor Blau
  0 siblings, 0 replies; 13+ messages in thread
From: Taylor Blau @ 2020-05-06 17:38 UTC (permalink / raw)
  To: Taylor Blau; +Cc: Pierrick Gaudry, git

On Wed, May 06, 2020 at 11:33:56AM -0600, Taylor Blau wrote:
> On Wed, May 06, 2020 at 11:28:23AM -0600, Taylor Blau wrote:
> > On Wed, May 06, 2020 at 11:43:27AM +0200, Pierrick Gaudry wrote:
> > > Hello,
> > >
> > > It seems that when "git commit" is run with the "-q" option, there are
> > > still, from time to time, messages that get printed. With the French
> > > locale the message is:
> > >   Compression automatique du dépôt en tâche de fond pour optimiser les performances.
> > >   Voir "git help gc" pour toute information sur le nettoyage manuel.
> > >
> > > From what I could guess, this is due to the fact that "git commit" calls
> > > "git gc --auto", but does not propagate the "-q" option if present.
> > >
> > > A similar problem was present some time ago with "git fetch" and was
> > > solved in the 2-line patch 6fceed3b . I guess that the same should be
> > > done for "git commit".
> >
> > Yes, I think so. A quick search through the list archive turns up [1],
> > which identifies and provides a patch for this issue. If I were
> > reviewing that patch today, I'd suggest the following:

Oh, [1] is an email from today. Clearly I am not finished reading my
inbox yet. Sorry about that.

> >   * break the change from a 'char **' to a 'struct argv_array' into a
> >     separate, preparatory patch.
> >
> >   * adjust the commit message of the second commit (which will only pass
> >     '-q' to the 'git-gc' sub-process) to indicate that there may be
> >     other locations
> >
> >   * fix those other locations that spawn 'git gc', if they exist, in a
> >     similar fashion
> >
> >   * in each of the previous two steps, add tests in the appropriate
> >     files in 't' to demonstrate that '-q' propagation works as expected.
>
> Junio identified a much better way to do this in the email below this
> one. Since it appears that there are multiple places that ignore
> '--quiet' when running 'git gc' as a sub-process, a helper function is
> certainly you want to be using.
>
> > I'm happy to do any and all of this, if you want, but you are also
> > welcome to submit the patches yourself.
> >
> > > Regards,
> > > Pierrick
> >
> > Thanks,
> > Taylor
> >
> > [1]: https://lore.kernel.org/git/20200506140138.650455-1-abhishekkumar8222@gmail.com/
> Thanks,
> Taylor
Thanks,
Taylor

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re* option -q not passed from "git commit" to "git gc --auto"
  2020-05-06 17:31 ` Junio C Hamano
@ 2020-05-06 18:56   ` Junio C Hamano
  2020-05-06 19:03     ` [PATCH 2/2] auto-gc: pass --quiet down from am, commit, merge and rebase Junio C Hamano
  0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2020-05-06 18:56 UTC (permalink / raw)
  To: Pierrick Gaudry; +Cc: git, Johannes Schindelin

Junio C Hamano <gitster@pobox.com> writes:

> An illustration to cover these (not even compile tested) to show
> what I mean and help anybody get started.


And this would be a good first step in the conversion.

-- >8 --
Subject: [PATCH] run_auto_gc(): extract a reusable helper

Earlier we taught "git fetch --quiet" to pass the "--quiet" option
down to "gc --auto".  This, however, is not limited to "fetch":

    $ git grep -e 'gc.*--auto' \*.c

finds hits in "am", "commit", "merge", and "rebase".

As a preparatory step, let's introduce a helper function
run_auto_gc(), that the caller can pass a boolean "quiet".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/fetch.c | 10 ++--------
 run-command.c   | 13 +++++++++++++
 run-command.h   |  5 +++++
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index bf6bab80fa..3e580b9559 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1759,7 +1759,6 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	struct remote *remote = NULL;
 	int result = 0;
 	int prune_tags_ok = 1;
-	struct argv_array argv_gc_auto = ARGV_ARRAY_INIT;
 
 	packet_trace_identity("fetch");
 
@@ -1886,13 +1885,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 
 	close_object_store(the_repository->objects);
 
-	if (enable_auto_gc) {
-		argv_array_pushl(&argv_gc_auto, "gc", "--auto", NULL);
-		if (verbosity < 0)
-			argv_array_push(&argv_gc_auto, "--quiet");
-		run_command_v_opt(argv_gc_auto.argv, RUN_GIT_CMD);
-		argv_array_clear(&argv_gc_auto);
-	}
+	if (enable_auto_gc)
+		run_auto_gc(verbosity < 0);
 
 	return result;
 }
diff --git a/run-command.c b/run-command.c
index f5e1149f9b..2771eb936f 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1864,3 +1864,16 @@ int run_processes_parallel_tr2(int n, get_next_task_fn get_next_task,
 
 	return result;
 }
+
+int run_auto_gc(int quiet)
+{
+	struct argv_array argv_gc_auto = ARGV_ARRAY_INIT;
+	int status;
+
+	argv_array_pushl(&argv_gc_auto, "gc", "--auto", NULL);
+	if (quiet)
+		argv_array_push(&argv_gc_auto, "--quiet");
+	status = run_command_v_opt(argv_gc_auto.argv, RUN_GIT_CMD);
+	argv_array_clear(&argv_gc_auto);
+	return status;
+}
diff --git a/run-command.h b/run-command.h
index 0f3cc73ab6..191dfcdafe 100644
--- a/run-command.h
+++ b/run-command.h
@@ -218,6 +218,11 @@ LAST_ARG_MUST_BE_NULL
 int run_hook_le(const char *const *env, const char *name, ...);
 int run_hook_ve(const char *const *env, const char *name, va_list args);
 
+/*
+ * Trigger an auto-gc
+ */
+int run_auto_gc(int quiet);
+
 #define RUN_COMMAND_NO_STDIN 1
 #define RUN_GIT_CMD	     2	/*If this is to be git sub-command */
 #define RUN_COMMAND_STDOUT_TO_STDERR 4

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] auto-gc: pass --quiet down from am, commit, merge and rebase
  2020-05-06 18:56   ` Re* " Junio C Hamano
@ 2020-05-06 19:03     ` Junio C Hamano
  2020-05-06 19:27       ` Junio C Hamano
  0 siblings, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2020-05-06 19:03 UTC (permalink / raw)
  To: git

A handful of commands take the --quiet option for their own
operation, but they forget to pass the option down when they
invoke "git gc --auto" internally.

Teach these commands to do so using the run_gc_auto() helper we
added earlier.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * And this is the earlier one, wrapped in a proper patch submission
   format, to conclude the two-patch series.

 builtin/am.c     | 3 +--
 builtin/commit.c | 3 +--
 builtin/merge.c  | 3 +--
 builtin/rebase.c | 3 +--
 4 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index e3dfd93c25..955f91f076 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1691,7 +1691,6 @@ static int do_interactive(struct am_state *state)
  */
 static void am_run(struct am_state *state, int resume)
 {
-	const char *argv_gc_auto[] = {"gc", "--auto", NULL};
 	struct strbuf sb = STRBUF_INIT;
 
 	unlink(am_path(state, "dirtyindex"));
@@ -1796,7 +1795,7 @@ static void am_run(struct am_state *state, int resume)
 	if (!state->rebasing) {
 		am_destroy(state);
 		close_object_store(the_repository->objects);
-		run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+		run_gc_auto(state->quiet);
 	}
 }
 
diff --git a/builtin/commit.c b/builtin/commit.c
index 7ba33a3bec..b08c51ae49 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1488,7 +1488,6 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 
 int cmd_commit(int argc, const char **argv, const char *prefix)
 {
-	const char *argv_gc_auto[] = {"gc", "--auto", NULL};
 	static struct wt_status s;
 	static struct option builtin_commit_options[] = {
 		OPT__QUIET(&quiet, N_("suppress summary after successful commit")),
@@ -1697,7 +1696,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		return 1;
 
 	repo_rerere(the_repository, 0);
-	run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+	run_gc_auto(quiet);
 	run_commit_hook(use_editor, get_index_file(), "post-commit", NULL);
 	if (amend && !no_post_rewrite) {
 		commit_post_rewrite(the_repository, current_head, &oid);
diff --git a/builtin/merge.c b/builtin/merge.c
index d127d2225f..b6e9cebabf 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -447,7 +447,6 @@ static void finish(struct commit *head_commit,
 		if (verbosity >= 0 && !merge_msg.len)
 			printf(_("No merge message -- not updating HEAD\n"));
 		else {
-			const char *argv_gc_auto[] = { "gc", "--auto", NULL };
 			update_ref(reflog_message.buf, "HEAD", new_head, head,
 				   0, UPDATE_REFS_DIE_ON_ERR);
 			/*
@@ -455,7 +454,7 @@ static void finish(struct commit *head_commit,
 			 * user should see them.
 			 */
 			close_object_store(the_repository->objects);
-			run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+			run_gc_auto(verbosity < 0);
 		}
 	}
 	if (new_head && show_diffstat) {
diff --git a/builtin/rebase.c b/builtin/rebase.c
index bff53d5d16..fe1a9aba45 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -763,7 +763,6 @@ static int apply_autostash(struct rebase_options *opts)
 static int finish_rebase(struct rebase_options *opts)
 {
 	struct strbuf dir = STRBUF_INIT;
-	const char *argv_gc_auto[] = { "gc", "--auto", NULL };
 	int ret = 0;
 
 	delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
@@ -773,7 +772,7 @@ static int finish_rebase(struct rebase_options *opts)
 	 * We ignore errors in 'gc --auto', since the
 	 * user should see them.
 	 */
-	run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+	run_auto_gc(!(opts->flags & (REBASE_NO_QUIET|REBASE_VERBOSE)));
 	if (opts->type == REBASE_MERGE) {
 		struct replay_opts replay = REPLAY_OPTS_INIT;
 
-- 
2.26.2-561-g07d8ea56f2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] auto-gc: pass --quiet down from am, commit, merge and rebase
  2020-05-06 19:03     ` [PATCH 2/2] auto-gc: pass --quiet down from am, commit, merge and rebase Junio C Hamano
@ 2020-05-06 19:27       ` Junio C Hamano
  0 siblings, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2020-05-06 19:27 UTC (permalink / raw)
  To: git

Junio C Hamano <gitster@pobox.com> writes:

> Teach these commands to do so using the run_gc_auto() helper we
> added earlier.

Heh, what was added was run_auto_gc(), so this patch would not work
well with the previous one.

I'll send out a fixed one sometime later today.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 0/2] Pass down "--quiet" to "gc --auto"
  2020-05-06  9:43 option -q not passed from "git commit" to "git gc --auto" Pierrick Gaudry
  2020-05-06 17:28 ` Taylor Blau
  2020-05-06 17:31 ` Junio C Hamano
@ 2020-05-06 20:18 ` Junio C Hamano
  2020-05-06 20:18   ` [PATCH 1/2] auto-gc: extract a reusable helper from "git fetch" Junio C Hamano
  2020-05-06 20:18   ` [PATCH 2/2] auto-gc: pass --quiet down from am, commit, merge and rebase Junio C Hamano
  2 siblings, 2 replies; 13+ messages in thread
From: Junio C Hamano @ 2020-05-06 20:18 UTC (permalink / raw)
  To: git

A handful commands take "--quiet" from their command line,
but forgets to pass it down, when they trigger an auto gc,
to the "git gc --auto" they internally invoke.

Back in 1991006c (fetch: convert argv_gc_auto to struct argv_array,
2014-08-16), we patched "git fetch" but seems to have forgotten
doing the same for "git merge".  Since then, rewrite of "git am" and
"git rebase" in C spread the same problem into two more codepaths.

Let's clean them up once and for all.

Junio C Hamano (2):
  run_auto_gc(): extract a reusable helper
  auto-gc: pass --quiet down from am, commit, merge and rebase

 builtin/am.c     |  3 +--
 builtin/commit.c |  3 +--
 builtin/fetch.c  | 10 ++--------
 builtin/merge.c  |  3 +--
 builtin/rebase.c |  3 +--
 run-command.c    | 13 +++++++++++++
 run-command.h    |  5 +++++
 7 files changed, 24 insertions(+), 16 deletions(-)

-- 
2.26.2-561-g07d8ea56f2


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/2] auto-gc: extract a reusable helper from "git fetch"
  2020-05-06 20:18 ` [PATCH 0/2] Pass down "--quiet" to "gc --auto" Junio C Hamano
@ 2020-05-06 20:18   ` Junio C Hamano
  2020-05-06 20:18   ` [PATCH 2/2] auto-gc: pass --quiet down from am, commit, merge and rebase Junio C Hamano
  1 sibling, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2020-05-06 20:18 UTC (permalink / raw)
  To: git

Back in 1991006c (fetch: convert argv_gc_auto to struct argv_array,
2014-08-16), we taught "git fetch --quiet" to pass the "--quiet"
option down to "gc --auto".  This issue, however, is not limited to
"fetch":

    $ git grep -e 'gc.*--auto' \*.c

finds hits in "am", "commit", "merge", and "rebase" and these
commands do not pass "--quiet" down to "gc --auto" when they
themselves are told to be quiet.

As a preparatory step, let's introduce a helper function
run_auto_gc(), that the caller can pass a boolean "quiet",
and redo the fix to "git fetch" using the helper.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/fetch.c | 10 ++--------
 run-command.c   | 13 +++++++++++++
 run-command.h   |  5 +++++
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index bf6bab80fa..3e580b9559 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1759,7 +1759,6 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	struct remote *remote = NULL;
 	int result = 0;
 	int prune_tags_ok = 1;
-	struct argv_array argv_gc_auto = ARGV_ARRAY_INIT;
 
 	packet_trace_identity("fetch");
 
@@ -1886,13 +1885,8 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 
 	close_object_store(the_repository->objects);
 
-	if (enable_auto_gc) {
-		argv_array_pushl(&argv_gc_auto, "gc", "--auto", NULL);
-		if (verbosity < 0)
-			argv_array_push(&argv_gc_auto, "--quiet");
-		run_command_v_opt(argv_gc_auto.argv, RUN_GIT_CMD);
-		argv_array_clear(&argv_gc_auto);
-	}
+	if (enable_auto_gc)
+		run_auto_gc(verbosity < 0);
 
 	return result;
 }
diff --git a/run-command.c b/run-command.c
index f5e1149f9b..2771eb936f 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1864,3 +1864,16 @@ int run_processes_parallel_tr2(int n, get_next_task_fn get_next_task,
 
 	return result;
 }
+
+int run_auto_gc(int quiet)
+{
+	struct argv_array argv_gc_auto = ARGV_ARRAY_INIT;
+	int status;
+
+	argv_array_pushl(&argv_gc_auto, "gc", "--auto", NULL);
+	if (quiet)
+		argv_array_push(&argv_gc_auto, "--quiet");
+	status = run_command_v_opt(argv_gc_auto.argv, RUN_GIT_CMD);
+	argv_array_clear(&argv_gc_auto);
+	return status;
+}
diff --git a/run-command.h b/run-command.h
index 0f3cc73ab6..191dfcdafe 100644
--- a/run-command.h
+++ b/run-command.h
@@ -218,6 +218,11 @@ LAST_ARG_MUST_BE_NULL
 int run_hook_le(const char *const *env, const char *name, ...);
 int run_hook_ve(const char *const *env, const char *name, va_list args);
 
+/*
+ * Trigger an auto-gc
+ */
+int run_auto_gc(int quiet);
+
 #define RUN_COMMAND_NO_STDIN 1
 #define RUN_GIT_CMD	     2	/*If this is to be git sub-command */
 #define RUN_COMMAND_STDOUT_TO_STDERR 4
-- 
2.26.2-561-g07d8ea56f2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/2] auto-gc: pass --quiet down from am, commit, merge and rebase
  2020-05-06 20:18 ` [PATCH 0/2] Pass down "--quiet" to "gc --auto" Junio C Hamano
  2020-05-06 20:18   ` [PATCH 1/2] auto-gc: extract a reusable helper from "git fetch" Junio C Hamano
@ 2020-05-06 20:18   ` Junio C Hamano
  2020-05-07 17:22     ` Taylor Blau
  1 sibling, 1 reply; 13+ messages in thread
From: Junio C Hamano @ 2020-05-06 20:18 UTC (permalink / raw)
  To: git

These commands take the --quiet option for their own operation, but
they forget to pass the option down when they invoke "git gc --auto"
internally.

Teach them to do so using the run_auto_gc() helper we added in the
previous step.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/am.c     | 3 +--
 builtin/commit.c | 3 +--
 builtin/merge.c  | 3 +--
 builtin/rebase.c | 3 +--
 4 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/builtin/am.c b/builtin/am.c
index e3dfd93c25..69e50de018 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -1691,7 +1691,6 @@ static int do_interactive(struct am_state *state)
  */
 static void am_run(struct am_state *state, int resume)
 {
-	const char *argv_gc_auto[] = {"gc", "--auto", NULL};
 	struct strbuf sb = STRBUF_INIT;
 
 	unlink(am_path(state, "dirtyindex"));
@@ -1796,7 +1795,7 @@ static void am_run(struct am_state *state, int resume)
 	if (!state->rebasing) {
 		am_destroy(state);
 		close_object_store(the_repository->objects);
-		run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+		run_auto_gc(state->quiet);
 	}
 }
 
diff --git a/builtin/commit.c b/builtin/commit.c
index 7ba33a3bec..fa8eca623e 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1488,7 +1488,6 @@ static int git_commit_config(const char *k, const char *v, void *cb)
 
 int cmd_commit(int argc, const char **argv, const char *prefix)
 {
-	const char *argv_gc_auto[] = {"gc", "--auto", NULL};
 	static struct wt_status s;
 	static struct option builtin_commit_options[] = {
 		OPT__QUIET(&quiet, N_("suppress summary after successful commit")),
@@ -1697,7 +1696,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		return 1;
 
 	repo_rerere(the_repository, 0);
-	run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+	run_auto_gc(quiet);
 	run_commit_hook(use_editor, get_index_file(), "post-commit", NULL);
 	if (amend && !no_post_rewrite) {
 		commit_post_rewrite(the_repository, current_head, &oid);
diff --git a/builtin/merge.c b/builtin/merge.c
index d127d2225f..c66b9b532c 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -447,7 +447,6 @@ static void finish(struct commit *head_commit,
 		if (verbosity >= 0 && !merge_msg.len)
 			printf(_("No merge message -- not updating HEAD\n"));
 		else {
-			const char *argv_gc_auto[] = { "gc", "--auto", NULL };
 			update_ref(reflog_message.buf, "HEAD", new_head, head,
 				   0, UPDATE_REFS_DIE_ON_ERR);
 			/*
@@ -455,7 +454,7 @@ static void finish(struct commit *head_commit,
 			 * user should see them.
 			 */
 			close_object_store(the_repository->objects);
-			run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+			run_auto_gc(verbosity < 0);
 		}
 	}
 	if (new_head && show_diffstat) {
diff --git a/builtin/rebase.c b/builtin/rebase.c
index bff53d5d16..fe1a9aba45 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -763,7 +763,6 @@ static int apply_autostash(struct rebase_options *opts)
 static int finish_rebase(struct rebase_options *opts)
 {
 	struct strbuf dir = STRBUF_INIT;
-	const char *argv_gc_auto[] = { "gc", "--auto", NULL };
 	int ret = 0;
 
 	delete_ref(NULL, "REBASE_HEAD", NULL, REF_NO_DEREF);
@@ -773,7 +772,7 @@ static int finish_rebase(struct rebase_options *opts)
 	 * We ignore errors in 'gc --auto', since the
 	 * user should see them.
 	 */
-	run_command_v_opt(argv_gc_auto, RUN_GIT_CMD);
+	run_auto_gc(!(opts->flags & (REBASE_NO_QUIET|REBASE_VERBOSE)));
 	if (opts->type == REBASE_MERGE) {
 		struct replay_opts replay = REPLAY_OPTS_INIT;
 
-- 
2.26.2-561-g07d8ea56f2


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] auto-gc: pass --quiet down from am, commit, merge and rebase
  2020-05-06 20:18   ` [PATCH 2/2] auto-gc: pass --quiet down from am, commit, merge and rebase Junio C Hamano
@ 2020-05-07 17:22     ` Taylor Blau
  2020-05-07 19:31       ` Junio C Hamano
  0 siblings, 1 reply; 13+ messages in thread
From: Taylor Blau @ 2020-05-07 17:22 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

Hi Junio,

On Wed, May 06, 2020 at 01:18:30PM -0700, Junio C Hamano wrote:
> These commands take the --quiet option for their own operation, but
> they forget to pass the option down when they invoke "git gc --auto"
> internally.
>
> Teach them to do so using the run_auto_gc() helper we added in the
> previous step.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  builtin/am.c     | 3 +--
>  builtin/commit.c | 3 +--
>  builtin/merge.c  | 3 +--
>  builtin/rebase.c | 3 +--
>  4 files changed, 4 insertions(+), 8 deletions(-)

Very nicely done. Sorry that these took me a little while to look at.
This and the patch before it both have my:

  Reviewed-by: Taylor Blau <me@ttaylorr.com>

Thanks,
Taylor

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/2] auto-gc: pass --quiet down from am, commit, merge and rebase
  2020-05-07 17:22     ` Taylor Blau
@ 2020-05-07 19:31       ` Junio C Hamano
  0 siblings, 0 replies; 13+ messages in thread
From: Junio C Hamano @ 2020-05-07 19:31 UTC (permalink / raw)
  To: Taylor Blau; +Cc: git

Taylor Blau <me@ttaylorr.com> writes:

> Hi Junio,
>
> On Wed, May 06, 2020 at 01:18:30PM -0700, Junio C Hamano wrote:
>> These commands take the --quiet option for their own operation, but
>> they forget to pass the option down when they invoke "git gc --auto"
>> internally.
>>
>> Teach them to do so using the run_auto_gc() helper we added in the
>> previous step.
>>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>>  builtin/am.c     | 3 +--
>>  builtin/commit.c | 3 +--
>>  builtin/merge.c  | 3 +--
>>  builtin/rebase.c | 3 +--
>>  4 files changed, 4 insertions(+), 8 deletions(-)
>
> Very nicely done. Sorry that these took me a little while to look at.
> This and the patch before it both have my:
>
>   Reviewed-by: Taylor Blau <me@ttaylorr.com>
>
> Thanks,
> Taylor

Thanks.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-05-07 19:31 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-06  9:43 option -q not passed from "git commit" to "git gc --auto" Pierrick Gaudry
2020-05-06 17:28 ` Taylor Blau
2020-05-06 17:33   ` Taylor Blau
2020-05-06 17:38     ` Taylor Blau
2020-05-06 17:31 ` Junio C Hamano
2020-05-06 18:56   ` Re* " Junio C Hamano
2020-05-06 19:03     ` [PATCH 2/2] auto-gc: pass --quiet down from am, commit, merge and rebase Junio C Hamano
2020-05-06 19:27       ` Junio C Hamano
2020-05-06 20:18 ` [PATCH 0/2] Pass down "--quiet" to "gc --auto" Junio C Hamano
2020-05-06 20:18   ` [PATCH 1/2] auto-gc: extract a reusable helper from "git fetch" Junio C Hamano
2020-05-06 20:18   ` [PATCH 2/2] auto-gc: pass --quiet down from am, commit, merge and rebase Junio C Hamano
2020-05-07 17:22     ` Taylor Blau
2020-05-07 19:31       ` 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).