Re: [PATCH 3/5] index-pack: --verify

"Shawn O. Pearce" [off-list ref] writes:

Skim reading the code it mostly looked OK, though the NEEDSWORK
stuff has to be cleaned up.

Yeah, the part that needs working is about thinking if such a clean-up
(i.e. extracting small piece of code from free-pack-by-name to make an
official API function to free storage for a packed_git while closing the
pack_index and its associated resource) is necessary.  There are other
remaining tasks such as integrating this into verify_pack() function,
which in turn requires to teach index-pack to produce the delta-chain
histograms (i.e. "verify-pack --verbose").  Also the codepath needs to be
taught about the plan for narrow clones---there are places that assume
everything referred to from objects need to exist.  These have higher
priority than this NEEDSWORK comment.

... And I wonder if the series cannot be
flipped around a bit to put the 4/5 earlier and try to avoid a
stage where `index-pack --verify` doesn't do the right thing on
the hand-rolled lower 32 bit index limit.

Oh, I fully agree that 4/5 should be squashed into this in the final
round.  I preserved the progression of thought in this WIP as it
illustrated pitfalls to avoid rather nicely (iow, hoping to show others a
BCP of building a series by example).  The separation would also hopefully
have made the series easier to review.

In this patch I'm not happy about csum-file having this check_fd
part of its contents. But its probably the shortest way to inject
validation into index-pack without butchering a large part of its
index generation function.

Yeah, I am only 70% happy about the approach myself.  Patches and
improvements are welcome, of course.