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

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

From
Shawn O. Pearce <spearce@spearce.org>
Date
Feb 28, 2011, 17:48 UTC
Message-ID
<20110228174857.GB17278@spearce.org>
In-Reply-To
<1298886583-30965-4-git-send-email-gitster@pobox.com>
Junio C Hamano <gitster@pobox.com> wrote:
Show 22 quoted lines
> Given an existing .pack file and the .idx file that describes it,
> this new mode of operation reads and re-index the packfile and makes
> sure the existing .idx file matches the result byte-for-byte.
> 
> All the objects in the .pack file are validated during this operation as
> well.  Unlike verify-pack, which visits each object described in the .idx
> file in the SHA-1 order, index-pack efficiently exploits the delta-chain
> to avoid rebuilding the objects that are used as the base of deltified
> objects over and over again while validating the objects.  This should
> result in much quicker verification of the .pack file and its .idx file.
> 
> This version however cannot verify a .pack/.idx pair with a handcrafted v2
> index that uses 64-bit offset representation for offsets that would fit
> within 31-bit. You can create such an .idx file by giving a custom offset
> to --index-version option to the command.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 
>  * This does not make much of a difference in a small project like git.git
>    itself, especially when the repository is packed with not very big depth
>    value.

However it is useful on larger repositories, like linux-2.6. I'd like to see this series cleaned up and submitted, because as Peff's tests shows, it shaves 1 minute off the linux-2.6 fsck.

Skim reading the code it mostly looked OK, though the NEEDSWORK stuff has to be cleaned up. 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.

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.

-- 
Shawn.
Previous: Junio C HamanoNext: Junio C Hamano
Message 5 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.