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