git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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
> 
Previous: Taylor BlauNext: Derrick Stolee
Message 52 of 59 in “commit-graph: drop CHECK_OIDS, peel in callers”
  1. 0/8 commit-graph: drop CHECK_OIDS, peel in callersTaylor Blau, May 5, 2020
  2. 1/8 commit-graph.c: extract 'refs_cb_data'Taylor Blau, May 5, 2020
  3. 2/8 commit-graph.c: show progress of finding reachable commitsTaylor Blau, May 5, 2020
  4. Derrick StoleeMay 5, 2020
  5. Taylor BlauMay 5, 2020
  6. 3/8 commit-graph.c: peel refs in 'add_ref_to_set'Taylor Blau, May 5, 2020
  7. Jeff KingMay 7, 2020
  8. Taylor BlauMay 13, 2020
  9. Jeff KingMay 13, 2020
  10. 4/8 builtin/commit-graph.c: extract 'read_one_commit()'Taylor Blau, May 5, 2020
  11. Jeff KingMay 7, 2020
  12. Taylor BlauMay 13, 2020
  13. 5/8 builtin/commit-graph.c: dereference tags in builtinTaylor Blau, May 5, 2020
  14. Derrick StoleeMay 5, 2020
  15. Taylor BlauMay 5, 2020
  16. Jeff KingMay 7, 2020
  17. 6/8 commit-graph.c: simplify 'fill_oids_from_commits'Taylor Blau, May 5, 2020
  18. Derrick StoleeMay 5, 2020
  19. Jeff KingMay 7, 2020
  20. 7/8 t5318: reorder test below 'graph_read_expect'Taylor Blau, May 5, 2020
  21. 8/8 commit-graph: drop COMMIT_GRAPH_WRITE_CHECK_OIDS flagTaylor Blau, May 5, 2020
  22. Derrick StoleeMay 5, 2020
  23. Taylor BlauMay 5, 2020
  24. Jeff KingMay 7, 2020
  25. Taylor BlauMay 13, 2020
  26. Derrick StoleeMay 5, 2020
  27. Taylor BlauMay 5, 2020
  28. 0/8 commit-graph: drop CHECK_OIDS, peel in callersTaylor Blau, May 6, 2020
  29. 1/8 commit-graph.c: extract 'refs_cb_data'Taylor Blau, May 6, 2020
  30. 2/8 commit-graph.c: show progress of finding reachable commitsTaylor Blau, May 6, 2020
  31. 3/8 commit-graph.c: peel refs in 'add_ref_to_set'Taylor Blau, May 6, 2020
  32. 4/8 builtin/commit-graph.c: extract 'read_one_commit()'Taylor Blau, May 6, 2020
  33. 5/8 builtin/commit-graph.c: dereference tags in builtinTaylor Blau, May 6, 2020
  34. 6/8 commit-graph.c: simplify 'fill_oids_from_commits'Taylor Blau, May 6, 2020
  35. 7/8 t5318: reorder test below 'graph_read_expect'Taylor Blau, May 6, 2020
  36. 8/8 commit-graph: drop COMMIT_GRAPH_WRITE_CHECK_OIDS flagTaylor Blau, May 6, 2020
  37. Jeff KingMay 7, 2020
  38. 0/8 commit-graph: drop CHECK_OIDS, peel in callersTaylor Blau, May 13, 2020
  39. 1/8 commit-graph.c: extract 'refs_cb_data'Taylor Blau, May 13, 2020
  40. 2/8 commit-graph.c: show progress of finding reachable commitsTaylor Blau, May 13, 2020
  41. 3/8 commit-graph.c: peel refs in 'add_ref_to_set'Taylor Blau, May 13, 2020
  42. 4/8 builtin/commit-graph.c: extract 'read_one_commit()'Taylor Blau, May 13, 2020
  43. Jeff KingMay 14, 2020
  44. Taylor BlauMay 14, 2020
  45. Junio C HamanoMay 14, 2020
  46. Taylor BlauMay 18, 2020
  47. 5/8 builtin/commit-graph.c: dereference tags in builtinTaylor Blau, May 13, 2020
  48. Jeff KingMay 14, 2020
  49. Taylor BlauMay 14, 2020
  50. commit-graph: fix "Collecting commits from input" progress lineSZEDER Gábor, Jul 10, 2020
  51. Taylor BlauJul 10, 2020
  52. SZEDER GáborJul 15, 2020
  53. Derrick StoleeJul 15, 2020
  54. Junio C HamanoJul 15, 2020
  55. 6/8 commit-graph.c: simplify 'fill_oids_from_commits'Taylor Blau, May 13, 2020
  56. 7/8 t5318: reorder test below 'graph_read_expect'Taylor Blau, May 13, 2020
  57. 8/8 commit-graph: drop COMMIT_GRAPH_WRITE_CHECK_OIDS flagTaylor Blau, May 13, 2020
  58. Jeff KingMay 14, 2020
  59. Jeff KingMay 14, 2020

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.