mbox series

[0/6] some "commit-graph verify" fixes for chains

Message ID 20230926055452.GA1341109@coredump.intra.peff.net (mailing list archive)
Headers show
Series some "commit-graph verify" fixes for chains | expand

Message

Jeff King Sept. 26, 2023, 5:54 a.m. UTC
While yak-shaving another topic, I noticed that there are a bunch of
error cases that "git commit-graph verify" does not notice when checking
chained graph files. This series should make it more robust.

Some of these cases are actually covered in t5324 already, but the tests
expect the verify command not to fail, which just seems wrong. I don't
see anything in the commit message or mailing list explaining it, so I
think it was just "well, this is how it behaves now", and not some
clever scheme.

  [1/6]: commit-graph: factor out chain opening function
  [2/6]: commit-graph: check mixed generation validation when loading chain file
  [3/6]: t5324: harmonize sha1/sha256 graph chain corruption
  [4/6]: commit-graph: detect read errors when verifying graph chain
  [5/6]: commit-graph: tighten chain size check
  [6/6]: commit-graph: report incomplete chains during verification

 builtin/commit-graph.c        |  31 +++++++---
 commit-graph.c                | 109 ++++++++++++++++++++++------------
 commit-graph.h                |   4 ++
 t/t5324-split-commit-graph.sh |  48 +++++++++++++--
 4 files changed, 141 insertions(+), 51 deletions(-)

-Peff

PS If you are curious about the yak-shave, it is:

     3. "commit-graph verify" fails to propagate some errors (this
	series)
     2. we are missing a bunch of bounds checks on commit-graph files;
	fixing that makes the tests inconsistent because of (3)
     1. there are some unused parameters that should be used for bounds
	checks, hence (2)

  Just in case anybody thought there was a part of my life that was not
  motivated by removing -Wunused-parameter warnings.