* [RFC PATCH] index-pack: optionally allow duplicate objects
@ 2026-07-28 4:25 friel
0 siblings, 0 replies; only message in thread
From: friel @ 2026-07-28 4:25 UTC (permalink / raw)
To: git; +Cc: Friel, gitster, peff, stolee, me, ps, jonathantanmy
From: Friel <friel@openai.com>
index-pack accepts repeated object IDs by default. However,
--check-self-contained-and-connected also enables strict index
validation, which rejects them. A clone therefore rejects a pack
containing duplicate objects even when all of its objects are connected.
By default, shallow and filtered clones do not request this pack-local
check and already accept duplicate objects. Ordinary full clones reject
the same pack because they enable strict index validation.
Git's upload-pack normally uses pack-objects to select each reachable
object once before writing a response. A server can instead construct
that response by streaming entries from existing packs. When those
packs overlap, the same object can appear more than once.
Avoiding duplicates requires the producers to coordinate their object
selection or track object IDs across all input packs. A duplicate can
also be used as a delta base, so removing it can require buffering and
rewriting the response. Doing that work at request time gives up the
memory and latency benefits of streaming existing packs.
Add pack.allowDuplicateObjects so a client can accept these packs during
its initial fetch:
git clone -c pack.allowDuplicateObjects <repository>
Add --[no-]allow-duplicate-objects to override the configuration for
index-pack. Preserve the default rejection introduced by 68be2fea50
(receive-pack, fetch-pack: reject bogus pack that records objects twice,
2011-11-16). An explicit --strict or --verify remains strict regardless
of the configuration and cannot be combined with
--allow-duplicate-objects. Object validation, connectivity checks, and
delta resolution remain unchanged.
Signed-off-by: Friel <friel@openai.com>
---
Applies on top of tb/pack-with-duplicates.
Documentation/config/pack.adoc | 11 ++
Documentation/git-index-pack.adoc | 12 ++-
builtin/index-pack.c | 41 ++++++-
t/t5308-pack-detect-duplicates.sh | 171 ++++++++++++++++++++++++++++++
t/t5309-pack-delta-cycles.sh | 10 ++
5 files changed, 241 insertions(+), 4 deletions(-)
diff --git a/Documentation/config/pack.adoc b/Documentation/config/pack.adoc
index 22384c2d2f..2229878abe 100644
--- a/Documentation/config/pack.adoc
+++ b/Documentation/config/pack.adoc
@@ -39,6 +39,17 @@ is set to "multi", reuse parts of just the bitmapped packfile. This
can reduce memory and CPU usage to serve fetches, but might result in
sending a slightly larger pack. Defaults to true.
+pack.allowDuplicateObjects::
+ Allow linkgit:git-index-pack[1] to accept a pack containing
+ multiple copies of the same object while checking that the pack
+ is self-contained and connected. For example,
+ `git clone -c pack.allowDuplicateObjects <repository>` can accept
+ a pack generated from overlapping existing packs. Object and
+ connectivity checks are preserved. Explicit `--strict` and
+ `--verify` continue to reject duplicate objects.
+ `--no-allow-duplicate-objects` overrides this setting.
+ Defaults to `false`.
+
pack.island::
An extended regular expression configuring a set of delta
islands. See "DELTA ISLANDS" in linkgit:git-pack-objects[1]
diff --git a/Documentation/git-index-pack.adoc b/Documentation/git-index-pack.adoc
index 18036953c0..1cb11ff898 100644
--- a/Documentation/git-index-pack.adoc
+++ b/Documentation/git-index-pack.adoc
@@ -11,7 +11,9 @@ SYNOPSIS
[verse]
'git index-pack' [-v] [-o <index-file>] [--[no-]rev-index] <pack-file>
'git index-pack' --stdin [--fix-thin] [--keep] [-v] [-o <index-file>]
- [--[no-]rev-index] [<pack-file>]
+ [--[no-]rev-index]
+ [--[no-]allow-duplicate-objects]
+ [<pack-file>]
DESCRIPTION
@@ -97,6 +99,14 @@ default and "Indexing objects" when `--stdin` is specified.
--check-self-contained-and-connected::
Die if the pack contains broken links. For internal use only.
+--allow-duplicate-objects::
+--no-allow-duplicate-objects::
+ Allow or reject multiple copies of the same object while checking
+ that the pack is self-contained and connected. The default is
+ controlled by `pack.allowDuplicateObjects`. The command-line
+ option overrides the configuration. `--allow-duplicate-objects`
+ cannot be combined with `--strict` or `--verify`.
+
--fsck-objects[=<msg-id>=<severity>...]::
Die if the pack contains broken objects, but unlike `--strict`, don't
choke on broken links. If the pack contains a tree pointing to a
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index bc86925ad0..4ffcbb6c28 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -33,7 +33,7 @@
#include "strvec.h"
static const char index_pack_usage[] =
-"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-id>=<severity>...]] [--fsck-objects[=<msg-id>=<severity>...]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
+"git index-pack [-v] [-o <index-file>] [--keep | --keep=<msg>] [--[no-]rev-index] [--verify] [--strict[=<msg-id>=<severity>...]] [--[no-]allow-duplicate-objects] [--fsck-objects[=<msg-id>=<severity>...]] (<pack-file> | --stdin [--fix-thin] [<pack-file>])";
struct object_entry {
struct pack_idx_entry idx;
@@ -135,6 +135,11 @@ static int nr_threads;
static int from_stdin;
static int strict;
+static enum {
+ DUPLICATE_OBJECTS_REJECT = 0,
+ DUPLICATE_OBJECTS_ALLOW_CONFIG,
+ DUPLICATE_OBJECTS_ALLOW_OPTION,
+} allow_duplicate_objects;
static int do_fsck_object;
static struct fsck_options fsck_options;
static int verbose;
@@ -1673,6 +1678,12 @@ static int git_index_pack_config(const char *k, const char *v,
die(_("bad pack.indexVersion=%"PRIu32), opts->version);
return 0;
}
+ if (!strcmp(k, "pack.allowduplicateobjects")) {
+ allow_duplicate_objects = git_config_bool(k, v) ?
+ DUPLICATE_OBJECTS_ALLOW_CONFIG :
+ DUPLICATE_OBJECTS_REJECT;
+ return 0;
+ }
if (!strcmp(k, "pack.threads")) {
nr_threads = git_config_int(k, v, ctx->kvi);
if (nr_threads < 0)
@@ -1889,6 +1900,7 @@ int cmd_index_pack(int argc,
struct repository *repo UNUSED)
{
int i, fix_thin_pack = 0, verify = 0, stat_only = 0, rev_index;
+ int write_idx_strict;
const char *curr_index;
char *curr_rev_index = NULL;
const char *index_name = NULL, *pack_name = NULL, *rev_index_name = NULL;
@@ -1941,8 +1953,11 @@ int cmd_index_pack(int argc,
strict = 1;
do_fsck_object = 1;
fsck_set_msg_types(&fsck_options, arg);
+ } else if (!strcmp(arg, "--allow-duplicate-objects")) {
+ allow_duplicate_objects = DUPLICATE_OBJECTS_ALLOW_OPTION;
+ } else if (!strcmp(arg, "--no-allow-duplicate-objects")) {
+ allow_duplicate_objects = DUPLICATE_OBJECTS_REJECT;
} else if (!strcmp(arg, "--check-self-contained-and-connected")) {
- strict = 1;
check_self_contained_and_connected = 1;
} else if (skip_to_optional_arg(arg, "--fsck-objects", &arg)) {
do_fsck_object = 1;
@@ -2022,6 +2037,26 @@ int cmd_index_pack(int argc,
usage(index_pack_usage);
if (fix_thin_pack && !from_stdin)
die(_("the option '%s' requires '%s'"), "--fix-thin", "--stdin");
+
+ /*
+ * Connectivity checks require strict object traversal, but may allow
+ * duplicate entries in the pack index.
+ */
+ write_idx_strict = strict;
+ if (check_self_contained_and_connected) {
+ strict = 1;
+ if (allow_duplicate_objects == DUPLICATE_OBJECTS_REJECT)
+ write_idx_strict = 1;
+ }
+
+ if (write_idx_strict &&
+ allow_duplicate_objects == DUPLICATE_OBJECTS_ALLOW_OPTION)
+ die(_("options '%s' and '%s' cannot be used together"),
+ "--allow-duplicate-objects", "--strict");
+ if (verify &&
+ allow_duplicate_objects == DUPLICATE_OBJECTS_ALLOW_OPTION)
+ die(_("options '%s' and '%s' cannot be used together"),
+ "--allow-duplicate-objects", "--verify");
if (promisor_msg && pack_name)
die(_("--promisor cannot be used with a pack name"));
if (from_stdin && !startup_info->have_repository)
@@ -2055,7 +2090,7 @@ int cmd_index_pack(int argc,
read_idx_option(&opts, index_name);
opts.flags |= WRITE_IDX_VERIFY | WRITE_IDX_STRICT;
}
- if (strict)
+ if (write_idx_strict)
opts.flags |= WRITE_IDX_STRICT;
if (HAVE_THREADS && !nr_threads) {
diff --git a/t/t5308-pack-detect-duplicates.sh b/t/t5308-pack-detect-duplicates.sh
index c6273a1aeb..95c81fa7b4 100755
--- a/t/t5308-pack-detect-duplicates.sh
+++ b/t/t5308-pack-detect-duplicates.sh
@@ -139,4 +139,175 @@ test_expect_success 'index-pack can reject packs with duplicates' '
test_expect_code 1 git cat-file -e $LO_SHA1
'
+test_expect_success 'connectivity check rejects duplicate objects by default' '
+ clear_packs &&
+ create_pack dups.pack 2 &&
+ test_must_fail git index-pack \
+ --check-self-contained-and-connected --stdin <dups.pack &&
+ test_expect_code 1 git cat-file -e $LO_SHA1
+'
+
+test_expect_success 'connectivity check can allow duplicate objects' '
+ clear_packs &&
+ create_pack dups.pack 2 &&
+ git index-pack --check-self-contained-and-connected \
+ --allow-duplicate-objects --stdin <dups.pack &&
+ git cat-file -e "$LO_SHA1" &&
+ git cat-file -e "$HI_SHA1"
+'
+
+test_expect_success 'configuration allows duplicates with connectivity checks' '
+ clear_packs &&
+ create_pack dups.pack 2 &&
+ git -c pack.allowDuplicateObjects index-pack \
+ --check-self-contained-and-connected --stdin <dups.pack &&
+ git cat-file -e "$LO_SHA1" &&
+ git cat-file -e "$HI_SHA1"
+'
+
+test_expect_success 'command-line allowance overrides false configuration' '
+ clear_packs &&
+ create_pack dups.pack 2 &&
+ git -c pack.allowDuplicateObjects=false index-pack \
+ --check-self-contained-and-connected \
+ --allow-duplicate-objects --stdin <dups.pack &&
+ git cat-file -e "$LO_SHA1" &&
+ git cat-file -e "$HI_SHA1"
+'
+
+test_expect_success 'command-line rejection overrides true configuration' '
+ clear_packs &&
+ create_pack dups.pack 2 &&
+ test_must_fail git -c pack.allowDuplicateObjects=true index-pack \
+ --check-self-contained-and-connected \
+ --no-allow-duplicate-objects --stdin <dups.pack 2>err &&
+ test_grep "appears twice in the pack" err
+'
+
+test_expect_success 'configured allowance does not relax explicit strict mode' '
+ clear_packs &&
+ create_pack dups.pack 2 &&
+ test_must_fail git -c pack.allowDuplicateObjects=true index-pack \
+ --strict --stdin <dups.pack 2>err &&
+ test_grep "appears twice in the pack" err
+'
+
+test_expect_success 'explicit strict mode cannot allow duplicate objects' '
+ clear_packs &&
+ create_pack dups.pack 2 &&
+ test_must_fail git index-pack --strict --allow-duplicate-objects \
+ --stdin <dups.pack 2>err &&
+ test_grep "cannot be used together" err
+'
+
+test_expect_success 'configured allowance does not relax verification' '
+ clear_packs &&
+ create_pack dups.pack 2 &&
+ git index-pack --check-self-contained-and-connected \
+ --allow-duplicate-objects --stdin <dups.pack &&
+ test_must_fail git -c pack.allowDuplicateObjects=true index-pack \
+ --verify .git/objects/pack/pack-*.pack 2>err &&
+ test_grep "appears twice in the pack" err
+'
+
+test_expect_success 'verify cannot allow duplicate objects' '
+ test_must_fail git index-pack --verify --allow-duplicate-objects \
+ .git/objects/pack/pack-*.pack 2>err &&
+ test_grep "cannot be used together" err
+'
+
+test_expect_success 'configured allowance preserves connectivity checks' '
+ clear_packs &&
+ tree=$(git mktree </dev/null) &&
+ commit=$(echo message | git commit-tree "$tree") &&
+ {
+ pack_header 2 &&
+ pack_obj $commit &&
+ pack_obj $commit
+ } >not-self-contained.pack &&
+ pack_trailer not-self-contained.pack &&
+ rm .git/objects/$(test_oid_to_path $commit) &&
+ test_expect_code 1 git -c pack.allowDuplicateObjects index-pack \
+ --check-self-contained-and-connected \
+ --stdin <not-self-contained.pack &&
+ test "$(git cat-file -t $commit)" = commit
+'
+
+test_expect_success 'allowing duplicates preserves object checks' '
+ clear_packs &&
+ tree=$(git mktree </dev/null) &&
+ cat >bad-commit <<-EOF &&
+ tree $tree
+ author A U Thor 1234567890 +0000
+ committer C O Mitter <committer@example.com> 1234567890 +0000
+
+ message
+ EOF
+ commit=$(git hash-object --literally -t commit -w --stdin \
+ <bad-commit) &&
+ {
+ pack_header 2 &&
+ pack_obj $commit &&
+ pack_obj $commit
+ } >bad-objects.pack &&
+ pack_trailer bad-objects.pack &&
+ rm .git/objects/$(test_oid_to_path $commit) &&
+ test_must_fail git index-pack \
+ --check-self-contained-and-connected --fsck-objects \
+ --allow-duplicate-objects --stdin <bad-objects.pack 2>err &&
+ test_grep "missingEmail" err
+'
+
+test_expect_success 'set up upload-pack that serves duplicate objects' '
+ test_commit clone-duplicates &&
+ commit=$(git rev-parse HEAD) &&
+ tree=$(git rev-parse HEAD^{tree}) &&
+ blob=$(git rev-parse HEAD:clone-duplicates.t) &&
+ {
+ pack_header 4 &&
+ pack_obj "$commit" &&
+ pack_obj "$tree" &&
+ pack_obj "$blob" &&
+ pack_obj "$blob"
+ } >clone-duplicates.pack &&
+ pack_trailer clone-duplicates.pack &&
+ write_script .git/duplicate-pack-hook <<-EOF
+ cat >/dev/null &&
+ cat "$TRASH_DIRECTORY/clone-duplicates.pack"
+ EOF
+'
+
+test_expect_success 'clone rejects duplicate objects by default' '
+ test_must_fail git clone --no-local \
+ -u "git -c uploadpack.packObjectsHook=./duplicate-pack-hook upload-pack" \
+ . clone-reject 2>err &&
+ test_grep "appears twice in the pack" err
+'
+
+test_expect_success 'shallow clone already accepts duplicate objects' '
+ git clone --no-local --depth=1 \
+ -u "git -c uploadpack.packObjectsHook=./duplicate-pack-hook upload-pack" \
+ . clone-shallow &&
+ test "$(git -C clone-shallow rev-parse HEAD)" = "$commit" &&
+ git -C clone-shallow fsck --full
+'
+
+test_expect_success 'filtered clone already accepts duplicate objects' '
+ git clone --no-local --filter=blob:none \
+ -u "git -c uploadpack.allowFilter=true -c uploadpack.packObjectsHook=./duplicate-pack-hook upload-pack" \
+ . clone-filter &&
+ test "$(git -C clone-filter config remote.origin.partialclonefilter)" = blob:none &&
+ test "$(git -C clone-filter rev-parse HEAD)" = "$commit" &&
+ git -C clone-filter fsck --full
+'
+
+test_expect_success 'clone -c pack.allowDuplicateObjects accepts duplicate objects' '
+ git clone --no-local -c pack.allowDuplicateObjects \
+ -u "git -c uploadpack.packObjectsHook=./duplicate-pack-hook upload-pack" \
+ . clone-allow &&
+ test "$(git -C clone-allow config --bool pack.allowDuplicateObjects)" = true &&
+ test "$(git -C clone-allow rev-parse HEAD)" = "$commit" &&
+ git -C clone-allow fsck --full
+'
+
test_done
diff --git a/t/t5309-pack-delta-cycles.sh b/t/t5309-pack-delta-cycles.sh
index f613950e38..fb5ed81ca3 100755
--- a/t/t5309-pack-delta-cycles.sh
+++ b/t/t5309-pack-delta-cycles.sh
@@ -217,6 +217,16 @@ test_expect_success 'failover after a tail into a three-object delta cycle' '
check_blob "$T" tail
'
+test_expect_success 'configured connectivity check recovers duplicate delta bases' '
+ clear_packs &&
+ git -c pack.allowDuplicateObjects index-pack --fix-thin \
+ --check-self-contained-and-connected \
+ --stdin <recoverable-1.pack &&
+ printf "\7\0" >expect.duplicate-base &&
+ check_blob "$A" expect.duplicate-base &&
+ git cat-file -e "$B"
+'
+
test_expect_success 'index-pack works with thin pack A->B->C with B on disk' '
git init server &&
(
base-commit: fd2739b159d075cd4c6fa69b2cd876ba1caa5c88
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2026-07-28 4:25 UTC | newest]
Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-28 4:25 [RFC PATCH] index-pack: optionally allow duplicate objects friel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox