git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 28, 2011, 18:54 UTC
Message-ID
<7v7hckhtwj.fsf@alter.siamese.dyndns.org>
In-Reply-To
<20110228174857.GB17278@spearce.org>
"Shawn O. Pearce" <spearce@spearce.org> 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.

Previous: Shawn O. PearceNext: Junio C Hamano
Message 6 of 15 in “Using index-pack in place of verify-pack”
  1. 0/5 Using index-pack in place of verify-packJunio C Hamano, Feb 28, 2011
  2. 1/5 index-pack: group the delta-base array entries also by typeJunio C Hamano, Feb 28, 2011
  3. 2/5 write_idx_file: introduce a struct to hold idx customization optionsJunio C Hamano, Feb 28, 2011
  4. 3/5 index-pack: --verifyJunio C Hamano, Feb 28, 2011
  5. Shawn O. PearceFeb 28, 2011
  6. Junio C HamanoFeb 28, 2011
  7. 4/5 write_idx_file: need_large_offset() helper functionJunio C Hamano, Feb 28, 2011
  8. 5/5 index-pack --verify: read anomalous offsets from v2 idx fileJunio C Hamano, Feb 28, 2011
  9. Sverre RabbelierFeb 28, 2011
  10. Junio C HamanoFeb 28, 2011
  11. Sverre RabbelierFeb 28, 2011
  12. Jeff KingFeb 28, 2011
  13. Sverre RabbelierFeb 28, 2011
  14. Shawn O. PearceFeb 28, 2011
  15. Junio C HamanoFeb 28, 2011

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.