[0/6] Create commit-graph file format v2

Message

Derrick Stolee via GitGitGadget Jan. 23, 2019, 9:59 p.m. UTC

The commit-graph file format has some shortcomings that were discussed
on-list:

 1. It doesn't use the 4-byte format ID from the_hash_algo.
    
    
 2. There is no way to change the reachability index from generation numbers
    to corrected commit date [1].
    
    
 3. The unused byte in the format could be used to signal the file is
    incremental, but current clients ignore the value even if it is
    non-zero.
    
    

This series adds a new version (2) to the commit-graph file. The fifth byte
already specified the file format, so existing clients will gracefully
respond to files with a different version number. The only real change now
is that the header takes 12 bytes instead of 8, due to using the 4-byte
format ID for the hash algorithm.

The new bytes reserved for the reachability index version and incremental
file formats are now expected to be equal to the defaults. When we update
these values to be flexible in the future, if a client understands
commit-graph v2 but not those new values, then it will fail gracefully.

This series is based on ab/commit-graph-write-progress and bc/sha-256.

Thanks, -Stolee

[1] 
https://public-inbox.org/git/[e-mail]/

Derrick Stolee (6):
  commit-graph: return with errors during write
  commit-graph: collapse parameters into flags
  commit-graph: create new version flags
  commit-graph: add --version=<n> option
  commit-graph: implement file format version 2
  commit-graph: test verifying a corrupt v2 header

 Documentation/git-commit-graph.txt            |   3 +
 .../technical/commit-graph-format.txt         |  26 ++-
 builtin/commit-graph.c                        |  43 +++--
 builtin/commit.c                              |   5 +-
 builtin/gc.c                                  |   7 +-
 commit-graph.c                                | 158 +++++++++++++-----
 commit-graph.h                                |  16 +-
 t/t5318-commit-graph.sh                       |  42 ++++-
 8 files changed, 233 insertions(+), 67 deletions(-)


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

Comments

Junio C Hamano Jan. 24, 2019, 11:05 p.m. UTC | #1

"Derrick Stolee via GitGitGadget" <[e-mail]> writes:

> The commit-graph file format has some shortcomings that were discussed
> on-list:
> ...
> This series adds a new version (2) to the commit-graph file.

Sigh.  It is unfortunate that we have to bump the format version
this early before we can say "it is no longer experimental" with
confidence X-<.  Perhaps we have been moving too fast for our own
good and should slow down in introducing new low-level machineries?

Will queue.  Thanks.

Junio C Hamano Jan. 24, 2019, 11:39 p.m. UTC | #2

"Derrick Stolee via GitGitGadget" <[e-mail]> writes:

> This series is based on ab/commit-graph-write-progress and bc/sha-256.

Thanks.

It seems that the base (i.e. merge between these two topics) you
used may have used a version of either topic (most likely the
latter) slightly older than what I have, as patches 1 and 2 seem to
lack the local variable "hashsz" in the context, near the beginning
of commit_graph_write().  I wiggled the patches in, but it has too
heavy conflict merging to 'pu', so it may have to wait until the
other topics stabilize a bit further.

Derrick Stolee Jan. 25, 2019, 1:54 p.m. UTC | #3

On 1/24/2019 6:39 PM, Junio C Hamano wrote:
> "Derrick Stolee via GitGitGadget" <[e-mail]> writes:
> 
>> This series is based on ab/commit-graph-write-progress and bc/sha-256.
> 
> Thanks.
> 
> It seems that the base (i.e. merge between these two topics) you
> used may have used a version of either topic (most likely the
> latter) slightly older than what I have, as patches 1 and 2 seem to
> lack the local variable "hashsz" in the context, near the beginning
> of commit_graph_write().  I wiggled the patches in, but it has too
> heavy conflict merging to 'pu', so it may have to wait until the
> other topics stabilize a bit further.

Sorry that the merge was painful. I would have waited longer for
things to stabilize, but I'm expecting to go on paternity leave
soon. Didn't want to get the idea out there before I disappear
for a while.

When things stabilize, I may have time to do a rebase and work
out the details myself. Otherwise, everyone has my blessing to
take work I've started and move it forward themselves.

Thanks,
-Stolee