Re: [PATCH] commit-graph: fix "Collecting commits from input" progress line
- From
SZEDER Gábor <szeder.dev@gmail.com>
- Date
- Jul 15, 2020, 18:33 UTC
- Message-ID
- <20200715183347.GH11341@szeder.dev>
- In-Reply-To
- <20200710190238.16376-1-szeder.dev@gmail.com>
Junio,
it looks like this patch hasn't been picked up, but it fixes a minor bug introduced in this release cycle.
On Fri, Jul 10, 2020 at 09:02:38PM +0200, SZEDER Gábor wrote:
Show 65 quoted lines
> To display a progress line while reading commits from standard input
> and looking them up, 5b6653e523 (builtin/commit-graph.c: dereference
> tags in builtin, 2020-05-13) should have added a pair of
> start_delayed_progress() and stop_progress() calls around the loop
> reading stdin. Alas, the stop_progress() call ended up at the wrong
> place, after write_commit_graph(), which does all the commit-graph
> computation and writing, and has several progress lines of its own.
> Consequintly, that new
>
> Collecting commits from input: 1234
>
> progress line is overwritten by the first progress line shown by
> write_commit_graph(), and its final "done" line is shown last, after
> everything is finished:
>
> $ { sleep 3 ; git rev-list -3 HEAD ; sleep 1 ; } | ~/src/git/git commit-graph write --stdin-commits
> Expanding reachable commits in commit graph: 873402, done.
> Writing out commit graph in 4 passes: 100% (3493608/3493608), done.
> Collecting commits from input: 3, done.
>
> Furthermore, that stop_progress() call was added after the 'cleanup'
> label, where that loop reading stdin jumps in case of an error. In
> case of invalid input this then results in the "done" line shown after
> the error message:
>
> $ { sleep 3 ; git rev-list -3 HEAD ; echo junk ; } | ~/src/git/git commit-graph write --stdin-commits
> error: unexpected non-hex object ID: junk
> Collecting commits from input: 3, done.
>
> Move that stop_progress() call to the right place.
>
> While at it, drop the unnecessary 'if (progress)' condition protecting
> the stop_progress() call, because that function is prepared to handle
> a NULL progress struct.
>
> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
> ---
> builtin/commit-graph.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c
> index 75455da138..796954da60 100644
> --- a/builtin/commit-graph.c
> +++ b/builtin/commit-graph.c
> @@ -251,7 +251,7 @@ static int graph_write(int argc, const char **argv)
> }
> }
>
> -
> + stop_progress(&progress);
> }
>
> if (write_commit_graph(odb,
> @@ -264,8 +264,6 @@ static int graph_write(int argc, const char **argv)
> cleanup:
> string_list_clear(&pack_indexes, 0);
> strbuf_release(&buf);
> - if (progress)
> - stop_progress(&progress);
> return result;
> }
>
> --
> 2.27.0.547.g4ba2d26563
>