{"thread":{"id":"52193","subject":"[PATCH 0/1] commit-graph: use start_delayed_progress()","startedAt":"2019-11-05T16:06:00Z","lastAt":"2019-12-01T09:33:15Z","messageCount":32,"participants":["Derrick Stolee via GitGitGadget","Derrick Stolee","Jeff King","Junio C Hamano","SZEDER Gábor"],"isPatch":true,"patchVersion":1,"patchTotal":1},"messages":[{"id":"385457","messageId":"pull.450.git.1572969955.gitgitgadget@gmail.com","threadId":"52193","inReplyTo":null,"subject":"[PATCH 0/1] commit-graph: use start_delayed_progress()","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-05T16:05:54Z","receivedAt":"2019-11-05T16:06:00Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"Thanks, Ryenus, for reporting this problem.\n\nDerrick Stolee (1):\n commit-graph: use start_delayed_progress()\n\n commit-graph.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\n\nbase-commit: da72936f544fec5a335e66432610e4cef4430991\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-450%2Fderrickstolee%2Fcommit-graph-progress-fix-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-450/derrickstolee/commit-graph-progress-fix-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/450\n-- \ngitgitgadget\n"},{"id":"385458","messageId":"174c05bf29bbd92342aa45132592c348f988cf0b.1572969955.git.gitgitgadget@gmail.com","threadId":"52193","inReplyTo":"pull.450.git.1572969955.gitgitgadget@gmail.com","subject":"[PATCH 1/1] commit-graph: use start_delayed_progress()","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-05T16:05:55Z","receivedAt":"2019-11-05T16:06:00Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen writing a commit-graph, we show progress along several commit\nwalks. When we use start_delayed_progress(), the progress line will\nonly appear if that step takes a decent amount of time.\n\nHowever, one place was missed: computing generation numbers. This is\nnormally a very fast operation as all commits have been parsed in a\nprevious step. But, this is showing up for all users no matter how few\ncommits are being added.\n\nReported-by: ryenus \nSigned-off-by: Derrick Stolee \n---\n commit-graph.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 0aea7b2ae5..071e1c6e9b 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1103,7 +1103,7 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx)\n \tstruct commit_list *list = NULL;\n \n \tif (ctx->report_progress)\n-\t\tctx->progress = start_progress(\n+\t\tctx->progress = start_delayed_progress(\n \t\t\t\t\t_(\"Computing commit graph generation numbers\"),\n \t\t\t\t\tctx->commits.nr);\n \tfor (i = 0; i < ctx->commits.nr; i++) {\n-- \ngitgitgadget\n"},{"id":"385473","messageId":"18dde6fe-9072-2069-8063-f1cfcdee3f38@gmail.com","threadId":"52193","inReplyTo":"174c05bf29bbd92342aa45132592c348f988cf0b.1572969955.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/1] commit-graph: use start_delayed_progress()","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-11-05T17:38:10Z","receivedAt":"2019-11-05T17:38:13Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/5/2019 11:05 AM, Derrick Stolee via GitGitGadget wrote:\n> From: Derrick Stolee \n> \n> When writing a commit-graph, we show progress along several commit\n> walks. When we use start_delayed_progress(), the progress line will\n> only appear if that step takes a decent amount of time.\n> \n> However, one place was missed: computing generation numbers. This is\n> normally a very fast operation as all commits have been parsed in a\n> previous step. But, this is showing up for all users no matter how few\n> commits are being added.\n\nFor course, now that we do not force at least one progress line to show\nup, the tests that check the `--progress` option (or `--no-quiet` for GC)\nwill fail with this patch.\n\nv2 coming soon.\n\n-Stolee\n\n"},{"id":"385486","messageId":"pull.450.v2.git.1572984842.gitgitgadget@gmail.com","threadId":"52193","inReplyTo":"pull.450.git.1572969955.gitgitgadget@gmail.com","subject":"[PATCH v2 0/1] commit-graph: use start_delayed_progress()","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-05T20:14:01Z","receivedAt":"2019-11-05T20:14:07Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"Thanks, Ryenus, for reporting this problem.\n\nDerrick Stolee (1):\n commit-graph: use start_delayed_progress()\n\n commit-graph.c | 2 +-\n t/t5318-commit-graph.sh | 6 ------\n t/t6500-gc.sh | 14 --------------\n 3 files changed, 1 insertion(+), 21 deletions(-)\n\n\nbase-commit: da72936f544fec5a335e66432610e4cef4430991\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-450%2Fderrickstolee%2Fcommit-graph-progress-fix-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-450/derrickstolee/commit-graph-progress-fix-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/450\n\nRange-diff vs v1:\n\n 1: 174c05bf29 ! 1: 78bd6bc2c0 commit-graph: use start_delayed_progress()\n @@ -11,6 +11,10 @@\n previous step. But, this is showing up for all users no matter how few\n commits are being added.\n \n + Now that we changed this method, very fast commands show no progess at\n + all. This means we need to stop testing for seeing these progress lines\n + in the test suite.\n +\n Reported-by: ryenus \n Signed-off-by: Derrick Stolee \n \n @@ -26,3 +30,45 @@\n \t\t\t\t\t_(\"Computing commit graph generation numbers\"),\n \t\t\t\t\tctx->commits.nr);\n \tfor (i = 0; i < ctx->commits.nr; i++) {\n +\n + diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\n + --- a/t/t5318-commit-graph.sh\n + +++ b/t/t5318-commit-graph.sh\n +@@\n + \ttest_line_count = 0 err\n + '\n + \n +-test_expect_success 'commit-graph write force progress on for stderr' '\n +-\tcd \"$TRASH_DIRECTORY/full\" &&\n +-\tgit commit-graph write --progress 2>err &&\n +-\ttest_file_not_empty err\n +-'\n +-\n + test_expect_success 'commit-graph write with the --no-progress option' '\n + \tcd \"$TRASH_DIRECTORY/full\" &&\n + \tgit commit-graph write --no-progress 2>err &&\n +\n + diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\n + --- a/t/t6500-gc.sh\n + +++ b/t/t6500-gc.sh\n +@@\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 +-\ttest_must_be_empty 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 +-\ttest_must_be_empty 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 + \ttest_must_be_empty stdout &&\n\n-- \ngitgitgadget\n"},{"id":"385487","messageId":"78bd6bc2c02f1daf13938a738d8eae56b5f6b74c.1572984842.git.gitgitgadget@gmail.com","threadId":"52193","inReplyTo":"pull.450.v2.git.1572984842.gitgitgadget@gmail.com","subject":"[PATCH v2 1/1] commit-graph: use start_delayed_progress()","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-05T20:14:02Z","receivedAt":"2019-11-05T20:14:08Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen writing a commit-graph, we show progress along several commit\nwalks. When we use start_delayed_progress(), the progress line will\nonly appear if that step takes a decent amount of time.\n\nHowever, one place was missed: computing generation numbers. This is\nnormally a very fast operation as all commits have been parsed in a\nprevious step. But, this is showing up for all users no matter how few\ncommits are being added.\n\nNow that we changed this method, very fast commands show no progess at\nall. This means we need to stop testing for seeing these progress lines\nin the test suite.\n\nReported-by: ryenus \nSigned-off-by: Derrick Stolee \n---\n commit-graph.c | 2 +-\n t/t5318-commit-graph.sh | 6 ------\n t/t6500-gc.sh | 14 --------------\n 3 files changed, 1 insertion(+), 21 deletions(-)\n\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 0aea7b2ae5..071e1c6e9b 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1103,7 +1103,7 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx)\n \tstruct commit_list *list = NULL;\n \n \tif (ctx->report_progress)\n-\t\tctx->progress = start_progress(\n+\t\tctx->progress = start_delayed_progress(\n \t\t\t\t\t_(\"Computing commit graph generation numbers\"),\n \t\t\t\t\tctx->commits.nr);\n \tfor (i = 0; i < ctx->commits.nr; i++) {\ndiff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\nindex d42b3efe39..8759ff0f3a 100755\n--- a/t/t5318-commit-graph.sh\n+++ b/t/t5318-commit-graph.sh\n@@ -130,12 +130,6 @@ test_expect_success 'commit-graph write progress off for redirected stderr' '\n \ttest_line_count = 0 err\n '\n \n-test_expect_success 'commit-graph write force progress on for stderr' '\n-\tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph write --progress 2>err &&\n-\ttest_file_not_empty err\n-'\n-\n test_expect_success 'commit-graph write with the --no-progress option' '\n \tcd \"$TRASH_DIRECTORY/full\" &&\n \tgit commit-graph write --no-progress 2>err &&\ndiff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\nindex c0f04dc6b0..23faa5345f 100755\n--- a/t/t6500-gc.sh\n+++ b/t/t6500-gc.sh\n@@ -102,20 +102,6 @@ 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-\ttest_must_be_empty 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-\ttest_must_be_empty 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 \ttest_must_be_empty stdout &&\n-- \ngitgitgadget\n"},{"id":"385526","messageId":"20191106040955.GD4307@sigill.intra.peff.net","threadId":"52193","inReplyTo":"78bd6bc2c02f1daf13938a738d8eae56b5f6b74c.1572984842.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v2 1/1] commit-graph: use start_delayed_progress()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-06T04:09:55Z","receivedAt":"2019-11-06T04:09:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Nov 05, 2019 at 08:14:02PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> From: Derrick Stolee \n> \n> When writing a commit-graph, we show progress along several commit\n> walks. When we use start_delayed_progress(), the progress line will\n> only appear if that step takes a decent amount of time.\n> \n> However, one place was missed: computing generation numbers. This is\n> normally a very fast operation as all commits have been parsed in a\n> previous step. But, this is showing up for all users no matter how few\n> commits are being added.\n\nYep, makes sense (especially that it should all the other progress as\npart of the same process).\n\n> Now that we changed this method, very fast commands show no progess at\n> all. This means we need to stop testing for seeing these progress lines\n> in the test suite.\n\nI think this is OK for now, though it does make me wonder if\n\"--progress\" ought to perhaps override \"delayed\" in some instances,\nsince it's a positive signal from the caller that they're interested in\nseeing progress.\n\n-Peff\n"},{"id":"385592","messageId":"832b3b8b-63e6-1d32-0013-158b3c6725c8@gmail.com","threadId":"52193","inReplyTo":"20191106040955.GD4307@sigill.intra.peff.net","subject":"Re: [PATCH v2 1/1] commit-graph: use start_delayed_progress()","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-11-06T13:21:48Z","receivedAt":"2019-11-06T13:21:54Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/5/2019 11:09 PM, Jeff King wrote:\n> On Tue, Nov 05, 2019 at 08:14:02PM +0000, Derrick Stolee via GitGitGadget wrote:\n> \n>> From: Derrick Stolee \n>>\n>> When writing a commit-graph, we show progress along several commit\n>> walks. When we use start_delayed_progress(), the progress line will\n>> only appear if that step takes a decent amount of time.\n>>\n>> However, one place was missed: computing generation numbers. This is\n>> normally a very fast operation as all commits have been parsed in a\n>> previous step. But, this is showing up for all users no matter how few\n>> commits are being added.\n> \n> Yep, makes sense (especially that it should all the other progress as\n> part of the same process).\n> \n>> Now that we changed this method, very fast commands show no progess at\n>> all. This means we need to stop testing for seeing these progress lines\n>> in the test suite.\n> \n> I think this is OK for now, though it does make me wonder if\n> \"--progress\" ought to perhaps override \"delayed\" in some instances,\n> since it's a positive signal from the caller that they're interested in\n> seeing progress.\n\nI was thinking that we could start with a GIT_TEST_PROGRESS environment\nvariable to force all delayed progress to act like non-delayed progress.\nThat would at least give us confirmation on these kinds of tests.\n\nThe impact of doing that inside the progress code could be large, so\nperhaps that is best left for a follow-up.\n\nThanks,\n-Stolee\n"},{"id":"385650","messageId":"xmqqpni4s3mn.fsf@gitster-ct.c.googlers.com","threadId":"52193","inReplyTo":"20191106040955.GD4307@sigill.intra.peff.net","subject":"Re: [PATCH v2 1/1] commit-graph: use start_delayed_progress()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-07T04:37:52Z","receivedAt":"2019-11-07T04:37:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I think this is OK for now, though it does make me wonder if\n> \"--progress\" ought to perhaps override \"delayed\" in some instances,\n> since it's a positive signal from the caller that they're interested in\n> seeing progress.\n\nI did have the same reaction after seeing the change to 5318 where\nthe expected output from \"git commit-graph write --progress\" has\nbecome unreliable.\n\nI think there are possibly three kinds of folks:\n\n - I do not want the output smudged with any progress (e.g. I am a\n script);\n\n - I want to see progress if it takes very long, but do not waste\n vertical screen real estate if it does not make me wait (e.g. I\n am an interactive user who occasionally wants a cue to leave the\n keyboard to grab coffee); and\n\n - I want to see all progress (... now who am I? Taking a\n screenshot to write a tutorial or something???).\n\nIn the ideal world, the three choices above should probably be\n\"--progress=(no|auto|always)\" where not having any defaults to one\nof them (probably \"auto\", as the code can use isatty() to further\nturn it to \"no\").\n\nMaking \"--progress\" to mean \"--progress=always\" is OK, but it leaves\nno way to override an earlier --[no-]progress on the command line,\nwhich feels somewhat satisfying.\n\nThanks.\n\n\n\n"},{"id":"385671","messageId":"20191107064058.GG6431@sigill.intra.peff.net","threadId":"52193","inReplyTo":"832b3b8b-63e6-1d32-0013-158b3c6725c8@gmail.com","subject":"Re: [PATCH v2 1/1] commit-graph: use start_delayed_progress()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-07T06:40:58Z","receivedAt":"2019-11-07T06:41:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Nov 06, 2019 at 08:21:48AM -0500, Derrick Stolee wrote:\n\n> >> Now that we changed this method, very fast commands show no progess at\n> >> all. This means we need to stop testing for seeing these progress lines\n> >> in the test suite.\n> > \n> > I think this is OK for now, though it does make me wonder if\n> > \"--progress\" ought to perhaps override \"delayed\" in some instances,\n> > since it's a positive signal from the caller that they're interested in\n> > seeing progress.\n> \n> I was thinking that we could start with a GIT_TEST_PROGRESS environment\n> variable to force all delayed progress to act like non-delayed progress.\n> That would at least give us confirmation on these kinds of tests.\n\nI think this could actually be a non-test variable. E.g., something like\nthis:\n\ndiff --git a/progress.c b/progress.c\nindex 0063559aab..74b90e8898 100644\n--- a/progress.c\n+++ b/progress.c\n@@ -14,6 +14,7 @@\n #include \"strbuf.h\"\n #include \"trace.h\"\n #include \"utf8.h\"\n+#include \"config.h\"\n \n #define TP_IDX_MAX 8\n \n@@ -269,7 +270,8 @@ static struct progress *start_progress_delay(const char *title, uint64_t total,\n \n struct progress *start_delayed_progress(const char *title, uint64_t total)\n {\n-\treturn start_progress_delay(title, total, 2, 0);\n+\tint delay_in_secs = git_env_ulong(\"GIT_PROGRESS_DELAY\", 2);\n+\treturn start_progress_delay(title, total, delay_in_secs, 0);\n }\n \n struct progress *start_progress(const char *title, uint64_t total)\n\n\nwhich lets you ask for more verbose progress. There are times when I'd\nuse something like this for general debugging. Though these days I might\nsuggest that something like GIT_TRACE2_PERF hook the progress code to\noutput. That's a bit more complicated to implement, though.\n\n-Peff\n"},{"id":"385672","messageId":"20191107064356.GH6431@sigill.intra.peff.net","threadId":"52193","inReplyTo":"xmqqpni4s3mn.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 1/1] commit-graph: use start_delayed_progress()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-07T06:43:56Z","receivedAt":"2019-11-07T06:43:58Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 07, 2019 at 01:37:52PM +0900, Junio C Hamano wrote:\n\n> Jeff King writes:\n> \n> > I think this is OK for now, though it does make me wonder if\n> > \"--progress\" ought to perhaps override \"delayed\" in some instances,\n> > since it's a positive signal from the caller that they're interested in\n> > seeing progress.\n> \n> I did have the same reaction after seeing the change to 5318 where\n> the expected output from \"git commit-graph write --progress\" has\n> become unreliable.\n> \n> I think there are possibly three kinds of folks:\n> \n> - I do not want the output smudged with any progress (e.g. I am a\n> script);\n> \n> - I want to see progress if it takes very long, but do not waste\n> vertical screen real estate if it does not make me wait (e.g. I\n> am an interactive user who occasionally wants a cue to leave the\n> keyboard to grab coffee); and\n> \n> - I want to see all progress (... now who am I? Taking a\n> screenshot to write a tutorial or something???).\n\nI think type 3 may be people who want to understand more about the\nprogram flow, and where it's at when it sees an error.\n\n> In the ideal world, the three choices above should probably be\n> \"--progress=(no|auto|always)\" where not having any defaults to one\n> of them (probably \"auto\", as the code can use isatty() to further\n> turn it to \"no\").\n\nI think any no/auto/always here is tricky, because it already has a\nmeaning: to use or disregard isatty(2). And overriding that might be\nindependent of the \"type\" (think pack-objects on a server generating\noutput that's going over the wire; we have to tell it \"yes, definitely\nshow progress even though there is no terminal\", but that has nothing to\ndo with any \"delay\" decisions).\n\n-Peff\n"},{"id":"385688","messageId":"xmqq36f0qak0.fsf@gitster-ct.c.googlers.com","threadId":"52193","inReplyTo":"20191107064356.GH6431@sigill.intra.peff.net","subject":"Re: [PATCH v2 1/1] commit-graph: use start_delayed_progress()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-07T09:51:11Z","receivedAt":"2019-11-07T09:51:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> I think any no/auto/always here is tricky, because it already has a\n> meaning: to use or disregard isatty(2). And overriding that might be\n> independent of the \"type\".\n\nYes, your \"let the users optionally give the delay\" in the other\nsubthread makes a lot more sense.\n\nThanks.\n"},{"id":"385705","messageId":"29e88f9c-2b35-efe6-9be1-2891061a7109@gmail.com","threadId":"52193","inReplyTo":"20191107064058.GG6431@sigill.intra.peff.net","subject":"Re: [PATCH v2 1/1] commit-graph: use start_delayed_progress()","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-11-07T13:30:55Z","receivedAt":"2019-11-07T13:31:01Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/7/2019 1:40 AM, Jeff King wrote:\n> On Wed, Nov 06, 2019 at 08:21:48AM -0500, Derrick Stolee wrote:\n> \n>>>> Now that we changed this method, very fast commands show no progess at\n>>>> all. This means we need to stop testing for seeing these progress lines\n>>>> in the test suite.\n>>>\n>>> I think this is OK for now, though it does make me wonder if\n>>> \"--progress\" ought to perhaps override \"delayed\" in some instances,\n>>> since it's a positive signal from the caller that they're interested in\n>>> seeing progress.\n>>\n>> I was thinking that we could start with a GIT_TEST_PROGRESS environment\n>> variable to force all delayed progress to act like non-delayed progress.\n>> That would at least give us confirmation on these kinds of tests.\n> \n> I think this could actually be a non-test variable. E.g., something like\n> this:\n> \n> diff --git a/progress.c b/progress.c\n> index 0063559aab..74b90e8898 100644\n> --- a/progress.c\n> +++ b/progress.c\n> @@ -14,6 +14,7 @@\n> #include \"strbuf.h\"\n> #include \"trace.h\"\n> #include \"utf8.h\"\n> +#include \"config.h\"\n> \n> #define TP_IDX_MAX 8\n> \n> @@ -269,7 +270,8 @@ static struct progress *start_progress_delay(const char *title, uint64_t total,\n> \n> struct progress *start_delayed_progress(const char *title, uint64_t total)\n> {\n> -\treturn start_progress_delay(title, total, 2, 0);\n> +\tint delay_in_secs = git_env_ulong(\"GIT_PROGRESS_DELAY\", 2);\n> +\treturn start_progress_delay(title, total, delay_in_secs, 0);\n> }\n> \n> struct progress *start_progress(const char *title, uint64_t total)\n\nI like this idea. It allows us to force the progress on in tests, and for\nusers to tweak their preferred delay. That includes _increasing_ the delay\nif they want to.\n\n> which lets you ask for more verbose progress. There are times when I'd\n> use something like this for general debugging. Though these days I might\n> suggest that something like GIT_TRACE2_PERF hook the progress code to\n> output. That's a bit more complicated to implement, though.\n\nWould it make sense to make delay_in_secs a local static variable, so we\nremember it between calls? That would allow us to check the environment only\nonce (not that it is usually expensive).\n\n-Stolee\n"},{"id":"385711","messageId":"pull.450.v3.git.1573148818.gitgitgadget@gmail.com","threadId":"52193","inReplyTo":"pull.450.v2.git.1572984842.gitgitgadget@gmail.com","subject":"[PATCH v3 0/2] commit-graph: use start_delayed_progress()","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-07T17:46:56Z","receivedAt":"2019-11-07T17:47:03Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"Thanks, Ryenus, for reporting this problem.\n\nUpdate in V3:\n\nBased on our discussion, I've added the suggested GIT_PROGRESS_DELAY\nenvironment variable. This allowed the existing tests to stick around with\none exception in the GC tests. The test remains, but we can no longer look\nat the commit-graph output.\n\nDerrick Stolee (2):\n progress: create GIT_PROGRESS_DELAY\n commit-graph: use start_delayed_progress()\n\n Documentation/git.txt | 4 ++++\n commit-graph.c | 2 +-\n progress.c | 8 +++++++-\n t/t5318-commit-graph.sh | 4 ++--\n t/t6500-gc.sh | 6 ++----\n 5 files changed, 16 insertions(+), 8 deletions(-)\n\n\nbase-commit: da72936f544fec5a335e66432610e4cef4430991\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-450%2Fderrickstolee%2Fcommit-graph-progress-fix-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-450/derrickstolee/commit-graph-progress-fix-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/450\n\nRange-diff vs v2:\n\n -: ---------- > 1: 656dba5afb progress: create GIT_PROGRESS_DELAY\n 1: 78bd6bc2c0 ! 2: 3c0c9675e1 commit-graph: use start_delayed_progress()\n @@ -11,9 +11,13 @@\n previous step. But, this is showing up for all users no matter how few\n commits are being added.\n \n - Now that we changed this method, very fast commands show no progess at\n - all. This means we need to stop testing for seeing these progress lines\n - in the test suite.\n + The tests that check for the progress output have already been updated\n + to use GIT_PROGRESS_DELAY=0 to force the expected output. However, there\n + is one test in t6500-gc.sh that uses the test_terminal method. This\n + mechanism does not preserve the GIT_PROGRESS_DELAY environment variable,\n + so we need to modify check on the output. We still watch for the\n + \"Enumerating objects\" progress but no longer look for \"Computing\n + commit graph generation numbers\".\n \n Reported-by: ryenus \n Signed-off-by: Derrick Stolee \n @@ -31,44 +35,16 @@\n \t\t\t\t\tctx->commits.nr);\n \tfor (i = 0; i < ctx->commits.nr; i++) {\n \n - diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\n - --- a/t/t5318-commit-graph.sh\n - +++ b/t/t5318-commit-graph.sh\n -@@\n - \ttest_line_count = 0 err\n - '\n - \n --test_expect_success 'commit-graph write force progress on for stderr' '\n --\tcd \"$TRASH_DIRECTORY/full\" &&\n --\tgit commit-graph write --progress 2>err &&\n --\ttest_file_not_empty err\n --'\n --\n - test_expect_success 'commit-graph write with the --no-progress option' '\n - \tcd \"$TRASH_DIRECTORY/full\" &&\n - \tgit commit-graph write --no-progress 2>err &&\n -\n diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\n --- a/t/t6500-gc.sh\n +++ b/t/t6500-gc.sh\n @@\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 --\ttest_must_be_empty 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 --\ttest_must_be_empty stdout &&\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 + \ttest_must_be_empty stdout &&\n -\ttest_i18ngrep \"Enumerating objects\" stderr &&\n -\ttest_i18ngrep \"Computing commit graph generation numbers\" stderr\n --'\n --\n ++\ttest_i18ngrep \"Enumerating objects\" stderr\n + '\n + \n test_expect_success 'gc --quiet' '\n - \tgit -c gc.writeCommitGraph=true gc --quiet >stdout 2>stderr &&\n - \ttest_must_be_empty stdout &&\n\n-- \ngitgitgadget\n"},{"id":"385712","messageId":"656dba5afb818d0caa7616d0e58c9728803f8d04.1573148818.git.gitgitgadget@gmail.com","threadId":"52193","inReplyTo":"pull.450.v3.git.1573148818.gitgitgadget@gmail.com","subject":"[PATCH v3 1/2] progress: create GIT_PROGRESS_DELAY","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-07T17:46:57Z","receivedAt":"2019-11-07T17:47:04Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe start_delayed_progress() method is a preferred way to show\noptional progress to users as it ignores steps that take less\nthan two seconds. However, this makes testing unreliable as tests\nexpect to be very fast.\n\nIn addition, users may want to decrease or increase this time\ninterval depending on their preferences for terminal noise.\n\nCreate the GIT_PROGRESS_DELAY environment variable to control\nthe delay set during start_delayed_progress(). Set the value\nin some tests to guarantee their output remains consistent.\n\nHelped-by: Jeff King \nSigned-off-by: Derrick Stolee \n---\n Documentation/git.txt | 4 ++++\n progress.c | 8 +++++++-\n t/t5318-commit-graph.sh | 4 ++--\n t/t6500-gc.sh | 3 +--\n 4 files changed, 14 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 9b82564d1a..1c420da208 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -544,6 +544,10 @@ other\n \ta pager. See also the `core.pager` option in\n \tlinkgit:git-config[1].\n \n+`GIT_PROGRESS_DELAY`::\n+\tA number controlling how many seconds to delay before showing\n+\toptional progress indicators. Defaults to 2.\n+\n `GIT_EDITOR`::\n \tThis environment variable overrides `$EDITOR` and `$VISUAL`.\n \tIt is used by several Git commands when, on interactive mode,\ndiff --git a/progress.c b/progress.c\nindex 0063559aab..4ad1a3c6eb 100644\n--- a/progress.c\n+++ b/progress.c\n@@ -14,6 +14,7 @@\n #include \"strbuf.h\"\n #include \"trace.h\"\n #include \"utf8.h\"\n+#include \"config.h\"\n \n #define TP_IDX_MAX 8\n \n@@ -269,7 +270,12 @@ static struct progress *start_progress_delay(const char *title, uint64_t total,\n \n struct progress *start_delayed_progress(const char *title, uint64_t total)\n {\n-\treturn start_progress_delay(title, total, 2, 0);\n+\tstatic int delay_in_secs = -1;\n+\n+\tif (delay_in_secs < 0)\n+\t\tdelay_in_secs = git_env_ulong(\"GIT_PROGRESS_DELAY\", 2);\n+\n+\treturn start_progress_delay(title, total, delay_in_secs, 0);\n }\n \n struct progress *start_progress(const char *title, uint64_t total)\ndiff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\nindex d42b3efe39..0824857e1f 100755\n--- a/t/t5318-commit-graph.sh\n+++ b/t/t5318-commit-graph.sh\n@@ -132,7 +132,7 @@ test_expect_success 'commit-graph write progress off for redirected stderr' '\n \n test_expect_success 'commit-graph write force progress on for stderr' '\n \tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph write --progress 2>err &&\n+\tGIT_PROGRESS_DELAY=0 git commit-graph write --progress 2>err &&\n \ttest_file_not_empty err\n '\n \n@@ -150,7 +150,7 @@ test_expect_success 'commit-graph verify progress off for redirected stderr' '\n \n test_expect_success 'commit-graph verify force progress on for stderr' '\n \tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph verify --progress 2>err &&\n+\tGIT_PROGRESS_DELAY=0 git commit-graph verify --progress 2>err &&\n \ttest_file_not_empty err\n '\n \ndiff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\nindex c0f04dc6b0..7f79eedd1c 100755\n--- a/t/t6500-gc.sh\n+++ b/t/t6500-gc.sh\n@@ -103,9 +103,8 @@ test_expect_success 'auto gc with too many loose objects does not attempt to cre\n '\n \n test_expect_success 'gc --no-quiet' '\n-\tgit -c gc.writeCommitGraph=true gc --no-quiet >stdout 2>stderr &&\n+\tGIT_PROGRESS_DELAY=0 git -c gc.writeCommitGraph=true gc --no-quiet >stdout 2>stderr &&\n \ttest_must_be_empty stdout &&\n-\ttest_line_count = 1 stderr &&\n \ttest_i18ngrep \"Computing commit graph generation numbers\" stderr\n '\n \n-- \ngitgitgadget\n\n"},{"id":"385713","messageId":"3c0c9675e125f9357aeadd76f290413aaa09e4cf.1573148818.git.gitgitgadget@gmail.com","threadId":"52193","inReplyTo":"pull.450.v3.git.1573148818.gitgitgadget@gmail.com","subject":"[PATCH v3 2/2] commit-graph: use start_delayed_progress()","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-07T17:46:58Z","receivedAt":"2019-11-07T17:47:05Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen writing a commit-graph, we show progress along several commit\nwalks. When we use start_delayed_progress(), the progress line will\nonly appear if that step takes a decent amount of time.\n\nHowever, one place was missed: computing generation numbers. This is\nnormally a very fast operation as all commits have been parsed in a\nprevious step. But, this is showing up for all users no matter how few\ncommits are being added.\n\nThe tests that check for the progress output have already been updated\nto use GIT_PROGRESS_DELAY=0 to force the expected output. However, there\nis one test in t6500-gc.sh that uses the test_terminal method. This\nmechanism does not preserve the GIT_PROGRESS_DELAY environment variable,\nso we need to modify check on the output. We still watch for the\n\"Enumerating objects\" progress but no longer look for \"Computing\ncommit graph generation numbers\".\n\nReported-by: ryenus \nSigned-off-by: Derrick Stolee \n---\n commit-graph.c | 2 +-\n t/t6500-gc.sh | 3 +--\n 2 files changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 0aea7b2ae5..071e1c6e9b 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1103,7 +1103,7 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx)\n \tstruct commit_list *list = NULL;\n \n \tif (ctx->report_progress)\n-\t\tctx->progress = start_progress(\n+\t\tctx->progress = start_delayed_progress(\n \t\t\t\t\t_(\"Computing commit graph generation numbers\"),\n \t\t\t\t\tctx->commits.nr);\n \tfor (i = 0; i < ctx->commits.nr; i++) {\ndiff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\nindex 7f79eedd1c..c68177510b 100755\n--- a/t/t6500-gc.sh\n+++ b/t/t6500-gc.sh\n@@ -111,8 +111,7 @@ test_expect_success 'gc --no-quiet' '\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 \ttest_must_be_empty stdout &&\n-\ttest_i18ngrep \"Enumerating objects\" stderr &&\n-\ttest_i18ngrep \"Computing commit graph generation numbers\" stderr\n+\ttest_i18ngrep \"Enumerating objects\" stderr\n '\n \n test_expect_success 'gc --quiet' '\n-- \ngitgitgadget\n"},{"id":"385739","messageId":"20191107212235.GB29042@sigill.intra.peff.net","threadId":"52193","inReplyTo":"656dba5afb818d0caa7616d0e58c9728803f8d04.1573148818.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 1/2] progress: create GIT_PROGRESS_DELAY","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-07T21:22:35Z","receivedAt":"2019-11-07T21:22:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 07, 2019 at 05:46:57PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> From: Derrick Stolee \n> \n> The start_delayed_progress() method is a preferred way to show\n> optional progress to users as it ignores steps that take less\n> than two seconds. However, this makes testing unreliable as tests\n> expect to be very fast.\n> \n> In addition, users may want to decrease or increase this time\n> interval depending on their preferences for terminal noise.\n> \n> Create the GIT_PROGRESS_DELAY environment variable to control\n> the delay set during start_delayed_progress(). Set the value\n> in some tests to guarantee their output remains consistent.\n\nThanks for wrapping this up. I obviously think this is a good direction\nto go. :) A few thoughts:\n\n> diff --git a/Documentation/git.txt b/Documentation/git.txt\n> index 9b82564d1a..1c420da208 100644\n> --- a/Documentation/git.txt\n> +++ b/Documentation/git.txt\n> @@ -544,6 +544,10 @@ other\n> \ta pager. See also the `core.pager` option in\n> \tlinkgit:git-config[1].\n> \n> +`GIT_PROGRESS_DELAY`::\n> +\tA number controlling how many seconds to delay before showing\n> +\toptional progress indicators. Defaults to 2.\n\nNot all progress meters use delay. I wonder if that might confuse some\nusers, who would try:\n\n GIT_PROGRESS_DELAY=10 git repack -ad\n\nor something, but still see \"Enumerating objects\".\n\nI guess the key word in your documentation is \"optional\", but maybe it\nneeds to be spelled out more clearly. I dunno.\n\n> @@ -269,7 +270,12 @@ static struct progress *start_progress_delay(const char *title, uint64_t total,\n> \n> struct progress *start_delayed_progress(const char *title, uint64_t total)\n> {\n> -\treturn start_progress_delay(title, total, 2, 0);\n> +\tstatic int delay_in_secs = -1;\n> +\n> +\tif (delay_in_secs < 0)\n> +\t\tdelay_in_secs = git_env_ulong(\"GIT_PROGRESS_DELAY\", 2);\n> +\n> +\treturn start_progress_delay(title, total, delay_in_secs, 0);\n> }\n\nYou asked earlier if it was worth memo-izing the git_env_ulong() call\nlike this. I suspect it doesn't matter much either way, since progress\nonly starts and stops a few times in a given program. But I'm certainly\nhappy with it this way, as it matches most other environment lookups.\n\n-Peff\n"},{"id":"385740","messageId":"20191107212614.GC29042@sigill.intra.peff.net","threadId":"52193","inReplyTo":"3c0c9675e125f9357aeadd76f290413aaa09e4cf.1573148818.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 2/2] commit-graph: use start_delayed_progress()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-07T21:26:14Z","receivedAt":"2019-11-07T21:26:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 07, 2019 at 05:46:58PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> From: Derrick Stolee \n> \n> When writing a commit-graph, we show progress along several commit\n> walks. When we use start_delayed_progress(), the progress line will\n> only appear if that step takes a decent amount of time.\n> \n> However, one place was missed: computing generation numbers. This is\n> normally a very fast operation as all commits have been parsed in a\n> previous step. But, this is showing up for all users no matter how few\n> commits are being added.\n\nThis part of the patch is a good thing, and obviously correct. But I\nwondered...\n\n> The tests that check for the progress output have already been updated\n> to use GIT_PROGRESS_DELAY=0 to force the expected output. However, there\n> is one test in t6500-gc.sh that uses the test_terminal method. This\n> mechanism does not preserve the GIT_PROGRESS_DELAY environment variable,\n\nWhy doesn't GIT_PROGRESS_DELAY make it through? Overall it's not that\nbig a deal to me if it doesn't, but in this test:\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> \ttest_must_be_empty stdout &&\n> -\ttest_i18ngrep \"Enumerating objects\" stderr &&\n> -\ttest_i18ngrep \"Computing commit graph generation numbers\" stderr\n> +\ttest_i18ngrep \"Enumerating objects\" stderr\n> '\n\nWe're not actually checking anything related to gc.writeCommitGraph\nanymore.\n\n> so we need to modify check on the output. We still watch for the\n\nMinor typo: s/modify/& the/ or similar?\n\n-Peff\n"},{"id":"385908","messageId":"20191111142739.GL4348@szeder.dev","threadId":"52193","inReplyTo":"656dba5afb818d0caa7616d0e58c9728803f8d04.1573148818.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v3 1/2] progress: create GIT_PROGRESS_DELAY","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-11-11T14:27:39Z","receivedAt":"2019-11-11T14:27:45Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Thu, Nov 07, 2019 at 05:46:57PM +0000, Derrick Stolee via GitGitGadget wrote:\n> From: Derrick Stolee \n> \n> The start_delayed_progress() method is a preferred way to show\n> optional progress to users as it ignores steps that take less\n> than two seconds. However, this makes testing unreliable as tests\n> expect to be very fast.\n> \n> In addition, users may want to decrease or increase this time\n> interval depending on their preferences for terminal noise.\n> \n> Create the GIT_PROGRESS_DELAY environment variable to control\n> the delay set during start_delayed_progress(). Set the value\n> in some tests to guarantee their output remains consistent.\n> \n> Helped-by: Jeff King \n> Signed-off-by: Derrick Stolee \n> ---\n> Documentation/git.txt | 4 ++++\n> progress.c | 8 +++++++-\n> t/t5318-commit-graph.sh | 4 ++--\n> t/t6500-gc.sh | 3 +--\n> 4 files changed, 14 insertions(+), 5 deletions(-)\n> \n> diff --git a/Documentation/git.txt b/Documentation/git.txt\n> index 9b82564d1a..1c420da208 100644\n> --- a/Documentation/git.txt\n> +++ b/Documentation/git.txt\n> @@ -544,6 +544,10 @@ other\n> \ta pager. See also the `core.pager` option in\n> \tlinkgit:git-config[1].\n> \n> +`GIT_PROGRESS_DELAY`::\n> +\tA number controlling how many seconds to delay before showing\n> +\toptional progress indicators. Defaults to 2.\n> +\n> `GIT_EDITOR`::\n> \tThis environment variable overrides `$EDITOR` and `$VISUAL`.\n> \tIt is used by several Git commands when, on interactive mode,\n> diff --git a/progress.c b/progress.c\n> index 0063559aab..4ad1a3c6eb 100644\n> --- a/progress.c\n> +++ b/progress.c\n> @@ -14,6 +14,7 @@\n> #include \"strbuf.h\"\n> #include \"trace.h\"\n> #include \"utf8.h\"\n> +#include \"config.h\"\n> \n> #define TP_IDX_MAX 8\n> \n> @@ -269,7 +270,12 @@ static struct progress *start_progress_delay(const char *title, uint64_t total,\n> \n> struct progress *start_delayed_progress(const char *title, uint64_t total)\n> {\n> -\treturn start_progress_delay(title, total, 2, 0);\n> +\tstatic int delay_in_secs = -1;\n> +\n> +\tif (delay_in_secs < 0)\n> +\t\tdelay_in_secs = git_env_ulong(\"GIT_PROGRESS_DELAY\", 2);\n> +\n> +\treturn start_progress_delay(title, total, delay_in_secs, 0);\n> }\n\nNote that there is the similar start_delayed_sparse_progress()\nfunction, which should respect GIT_PROGRESS_DELAY as well.\n\n> diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\n> index c0f04dc6b0..7f79eedd1c 100755\n> --- a/t/t6500-gc.sh\n> +++ b/t/t6500-gc.sh\n> @@ -103,9 +103,8 @@ test_expect_success 'auto gc with too many loose objects does not attempt to cre\n> '\n> \n> test_expect_success 'gc --no-quiet' '\n> -\tgit -c gc.writeCommitGraph=true gc --no-quiet >stdout 2>stderr &&\n> +\tGIT_PROGRESS_DELAY=0 git -c gc.writeCommitGraph=true gc --no-quiet >stdout 2>stderr &&\n> \ttest_must_be_empty stdout &&\n> -\ttest_line_count = 1 stderr &&\n\nGood, I'm glad to see this \"how many progress lines did we print?\"\ncheck gone.\n\n> \ttest_i18ngrep \"Computing commit graph generation numbers\" stderr\n> '\n> \n> -- \n> gitgitgadget\n> \n"},{"id":"386732","messageId":"pull.450.v4.git.1574351516.gitgitgadget@gmail.com","threadId":"52193","inReplyTo":"pull.450.v3.git.1573148818.gitgitgadget@gmail.com","subject":"[PATCH v4 0/2] commit-graph: use start_delayed_progress()","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-21T15:51:54Z","receivedAt":"2019-11-21T15:52:01Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"Thanks, Ryenus, for reporting this problem.\n\nUpdate in V3:\n\nBased on our discussion, I've added the suggested GIT_PROGRESS_DELAY\nenvironment variable. This allowed the existing tests to stick around with\none exception in the GC tests. The test remains, but we can no longer look\nat the commit-graph output.\n\nUpdate in V4:\n\nThe GIT_PROGRESS_DELAY check is extracted to a helper method so it can be\nused with start_delayed_sparse_progress().\n\nDerrick Stolee (2):\n progress: create GIT_PROGRESS_DELAY\n commit-graph: use start_delayed_progress()\n\n Documentation/git.txt | 4 ++++\n commit-graph.c | 2 +-\n progress.c | 15 +++++++++++++--\n t/t5318-commit-graph.sh | 4 ++--\n t/t6500-gc.sh | 6 ++----\n 5 files changed, 22 insertions(+), 9 deletions(-)\n\n\nbase-commit: da72936f544fec5a335e66432610e4cef4430991\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-450%2Fderrickstolee%2Fcommit-graph-progress-fix-v4\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-450/derrickstolee/commit-graph-progress-fix-v4\nPull-Request: https://github.com/gitgitgadget/git/pull/450\n\nRange-diff vs v3:\n\n 1: 656dba5afb ! 1: a7acdf9c8f progress: create GIT_PROGRESS_DELAY\n @@ -44,19 +44,35 @@\n #define TP_IDX_MAX 8\n \n @@\n + \treturn progress;\n + }\n \n - struct progress *start_delayed_progress(const char *title, uint64_t total)\n - {\n --\treturn start_progress_delay(title, total, 2, 0);\n ++static int get_default_delay(void)\n ++{\n +\tstatic int delay_in_secs = -1;\n +\n +\tif (delay_in_secs < 0)\n +\t\tdelay_in_secs = git_env_ulong(\"GIT_PROGRESS_DELAY\", 2);\n +\n -+\treturn start_progress_delay(title, total, delay_in_secs, 0);\n ++\treturn delay_in_secs;\n ++}\n ++\n + struct progress *start_delayed_progress(const char *title, uint64_t total)\n + {\n +-\treturn start_progress_delay(title, total, 2, 0);\n ++\treturn start_progress_delay(title, total, get_default_delay(), 0);\n }\n \n struct progress *start_progress(const char *title, uint64_t total)\n +@@\n + struct progress *start_delayed_sparse_progress(const char *title,\n + \t\t\t\t\t uint64_t total)\n + {\n +-\treturn start_progress_delay(title, total, 2, 1);\n ++\treturn start_progress_delay(title, total, get_default_delay(), 1);\n + }\n + \n + static void finish_if_sparse(struct progress *progress)\n \n diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\n --- a/t/t5318-commit-graph.sh\n 2: 3c0c9675e1 = 2: e62dcc1ce5 commit-graph: use start_delayed_progress()\n\n-- \ngitgitgadget\n"},{"id":"386733","messageId":"e62dcc1ce503311c6c11452862f384492965a35f.1574351516.git.gitgitgadget@gmail.com","threadId":"52193","inReplyTo":"pull.450.v4.git.1574351516.gitgitgadget@gmail.com","subject":"[PATCH v4 2/2] commit-graph: use start_delayed_progress()","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-21T15:51:56Z","receivedAt":"2019-11-21T15:52:03Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen writing a commit-graph, we show progress along several commit\nwalks. When we use start_delayed_progress(), the progress line will\nonly appear if that step takes a decent amount of time.\n\nHowever, one place was missed: computing generation numbers. This is\nnormally a very fast operation as all commits have been parsed in a\nprevious step. But, this is showing up for all users no matter how few\ncommits are being added.\n\nThe tests that check for the progress output have already been updated\nto use GIT_PROGRESS_DELAY=0 to force the expected output. However, there\nis one test in t6500-gc.sh that uses the test_terminal method. This\nmechanism does not preserve the GIT_PROGRESS_DELAY environment variable,\nso we need to modify check on the output. We still watch for the\n\"Enumerating objects\" progress but no longer look for \"Computing\ncommit graph generation numbers\".\n\nReported-by: ryenus \nSigned-off-by: Derrick Stolee \n---\n commit-graph.c | 2 +-\n t/t6500-gc.sh | 3 +--\n 2 files changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 0aea7b2ae5..071e1c6e9b 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1103,7 +1103,7 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx)\n \tstruct commit_list *list = NULL;\n \n \tif (ctx->report_progress)\n-\t\tctx->progress = start_progress(\n+\t\tctx->progress = start_delayed_progress(\n \t\t\t\t\t_(\"Computing commit graph generation numbers\"),\n \t\t\t\t\tctx->commits.nr);\n \tfor (i = 0; i < ctx->commits.nr; i++) {\ndiff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\nindex 7f79eedd1c..c68177510b 100755\n--- a/t/t6500-gc.sh\n+++ b/t/t6500-gc.sh\n@@ -111,8 +111,7 @@ test_expect_success 'gc --no-quiet' '\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 \ttest_must_be_empty stdout &&\n-\ttest_i18ngrep \"Enumerating objects\" stderr &&\n-\ttest_i18ngrep \"Computing commit graph generation numbers\" stderr\n+\ttest_i18ngrep \"Enumerating objects\" stderr\n '\n \n test_expect_success 'gc --quiet' '\n-- \ngitgitgadget\n"},{"id":"386734","messageId":"a7acdf9c8f8c85f9f39750315716f21e83ce67c6.1574351516.git.gitgitgadget@gmail.com","threadId":"52193","inReplyTo":"pull.450.v4.git.1574351516.gitgitgadget@gmail.com","subject":"[PATCH v4 1/2] progress: create GIT_PROGRESS_DELAY","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-21T15:51:55Z","receivedAt":"2019-11-21T15:52:03Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe start_delayed_progress() method is a preferred way to show\noptional progress to users as it ignores steps that take less\nthan two seconds. However, this makes testing unreliable as tests\nexpect to be very fast.\n\nIn addition, users may want to decrease or increase this time\ninterval depending on their preferences for terminal noise.\n\nCreate the GIT_PROGRESS_DELAY environment variable to control\nthe delay set during start_delayed_progress(). Set the value\nin some tests to guarantee their output remains consistent.\n\nHelped-by: Jeff King \nSigned-off-by: Derrick Stolee \n---\n Documentation/git.txt | 4 ++++\n progress.c | 15 +++++++++++++--\n t/t5318-commit-graph.sh | 4 ++--\n t/t6500-gc.sh | 3 +--\n 4 files changed, 20 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 9b82564d1a..1c420da208 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -544,6 +544,10 @@ other\n \ta pager. See also the `core.pager` option in\n \tlinkgit:git-config[1].\n \n+`GIT_PROGRESS_DELAY`::\n+\tA number controlling how many seconds to delay before showing\n+\toptional progress indicators. Defaults to 2.\n+\n `GIT_EDITOR`::\n \tThis environment variable overrides `$EDITOR` and `$VISUAL`.\n \tIt is used by several Git commands when, on interactive mode,\ndiff --git a/progress.c b/progress.c\nindex 0063559aab..19805ac646 100644\n--- a/progress.c\n+++ b/progress.c\n@@ -14,6 +14,7 @@\n #include \"strbuf.h\"\n #include \"trace.h\"\n #include \"utf8.h\"\n+#include \"config.h\"\n \n #define TP_IDX_MAX 8\n \n@@ -267,9 +268,19 @@ static struct progress *start_progress_delay(const char *title, uint64_t total,\n \treturn progress;\n }\n \n+static int get_default_delay(void)\n+{\n+\tstatic int delay_in_secs = -1;\n+\n+\tif (delay_in_secs < 0)\n+\t\tdelay_in_secs = git_env_ulong(\"GIT_PROGRESS_DELAY\", 2);\n+\n+\treturn delay_in_secs;\n+}\n+\n struct progress *start_delayed_progress(const char *title, uint64_t total)\n {\n-\treturn start_progress_delay(title, total, 2, 0);\n+\treturn start_progress_delay(title, total, get_default_delay(), 0);\n }\n \n struct progress *start_progress(const char *title, uint64_t total)\n@@ -294,7 +305,7 @@ struct progress *start_sparse_progress(const char *title, uint64_t total)\n struct progress *start_delayed_sparse_progress(const char *title,\n \t\t\t\t\t uint64_t total)\n {\n-\treturn start_progress_delay(title, total, 2, 1);\n+\treturn start_progress_delay(title, total, get_default_delay(), 1);\n }\n \n static void finish_if_sparse(struct progress *progress)\ndiff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\nindex d42b3efe39..0824857e1f 100755\n--- a/t/t5318-commit-graph.sh\n+++ b/t/t5318-commit-graph.sh\n@@ -132,7 +132,7 @@ test_expect_success 'commit-graph write progress off for redirected stderr' '\n \n test_expect_success 'commit-graph write force progress on for stderr' '\n \tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph write --progress 2>err &&\n+\tGIT_PROGRESS_DELAY=0 git commit-graph write --progress 2>err &&\n \ttest_file_not_empty err\n '\n \n@@ -150,7 +150,7 @@ test_expect_success 'commit-graph verify progress off for redirected stderr' '\n \n test_expect_success 'commit-graph verify force progress on for stderr' '\n \tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph verify --progress 2>err &&\n+\tGIT_PROGRESS_DELAY=0 git commit-graph verify --progress 2>err &&\n \ttest_file_not_empty err\n '\n \ndiff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\nindex c0f04dc6b0..7f79eedd1c 100755\n--- a/t/t6500-gc.sh\n+++ b/t/t6500-gc.sh\n@@ -103,9 +103,8 @@ test_expect_success 'auto gc with too many loose objects does not attempt to cre\n '\n \n test_expect_success 'gc --no-quiet' '\n-\tgit -c gc.writeCommitGraph=true gc --no-quiet >stdout 2>stderr &&\n+\tGIT_PROGRESS_DELAY=0 git -c gc.writeCommitGraph=true gc --no-quiet >stdout 2>stderr &&\n \ttest_must_be_empty stdout &&\n-\ttest_line_count = 1 stderr &&\n \ttest_i18ngrep \"Computing commit graph generation numbers\" stderr\n '\n \n-- \ngitgitgadget\n\n"},{"id":"386777","messageId":"20191121230326.GW23183@szeder.dev","threadId":"52193","inReplyTo":"20191107212614.GC29042@sigill.intra.peff.net","subject":"Re: [PATCH v3 2/2] commit-graph: use start_delayed_progress()","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-11-21T23:03:26Z","receivedAt":"2019-11-21T23:03:32Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Thu, Nov 07, 2019 at 04:26:14PM -0500, Jeff King wrote:\n> On Thu, Nov 07, 2019 at 05:46:58PM +0000, Derrick Stolee via GitGitGadget wrote:\n> \n> > From: Derrick Stolee \n> > \n> > When writing a commit-graph, we show progress along several commit\n> > walks. When we use start_delayed_progress(), the progress line will\n> > only appear if that step takes a decent amount of time.\n> > \n> > However, one place was missed: computing generation numbers. This is\n> > normally a very fast operation as all commits have been parsed in a\n> > previous step. But, this is showing up for all users no matter how few\n> > commits are being added.\n> \n> This part of the patch is a good thing, and obviously correct. But I\n> wondered...\n\nAgreed.\n\n> > The tests that check for the progress output have already been updated\n> > to use GIT_PROGRESS_DELAY=0 to force the expected output. However, there\n> > is one test in t6500-gc.sh that uses the test_terminal method. This\n> > mechanism does not preserve the GIT_PROGRESS_DELAY environment variable,\n> \n> Why doesn't GIT_PROGRESS_DELAY make it through? Overall it's not that\n> big a deal to me if it doesn't, but in this test:\n\nBut I was wondering this, too. If I run the following test:\n\n (\n write_script script <<-\\EOF &&\n echo \"GPD: $GIT_PROGRESS_DELAY\"\n EOF\n GIT_PROGRESS_DELAY=42 &&\n export GIT_PROGRESS_DELAY &&\n test_terminal ./script dummy-arg\n ) &&\n\nthen its output looks like this:\n\n + write_script script\n + GIT_PROGRESS_DELAY=42\n + export GIT_PROGRESS_DELAY\n + test_terminal ./script dummy-arg\n GPD: 42\n\nSo test_terminal does preserve GIT_PROGRESS_DELAY.\n\n"},{"id":"386805","messageId":"20191122071536.GC23225@sigill.intra.peff.net","threadId":"52193","inReplyTo":"a7acdf9c8f8c85f9f39750315716f21e83ce67c6.1574351516.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v4 1/2] progress: create GIT_PROGRESS_DELAY","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-22T07:15:36Z","receivedAt":"2019-11-22T07:15:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 21, 2019 at 03:51:55PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> @@ -267,9 +268,19 @@ static struct progress *start_progress_delay(const char *title, uint64_t total,\n> \treturn progress;\n> }\n> \n> +static int get_default_delay(void)\n> +{\n> +\tstatic int delay_in_secs = -1;\n> +\n> +\tif (delay_in_secs < 0)\n> +\t\tdelay_in_secs = git_env_ulong(\"GIT_PROGRESS_DELAY\", 2);\n> +\n> +\treturn delay_in_secs;\n> +}\n\nThanks, this factoring out looks good.\n\nSince the only callers of start_progress_delay() pass in either the\nresult of this function or \"0\", it _could_ become a bool flag and we\ncould just resolve it inside that function. But I don't think there's a\nbig advantage to doing so.\n\n> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\n> index d42b3efe39..0824857e1f 100755\n> --- a/t/t5318-commit-graph.sh\n> +++ b/t/t5318-commit-graph.sh\n> @@ -132,7 +132,7 @@ test_expect_success 'commit-graph write progress off for redirected stderr' '\n> \n> test_expect_success 'commit-graph write force progress on for stderr' '\n> \tcd \"$TRASH_DIRECTORY/full\" &&\n> -\tgit commit-graph write --progress 2>err &&\n> +\tGIT_PROGRESS_DELAY=0 git commit-graph write --progress 2>err &&\n> \ttest_file_not_empty err\n> '\n\nI'm tempted to suggest that we should just set GIT_PROGRESS_DELAY=0 for\nthe whole test suite. That would root out any potentially racy tests,\nthough given that the default is 2 seconds, it would probably take a\npretty horribly loaded system to trigger such a race.\n\nCuriously, doing this:\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 46c4440843..63357ed6c4 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -423,6 +423,9 @@ export EDITOR\n GIT_TRACE_BARE=1\n export GIT_TRACE_BARE\n \n+GIT_PROGRESS_DELAY=0\n+export GIT_PROGRESS_DELAY\n+\n check_var_migration () {\n \t# the warnings and hints given from this helper depends\n \t# on end-user settings, which will disrupt the self-test\n\nresults in a few test failures. It looks like unpack-trees is eager to\nprint the \"Updating files\" progress meter even when stderr is redirected\nto a file. Which seems like a bug. I don't mind if we put that off for\nnow, though, in order to get your fix here merged more quickly.\n\n-Peff\n"},{"id":"386806","messageId":"20191122071716.GD23225@sigill.intra.peff.net","threadId":"52193","inReplyTo":"e62dcc1ce503311c6c11452862f384492965a35f.1574351516.git.gitgitgadget@gmail.com","subject":"Re: [PATCH v4 2/2] commit-graph: use start_delayed_progress()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-22T07:17:16Z","receivedAt":"2019-11-22T07:17:19Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Nov 21, 2019 at 03:51:56PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> The tests that check for the progress output have already been updated\n> to use GIT_PROGRESS_DELAY=0 to force the expected output. However, there\n> is one test in t6500-gc.sh that uses the test_terminal method. This\n> mechanism does not preserve the GIT_PROGRESS_DELAY environment variable,\n> so we need to modify check on the output. We still watch for the\n> \"Enumerating objects\" progress but no longer look for \"Computing\n> commit graph generation numbers\".\n\nI'm still puzzled by this paragraph. If I replace the test hunk in your\npatch with this:\n\ndiff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\nindex 7f79eedd1c..0a69a67117 100755\n--- a/t/t6500-gc.sh\n+++ b/t/t6500-gc.sh\n@@ -109,7 +109,8 @@ test_expect_success 'gc --no-quiet' '\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+\ttest_terminal env GIT_PROGRESS_DELAY=0 \\\n+\t\tgit -c gc.writeCommitGraph=true gc --no-quiet >stdout 2>stderr &&\n \ttest_must_be_empty stdout &&\n \ttest_i18ngrep \"Enumerating objects\" stderr &&\n \ttest_i18ngrep \"Computing commit graph generation numbers\" stderr\n\nthe test works fine for me.\n\n-Peff\n"},{"id":"387012","messageId":"9d298c6d-303a-eecd-e79d-b90010fdf3d0@gmail.com","threadId":"52193","inReplyTo":"20191122071716.GD23225@sigill.intra.peff.net","subject":"Re: [PATCH v4 2/2] commit-graph: use start_delayed_progress()","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-11-25T18:57:05Z","receivedAt":"2019-11-25T18:57:09Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/22/2019 2:17 AM, Jeff King wrote:\n> On Thu, Nov 21, 2019 at 03:51:56PM +0000, Derrick Stolee via GitGitGadget wrote:\n> \n>> The tests that check for the progress output have already been updated\n>> to use GIT_PROGRESS_DELAY=0 to force the expected output. However, there\n>> is one test in t6500-gc.sh that uses the test_terminal method. This\n>> mechanism does not preserve the GIT_PROGRESS_DELAY environment variable,\n>> so we need to modify check on the output. We still watch for the\n>> \"Enumerating objects\" progress but no longer look for \"Computing\n>> commit graph generation numbers\".\n> \n> I'm still puzzled by this paragraph. If I replace the test hunk in your\n> patch with this:\n> \n> diff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\n> index 7f79eedd1c..0a69a67117 100755\n> --- a/t/t6500-gc.sh\n> +++ b/t/t6500-gc.sh\n> @@ -109,7 +109,8 @@ test_expect_success 'gc --no-quiet' '\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> +\ttest_terminal env GIT_PROGRESS_DELAY=0 \\\n> +\t\tgit -c gc.writeCommitGraph=true gc --no-quiet >stdout 2>stderr &&\n> \ttest_must_be_empty stdout &&\n> \ttest_i18ngrep \"Enumerating objects\" stderr &&\n> \ttest_i18ngrep \"Computing commit graph generation numbers\" stderr\n> \n> the test works fine for me.\n\nThanks! I was having trouble getting that to work by only\ninserting \"GIT_PROCESS_DELAY=0\" somewhere. The added \"env\"\nis what I was missing.\n\n-Stolee\n\n"},{"id":"387016","messageId":"a7acdf9c8f8c85f9f39750315716f21e83ce67c6.1574717303.git.gitgitgadget@gmail.com","threadId":"52193","inReplyTo":"pull.450.v5.git.1574717303.gitgitgadget@gmail.com","subject":"[PATCH v5 1/2] progress: create GIT_PROGRESS_DELAY","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-25T21:28:22Z","receivedAt":"2019-11-25T21:28:30Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nThe start_delayed_progress() method is a preferred way to show\noptional progress to users as it ignores steps that take less\nthan two seconds. However, this makes testing unreliable as tests\nexpect to be very fast.\n\nIn addition, users may want to decrease or increase this time\ninterval depending on their preferences for terminal noise.\n\nCreate the GIT_PROGRESS_DELAY environment variable to control\nthe delay set during start_delayed_progress(). Set the value\nin some tests to guarantee their output remains consistent.\n\nHelped-by: Jeff King \nSigned-off-by: Derrick Stolee \n---\n Documentation/git.txt | 4 ++++\n progress.c | 15 +++++++++++++--\n t/t5318-commit-graph.sh | 4 ++--\n t/t6500-gc.sh | 3 +--\n 4 files changed, 20 insertions(+), 6 deletions(-)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 9b82564d1a..1c420da208 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -544,6 +544,10 @@ other\n \ta pager. See also the `core.pager` option in\n \tlinkgit:git-config[1].\n \n+`GIT_PROGRESS_DELAY`::\n+\tA number controlling how many seconds to delay before showing\n+\toptional progress indicators. Defaults to 2.\n+\n `GIT_EDITOR`::\n \tThis environment variable overrides `$EDITOR` and `$VISUAL`.\n \tIt is used by several Git commands when, on interactive mode,\ndiff --git a/progress.c b/progress.c\nindex 0063559aab..19805ac646 100644\n--- a/progress.c\n+++ b/progress.c\n@@ -14,6 +14,7 @@\n #include \"strbuf.h\"\n #include \"trace.h\"\n #include \"utf8.h\"\n+#include \"config.h\"\n \n #define TP_IDX_MAX 8\n \n@@ -267,9 +268,19 @@ static struct progress *start_progress_delay(const char *title, uint64_t total,\n \treturn progress;\n }\n \n+static int get_default_delay(void)\n+{\n+\tstatic int delay_in_secs = -1;\n+\n+\tif (delay_in_secs < 0)\n+\t\tdelay_in_secs = git_env_ulong(\"GIT_PROGRESS_DELAY\", 2);\n+\n+\treturn delay_in_secs;\n+}\n+\n struct progress *start_delayed_progress(const char *title, uint64_t total)\n {\n-\treturn start_progress_delay(title, total, 2, 0);\n+\treturn start_progress_delay(title, total, get_default_delay(), 0);\n }\n \n struct progress *start_progress(const char *title, uint64_t total)\n@@ -294,7 +305,7 @@ struct progress *start_sparse_progress(const char *title, uint64_t total)\n struct progress *start_delayed_sparse_progress(const char *title,\n \t\t\t\t\t uint64_t total)\n {\n-\treturn start_progress_delay(title, total, 2, 1);\n+\treturn start_progress_delay(title, total, get_default_delay(), 1);\n }\n \n static void finish_if_sparse(struct progress *progress)\ndiff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh\nindex d42b3efe39..0824857e1f 100755\n--- a/t/t5318-commit-graph.sh\n+++ b/t/t5318-commit-graph.sh\n@@ -132,7 +132,7 @@ test_expect_success 'commit-graph write progress off for redirected stderr' '\n \n test_expect_success 'commit-graph write force progress on for stderr' '\n \tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph write --progress 2>err &&\n+\tGIT_PROGRESS_DELAY=0 git commit-graph write --progress 2>err &&\n \ttest_file_not_empty err\n '\n \n@@ -150,7 +150,7 @@ test_expect_success 'commit-graph verify progress off for redirected stderr' '\n \n test_expect_success 'commit-graph verify force progress on for stderr' '\n \tcd \"$TRASH_DIRECTORY/full\" &&\n-\tgit commit-graph verify --progress 2>err &&\n+\tGIT_PROGRESS_DELAY=0 git commit-graph verify --progress 2>err &&\n \ttest_file_not_empty err\n '\n \ndiff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\nindex c0f04dc6b0..7f79eedd1c 100755\n--- a/t/t6500-gc.sh\n+++ b/t/t6500-gc.sh\n@@ -103,9 +103,8 @@ test_expect_success 'auto gc with too many loose objects does not attempt to cre\n '\n \n test_expect_success 'gc --no-quiet' '\n-\tgit -c gc.writeCommitGraph=true gc --no-quiet >stdout 2>stderr &&\n+\tGIT_PROGRESS_DELAY=0 git -c gc.writeCommitGraph=true gc --no-quiet >stdout 2>stderr &&\n \ttest_must_be_empty stdout &&\n-\ttest_line_count = 1 stderr &&\n \ttest_i18ngrep \"Computing commit graph generation numbers\" stderr\n '\n \n-- \ngitgitgadget\n\n"},{"id":"387017","messageId":"efe1419dc6cfd18f5a91061a4a61cc7228406f93.1574717303.git.gitgitgadget@gmail.com","threadId":"52193","inReplyTo":"pull.450.v5.git.1574717303.gitgitgadget@gmail.com","subject":"[PATCH v5 2/2] commit-graph: use start_delayed_progress()","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-25T21:28:23Z","receivedAt":"2019-11-25T21:28:30Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"From: Derrick Stolee \n\nWhen writing a commit-graph, we show progress along several commit\nwalks. When we use start_delayed_progress(), the progress line will\nonly appear if that step takes a decent amount of time.\n\nHowever, one place was missed: computing generation numbers. This is\nnormally a very fast operation as all commits have been parsed in a\nprevious step. But, this is showing up for all users no matter how few\ncommits are being added.\n\nThe tests that check for the progress output have already been updated\nto use GIT_PROGRESS_DELAY=0 to force the expected output.\n\nHelped-by: Jeff King \nReported-by: ryenus \nSigned-off-by: Derrick Stolee \n---\n commit-graph.c | 2 +-\n t/t6500-gc.sh | 3 ++-\n 2 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/commit-graph.c b/commit-graph.c\nindex 0aea7b2ae5..071e1c6e9b 100644\n--- a/commit-graph.c\n+++ b/commit-graph.c\n@@ -1103,7 +1103,7 @@ static void compute_generation_numbers(struct write_commit_graph_context *ctx)\n \tstruct commit_list *list = NULL;\n \n \tif (ctx->report_progress)\n-\t\tctx->progress = start_progress(\n+\t\tctx->progress = start_delayed_progress(\n \t\t\t\t\t_(\"Computing commit graph generation numbers\"),\n \t\t\t\t\tctx->commits.nr);\n \tfor (i = 0; i < ctx->commits.nr; i++) {\ndiff --git a/t/t6500-gc.sh b/t/t6500-gc.sh\nindex 7f79eedd1c..0a69a67117 100755\n--- a/t/t6500-gc.sh\n+++ b/t/t6500-gc.sh\n@@ -109,7 +109,8 @@ test_expect_success 'gc --no-quiet' '\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+\ttest_terminal env GIT_PROGRESS_DELAY=0 \\\n+\t\tgit -c gc.writeCommitGraph=true gc --no-quiet >stdout 2>stderr &&\n \ttest_must_be_empty stdout &&\n \ttest_i18ngrep \"Enumerating objects\" stderr &&\n \ttest_i18ngrep \"Computing commit graph generation numbers\" stderr\n-- \ngitgitgadget\n"},{"id":"387018","messageId":"pull.450.v5.git.1574717303.gitgitgadget@gmail.com","threadId":"52193","inReplyTo":"pull.450.v4.git.1574351516.gitgitgadget@gmail.com","subject":"[PATCH v5 0/2] commit-graph: use start_delayed_progress()","fromName":"Derrick Stolee via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2019-11-25T21:28:21Z","receivedAt":"2019-11-25T21:28:30Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"Thanks, Ryenus, for reporting this problem.\n\nUpdate in V3:\n\nBased on our discussion, I've added the suggested GIT_PROGRESS_DELAY\nenvironment variable. This allowed the existing tests to stick around with\none exception in the GC tests. The test remains, but we can no longer look\nat the commit-graph output.\n\nUpdate in V4:\n\nThe GIT_PROGRESS_DELAY check is extracted to a helper method so it can be\nused with start_delayed_sparse_progress().\n\nUpdate in V5:\n\nI took Peff's advice for using \"env\" to use this delay in the GC test.\n\nDerrick Stolee (2):\n progress: create GIT_PROGRESS_DELAY\n commit-graph: use start_delayed_progress()\n\n Documentation/git.txt | 4 ++++\n commit-graph.c | 2 +-\n progress.c | 15 +++++++++++++--\n t/t5318-commit-graph.sh | 4 ++--\n t/t6500-gc.sh | 6 +++---\n 5 files changed, 23 insertions(+), 8 deletions(-)\n\n\nbase-commit: da72936f544fec5a335e66432610e4cef4430991\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-450%2Fderrickstolee%2Fcommit-graph-progress-fix-v5\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-450/derrickstolee/commit-graph-progress-fix-v5\nPull-Request: https://github.com/gitgitgadget/git/pull/450\n\nRange-diff vs v4:\n\n 1: a7acdf9c8f = 1: a7acdf9c8f progress: create GIT_PROGRESS_DELAY\n 2: e62dcc1ce5 ! 2: efe1419dc6 commit-graph: use start_delayed_progress()\n @@ -12,13 +12,9 @@\n commits are being added.\n \n The tests that check for the progress output have already been updated\n - to use GIT_PROGRESS_DELAY=0 to force the expected output. However, there\n - is one test in t6500-gc.sh that uses the test_terminal method. This\n - mechanism does not preserve the GIT_PROGRESS_DELAY environment variable,\n - so we need to modify check on the output. We still watch for the\n - \"Enumerating objects\" progress but no longer look for \"Computing\n - commit graph generation numbers\".\n + to use GIT_PROGRESS_DELAY=0 to force the expected output.\n \n + Helped-by: Jeff King \n Reported-by: ryenus \n Signed-off-by: Derrick Stolee \n \n @@ -39,12 +35,12 @@\n --- a/t/t6500-gc.sh\n +++ b/t/t6500-gc.sh\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 - \ttest_must_be_empty stdout &&\n --\ttest_i18ngrep \"Enumerating objects\" stderr &&\n --\ttest_i18ngrep \"Computing commit graph generation numbers\" stderr\n -+\ttest_i18ngrep \"Enumerating objects\" stderr\n '\n \n - test_expect_success 'gc --quiet' '\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 ++\ttest_terminal env GIT_PROGRESS_DELAY=0 \\\n ++\t\tgit -c gc.writeCommitGraph=true gc --no-quiet >stdout 2>stderr &&\n + \ttest_must_be_empty stdout &&\n + \ttest_i18ngrep \"Enumerating objects\" stderr &&\n + \ttest_i18ngrep \"Computing commit graph generation numbers\" stderr\n\n-- \ngitgitgadget\n"},{"id":"387080","messageId":"20191126122022.GB10290@sigill.intra.peff.net","threadId":"52193","inReplyTo":"pull.450.v5.git.1574717303.gitgitgadget@gmail.com","subject":"Re: [PATCH v5 0/2] commit-graph: use start_delayed_progress()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2019-11-26T12:20:22Z","receivedAt":"2019-11-26T12:20:25Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Nov 25, 2019 at 09:28:21PM +0000, Derrick Stolee via GitGitGadget wrote:\n\n> Update in V5:\n> \n> I took Peff's advice for using \"env\" to use this delay in the GC test.\n\nThanks. No further complaints from me. :)\n\nThere's an open issue to investigate unpack-trees sending progress to a\nnon-stderr tty (which you can see in the test suite by setting\nGIT_PROGRESS_DELAY=0), but I think that should be handled separately.\nMy limited digging suggests that it goes back to 2007 or earlier.\n\n-Peff\n"},{"id":"387087","messageId":"fe7c0e41-8379-4933-4ff1-026d3f1f8472@gmail.com","threadId":"52193","inReplyTo":"20191126122022.GB10290@sigill.intra.peff.net","subject":"Re: [PATCH v5 0/2] commit-graph: use start_delayed_progress()","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2019-11-26T15:39:14Z","receivedAt":"2019-11-26T15:39:21Z","isPatch":true,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/26/2019 7:20 AM, Jeff King wrote:\n> On Mon, Nov 25, 2019 at 09:28:21PM +0000, Derrick Stolee via GitGitGadget wrote:\n> \n>> Update in V5:\n>>\n>> I took Peff's advice for using \"env\" to use this delay in the GC test.\n> \n> Thanks. No further complaints from me. :)\n> \n> There's an open issue to investigate unpack-trees sending progress to a\n> non-stderr tty (which you can see in the test suite by setting\n> GIT_PROGRESS_DELAY=0), but I think that should be handled separately.\n> My limited digging suggests that it goes back to 2007 or earlier.\n\nThis is on my personal to-do list. Thanks!\n\n-Stolee\n\n"},{"id":"387334","messageId":"xmqq36e54eie.fsf@gitster-ct.c.googlers.com","threadId":"52193","inReplyTo":"fe7c0e41-8379-4933-4ff1-026d3f1f8472@gmail.com","subject":"Re: [PATCH v5 0/2] commit-graph: use start_delayed_progress()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-11-30T14:36:57Z","receivedAt":"2019-11-30T14:37:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> On 11/26/2019 7:20 AM, Jeff King wrote:\n>> On Mon, Nov 25, 2019 at 09:28:21PM +0000, Derrick Stolee via GitGitGadget wrote:\n>> \n>>> Update in V5:\n>>>\n>>> I took Peff's advice for using \"env\" to use this delay in the GC test.\n>> \n>> Thanks. No further complaints from me. :)\n>> \n>> There's an open issue to investigate unpack-trees sending progress to a\n>> non-stderr tty (which you can see in the test suite by setting\n>> GIT_PROGRESS_DELAY=0), but I think that should be handled separately.\n>> My limited digging suggests that it goes back to 2007 or earlier.\n>\n> This is on my personal to-do list. Thanks!\n\nThanks, all. It seems that this round is ready for 'next'?\n"},{"id":"387369","messageId":"20191201093308.GI23183@szeder.dev","threadId":"52193","inReplyTo":"xmqq36e54eie.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v5 0/2] commit-graph: use start_delayed_progress()","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2019-12-01T09:33:08Z","receivedAt":"2019-12-01T09:33:15Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Sat, Nov 30, 2019 at 06:36:57AM -0800, Junio C Hamano wrote:\n> Derrick Stolee writes:\n> \n> > On 11/26/2019 7:20 AM, Jeff King wrote:\n> >> On Mon, Nov 25, 2019 at 09:28:21PM +0000, Derrick Stolee via GitGitGadget wrote:\n> >> \n> >>> Update in V5:\n> >>>\n> >>> I took Peff's advice for using \"env\" to use this delay in the GC test.\n> >> \n> >> Thanks. No further complaints from me. :)\n\n> Thanks, all. It seems that this round is ready for 'next'?\n\nYes, I think it is.\n\n"}]}