"Shawn O. Pearce" <spearce@xxxxxxxxxxx> 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. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html