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

Re: What's cooking in git.git (Sep 2013, #01; Tue, 3)

From
Jeff King <peff@peff.net>
Date
Sep 4, 2013, 09:01 UTC
Message-ID
<20130904090153.GA22348@sigill.intra.peff.net>
In-Reply-To
<xmqqppspo6i7.fsf@gitster.dls.corp.google.com>
On Tue, Sep 03, 2013 at 04:00:32PM -0700, Junio C Hamano wrote:
Show 12 quoted lines
> * jk/duplicate-objects-in-packs (2013-08-24) 6 commits
>  - default pack.indexDuplicates to false
>  - index-pack: optionally reject packs with duplicate objects
>  - test index-pack on packs with recoverable delta cycles
>  - add tests for indexing packs with delta cycles
>  - sha1-lookup: handle duplicate keys with GIT_USE_LOOKUP
>  - test-sha1: add a binary output mode
> 
>  A packfile that stores the same object more than once is broken and
>  will be rejected.
> 
>  Will merge to 'next'.

Hmph. I happened to be looking at the write_idx_file code a few minutes ago, and noticed the results of commit 68be2fe (receive-pack, fetch-pack: reject bogus pack that records objects twice, 2011-11-16).

Now I feel like a dunce for reimplementing the same thing a few lines above (and the reason I did not catch it in my tests is that "index-pack --strict" is not enabled by default).

We should probably drop the two top commits. We may want to keep the test from the second-to-last, though, but change it to use "index-pack --strict". Like this:

-- >8 --
Subject: t5308: check that index-pack --strict detects duplicate objects

Commit 68be2fe long ago taught index-pack to notice and reject duplicate objects if --strict is given (which it is for incoming packs, if transfer.fsckObjects is set). However, it never tested the code, because we did not have an easy way of generating such a bogus pack.

Now that we have test infrastructure to handle this, let's confirm that it works.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t5308-pack-detect-duplicates.sh | 7 +++++++
 1 file changed, 7 insertions(+)
diff --git a/t/t5308-pack-detect-duplicates.sh b/t/t5308-pack-detect-duplicates.sh
index 04fe242..9c5a876 100755
--- a/t/t5308-pack-detect-duplicates.sh
+++ b/t/t5308-pack-detect-duplicates.sh
@@ -70,4 +70,11 @@ test_expect_success 'lookup in duplicated pack (GIT_USE_LOOKUP)' '
 	test_cmp expect actual
 '
 
+test_expect_success 'index-pack can reject packs with duplicates' '
+	clear_packs &&
+	create_pack dups.pack 2 &&
+	test_must_fail git index-pack --strict --stdin <dups.pack &&
+	test_expect_code 1 git cat-file -e $LO_SHA1
+'
+
 test_done
-- 
1.8.4.2.g87d4a77
Previous: Junio C HamanoNext: Matthieu Moy
Message 2 of 8 in “What's cooking in git.git (Sep 2013, #01; Tue, 3)”
  1. Junio C HamanoSep 3, 2013
  2. Jeff KingSep 4, 2013
  3. Matthieu MoySep 4, 2013
  4. Junio C HamanoSep 4, 2013
  5. Jens LehmannSep 4, 2013
  6. Matthieu MoySep 5, 2013
  7. Jiang XinSep 6, 2013
  8. Junio C HamanoSep 6, 2013

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.