[v2,0/8] commit-graph: drop CHECK_OIDS, peel in callers

Message

Taylor Blau May 6, 2020, 12:07 a.m. UTC

Hi,

Here's a small re-roll of my series to move peeling outside of the
commit-graph internals, and drop the CHECK_OIDS flag. This re-roll was
promised in [1], and only updates the messages used in the new progress
meters. For convenience, a range-diff from v1 is included below.

Thanks in advance for another review.

[1]: https://lore.kernel.org/git/[e-mail]/

Taylor Blau (8):
  commit-graph.c: extract 'refs_cb_data'
  commit-graph.c: show progress of finding reachable commits
  commit-graph.c: peel refs in 'add_ref_to_set'
  builtin/commit-graph.c: extract 'read_one_commit()'
  builtin/commit-graph.c: dereference tags in builtin
  commit-graph.c: simplify 'fill_oids_from_commits'
  t5318: reorder test below 'graph_read_expect'
  commit-graph: drop COMMIT_GRAPH_WRITE_CHECK_OIDS flag

 Documentation/git-commit-graph.txt |  6 ++-
 builtin/commit-graph.c             | 73 ++++++++++++++++++++----------
 commit-graph.c                     | 61 +++++++++++--------------
 commit-graph.h                     | 10 ++--
 t/t5318-commit-graph.sh            | 25 ++++++----
 5 files changed, 98 insertions(+), 77 deletions(-)

Range-diff against v1:
1:  5bdbeaf374 ! 1:  cb56a9a73b commit-graph.c: show progress of finding reachable commits
    @@ commit-graph.c: int write_commit_graph_reachable(struct object_directory *odb,
      	data.commits = &commits;
     +	if (flags & COMMIT_GRAPH_WRITE_PROGRESS)
     +		data.progress = start_delayed_progress(
    -+			_("Finding reachable commits"), 0);
    ++			_("Collecting referenced commits"), 0);

      	for_each_ref(add_ref_to_set, &data);
      	result = write_commit_graph(odb, NULL, &commits,
2:  5ff56feab5 = 2:  85c388a077 commit-graph.c: peel refs in 'add_ref_to_set'
3:  9ae8745dc0 = 3:  cef441b465 builtin/commit-graph.c: extract 'read_one_commit()'
4:  513a634f14 ! 4:  d449d83ce2 builtin/commit-graph.c: dereference tags in builtin
    @@ builtin/commit-graph.c: static int graph_write(int argc, const char **argv)
      			flags |= COMMIT_GRAPH_WRITE_CHECK_OIDS;
     +			if (opts.progress)
     +				progress = start_delayed_progress(
    -+					_("Analyzing commits from stdin"), 0);
    ++					_("Collecting commits from input"), 0);
      		}

      		while (strbuf_getline(&buf, stdin) != EOF) {
5:  7e9d8c1f1a = 5:  61887870c7 commit-graph.c: simplify 'fill_oids_from_commits'
6:  f2f018b54c = 6:  e393b16097 t5318: reorder test below 'graph_read_expect'
7:  6c2d130b0c ! 7:  ad373f05ff commit-graph: drop COMMIT_GRAPH_WRITE_CHECK_OIDS flag
    @@ builtin/commit-graph.c: static int graph_write(int argc, const char **argv)
     -			flags |= COMMIT_GRAPH_WRITE_CHECK_OIDS;
      			if (opts.progress)
      				progress = start_delayed_progress(
    - 					_("Analyzing commits from stdin"), 0);
    + 					_("Collecting commits from input"), 0);

      ## commit-graph.c ##
     @@ commit-graph.c: struct write_commit_graph_context {
--
2.26.0.113.ge9739cdccc

Comments

Jeff King May 7, 2020, 8:42 p.m. UTC | #1

On Tue, May 05, 2020 at 06:07:09PM -0600, Taylor Blau wrote:

> Here's a small re-roll of my series to move peeling outside of the
> commit-graph internals, and drop the CHECK_OIDS flag. This re-roll was
> promised in [1], and only updates the messages used in the new progress
> meters. For convenience, a range-diff from v1 is included below.

I just reviewed v1. :) But I think the revised progress messages in v2
are much better.

The overall direction looks very good to me. I left a couple of
comments. Nothing too major, but I think there are some things worth
addressing (leaks, and more robust error checking in the final patch).

-Peff