* 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).