{"thread":{"id":"49388","subject":"[PATCH] gc: fix regression in 7b0f229222 impacting --quiet","startedAt":"2018-09-19T21:01:57Z","lastAt":"2018-09-20T12:52:52Z","messageCount":3,"participants":["Ævar Arnfjörð Bjarmason","Martin Ågren","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"358490","messageId":"20180919210138.31940-1-avarab@gmail.com","threadId":"49388","inReplyTo":null,"subject":"[PATCH] gc: fix regression in 7b0f229222 impacting --quiet","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-09-19T21:01:38Z","receivedAt":"2018-09-19T21:01:57Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a regression in my recent 7b0f229222 (\"commit-graph write: add\nprogress output\", 2018-09-17), the newly added progress output for\n\"commit-graph write\" didn't check the --quiet option.\n\nDo so, and add a test asserting that this works as expected. Since the\nTTY perquisite isn't available everywhere let's add a version of this\nthat both requires and doesn't require that. This test might be overly\nspecific and will break if new progress output is added, but I think\nit'll serve as a good reminder to test the undertested progress\nmode(s).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/gc.c  |  2 +-\n t/t6500-gc.sh | 21 +++++++++++++++++++++\n 2 files changed, 22 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 06ddd3aea5..91ffb1a803 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -647,7 +647,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \n \tif (gc_write_commit_graph)\n \t\twrite_commit_graph_reachable(get_object_directory(), 0,\n-\t\t\t\t\t     !daemonized);\n+\t\t\t\t\t     !quiet && !daemonized);\n \n \tif (auto_gc && too_many_loose_objects())\n \t\twarning(_(\"There are too many unreachable loose objects; \"\ndiff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\nindex 818435f04e..6183702c30 100755\n--- a/t/t6500-gc.sh\n+++ b/t/t6500-gc.sh\n@@ -4,6 +4,7 @@ test_description='basic git gc tests\n '\n \n . ./test-lib.sh\n+. \"$TEST_DIRECTORY\"/lib-terminal.sh\n \n test_expect_success 'setup' '\n \t# do not let the amount of physical memory affects gc\n@@ -99,6 +100,26 @@ test_expect_success 'auto gc with too many loose objects does not attempt to cre\n \ttest_line_count = 2 new # There is one new pack and its .idx\n '\n \n+test_expect_success 'gc --no-quiet' '\n+\tgit -c gc.writeCommitGraph=true gc --no-quiet >stdout 2>stderr &&\n+\t! test -s stdout &&\n+\ttest_line_count = 1 stderr &&\n+\ttest_i18ngrep \"Computing commit graph generation numbers\" stderr\n+'\n+\n+test_expect_success TTY 'with TTY: gc --no-quiet' '\n+\ttest_terminal git -c gc.writeCommitGraph=true gc --no-quiet >stdout 2>stderr &&\n+\t! test -s stdout &&\n+\ttest_i18ngrep \"Enumerating objects\" stderr &&\n+\ttest_i18ngrep \"Computing commit graph generation numbers\" stderr\n+'\n+\n+test_expect_success 'gc --quiet' '\n+\tgit -c gc.writeCommitGraph=true gc --quiet >stdout 2>stderr &&\n+\t! test -s stdout &&\n+\t! test -s stderr\n+'\n+\n run_and_wait_for_auto_gc () {\n \t# We read stdout from gc for the side effect of waiting until the\n \t# background gc process exits, closing its fd 9.  Furthermore, the\n-- \n2.19.0.444.g18242da7ef\n\n"},{"id":"358491","messageId":"CAN0heSr=upqAVzjm=gQoGVnOjBZcDWhmj3W8gFFa8bcfCgaHTg@mail.gmail.com","threadId":"49388","inReplyTo":"20180919210138.31940-1-avarab@gmail.com","subject":"Re: [PATCH] gc: fix regression in 7b0f229222 impacting --quiet","fromName":"Martin Ågren","fromEmail":"martin.agren@gmail.com","sentAt":"2018-09-19T21:21:54Z","receivedAt":"2018-09-19T21:22:07Z","isPatch":true,"sender":{"key":"martin.agren@gmail.com","avatar":null},"body":"On Wed, 19 Sep 2018 at 23:04, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n> Fix a regression in my recent 7b0f229222 (\"commit-graph write: add\n> progress output\", 2018-09-17), the newly added progress output for\n> \"commit-graph write\" didn't check the --quiet option.\n\ns/, t/. T/, perhaps. Maybe also s/did/does/.\n\n> Do so, and add a test asserting that this works as expected. Since the\n> TTY perquisite isn't available everywhere let's add a version of this\n\ns/perq/prereq/\n\n> that both requires and doesn't require that. This test might be overly\n> specific and will break if new progress output is added, but I think\n> it'll serve as a good reminder to test the undertested progress\n> mode(s).\n\n> +test_expect_success 'gc --no-quiet' '\n> +       git -c gc.writeCommitGraph=true gc --no-quiet >stdout 2>stderr &&\n> +       ! test -s stdout &&\n\n`test_must_be_empty` for easier debugging?\n\n> +       test_line_count = 1 stderr &&\n> +       test_i18ngrep \"Computing commit graph generation numbers\" stderr\n> +'\n"},{"id":"358507","messageId":"0189570e-2b64-5933-8d60-f5186eb063b1@gmail.com","threadId":"49388","inReplyTo":"CAN0heSr=upqAVzjm=gQoGVnOjBZcDWhmj3W8gFFa8bcfCgaHTg@mail.gmail.com","subject":"Re: [PATCH] gc: fix regression in 7b0f229222 impacting --quiet","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2018-09-19T23:22:24Z","receivedAt":"2018-09-20T12:52:52Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 9/19/2018 5:21 PM, Martin Ågren wrote:\n> On Wed, 19 Sep 2018 at 23:04, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n>> Fix a regression in my recent 7b0f229222 (\"commit-graph write: add\n>> progress output\", 2018-09-17), the newly added progress output for\n>> \"commit-graph write\" didn't check the --quiet option.\nThanks for fixing this issue. Sorry I missed it in review.\n\nI looked through the patch and didn't find anything more than Martin \nalready did. I appreciate the tests.\n\nThanks,\n-Stolee\n\n"}]}