Subject: [PATCH v4 00/11] Commit-graph write refactor (was: Create commit-graph file format v2)

From: "Derrick Stolee via GitGitGadget" <[email-masked]>
Date: Thu, 09 May 2019 07:22:30 -0700 (PDT)

This series replaces ds/commit-graph-file-v2, and I'm using the same
gitgitgadget PR to continue the version numbers and hopefully make that
clear. This is a slight modification on patches 1-11 from the incremental
file format RFC [0].

The commit-graph feature is growing, thanks to all of the contributions by
several community members. This also means that the write_commit_graph()
method is a bit unwieldy now. This series refactors that method to use a
write_commit_graph_context struct that is passed between several smaller
methods. The final result should be a write_commit_graph() method that has a
clear set of steps. Future changes should then be easier to understand.

 * Patches 1-4: these are small changes which either fix issues or just
   provide clean-up. These are mostly borrowed from
   ds/commit-graph-format-v2. 

 * Patches 5-11: these provide a non-functional refactor of
   write_commit_graph() into several methods using a "struct
   write_commit_graph_context" to share across the methods.

Updates to commits previously in this thread:

 * "commit-graph: remove Future Work section" no longer says that 'verify'
   takes as long as 'write'. [1]

 * "commit-graph: return with errors during write" now has a test to check
   we don't die(). [2]

Ævar: Looking at the old thread, I only saw two comments that still apply to
this series [1] [2]. Please point me to any comments I have missed.

Thanks, -Stolee

[0] https://public-inbox.org/git/[email-masked]/

[1] https://public-inbox.org/git/[email-masked]/

[2] https://public-inbox.org/git/[email-masked]/

Derrick Stolee (11):
  commit-graph: fix the_repository reference
  commit-graph: return with errors during write
  commit-graph: collapse parameters into flags
  commit-graph: remove Future Work section
  commit-graph: create write_commit_graph_context
  commit-graph: extract fill_oids_from_packs()
  commit-graph: extract fill_oids_from_commit_hex()
  commit-graph: extract fill_oids_from_all_packs()
  commit-graph: extract count_distinct_commits()
  commit-graph: extract copy_oids_to_commits()
  commit-graph: extract write_commit_graph_file()

 Documentation/technical/commit-graph.txt |  17 -
 builtin/commit-graph.c                   |  21 +-
 builtin/commit.c                         |   5 +-
 builtin/gc.c                             |   7 +-
 commit-graph.c                           | 607 +++++++++++++----------
 commit-graph.h                           |  14 +-
 commit.c                                 |   2 +-
 t/t5318-commit-graph.sh                  |   8 +
 8 files changed, 371 insertions(+), 310 deletions(-)

base-commit: 93b4405ffe4ad9308740e7c1c71383bfc369baaa
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-112%2Fderrickstolee%2Fgraph%2Fv2-head-v4
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-112/derrickstolee/graph/v2-head-v4
Pull-Request: https://github.com/gitgitgadget/git/pull/112

Range-diff vs v3:

  -:  ---------- >  1:  0be7713a25 commit-graph: fix the_repository reference
  1:  91f300ec0a !  2:  a4082b827e commit-graph: return with errors during write
     @@ -253,3 +253,22 @@
       
       int verify_commit_graph(struct repository *r, struct commit_graph *g);
       
     +
     + diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh
     + --- a/t/t5318-commit-graph.sh
     + +++ b/t/t5318-commit-graph.sh
     +@@
     + 	test_path_is_file info/commit-graph
     + '
     + 
     ++test_expect_success 'close with correct error on bad input' '
     ++	cd "$TRASH_DIRECTORY/full" &&
     ++	echo doesnotexist >in &&
     ++	{ git commit-graph write --stdin-packs <in 2>stderr; ret=$?; } &&
     ++	test "$ret" = 1 &&
     ++	test_i18ngrep "error adding pack" stderr
     ++'
     ++
     + test_expect_success 'create commits and repack' '
     + 	cd "$TRASH_DIRECTORY/full" &&
     + 	for i in $(test_seq 3)
  2:  924b22f990 =  3:  469d0c9a32 commit-graph: collapse parameters into flags
  3:  8446011a43 <  -:  ---------- commit-graph: create new version parameter
  4:  6a0e99f9f9 <  -:  ---------- commit-graph: add --version=<n> option
  5:  cca8267dfe <  -:  ---------- commit-graph: implement file format version 2
  6:  e72bca6c78 !  4:  130007d0e1 commit-graph: remove Future Work section
     @@ -12,9 +12,8 @@
      
          It is unlikely that we will ever send a commit-graph file
          as part of the protocol, since we would need to verify the
     -    data, and that is as expensive as writing a commit-graph from
     -    scratch. If we want to start trusting remote content, then
     -    that item can be investigated again.
     +    data, and that is expensive. If we want to start trusting
     +    remote content, then that item can be investigated again.
      
          While there is more work to be done on the feature, having
          a section of the docs devoted to a TODO list is wasteful and
  -:  ---------- >  5:  0ca4e18e98 commit-graph: create write_commit_graph_context
  -:  ---------- >  6:  30c1b618b1 commit-graph: extract fill_oids_from_packs()
  -:  ---------- >  7:  8cb2613dfa commit-graph: extract fill_oids_from_commit_hex()
  -:  ---------- >  8:  8f7129672a commit-graph: extract fill_oids_from_all_packs()
  -:  ---------- >  9:  a37548745b commit-graph: extract count_distinct_commits()
  -:  ---------- > 10:  57366ffdaa commit-graph: extract copy_oids_to_commits()
  -:  ---------- > 11:  fc81c8946d commit-graph: extract write_commit_graph_file()

-- 
gitgitgadget