{"thread":{"id":"66076","subject":"[RFC PATCH] index-pack: optionally allow duplicate objects","startedAt":"2026-07-28T04:25:52Z","lastAt":"2026-07-29T21:29:09Z","messageCount":8,"participants":["friel@openai.com","Taylor Blau","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"549111","messageId":"20260728042550.91133-2-friel@openai.com","threadId":"66076","inReplyTo":null,"subject":"[RFC PATCH] index-pack: optionally allow duplicate objects","fromName":"","fromEmail":"friel@openai.com","sentAt":"2026-07-28T04:25:32Z","receivedAt":"2026-07-28T04:25:52Z","isPatch":true,"body":"From: Friel \n\nindex-pack accepts repeated object IDs by default. However,\n--check-self-contained-and-connected also enables strict index\nvalidation, which rejects them. A clone therefore rejects a pack\ncontaining duplicate objects even when all of its objects are connected.\n\nBy default, shallow and filtered clones do not request this pack-local\ncheck and already accept duplicate objects. Ordinary full clones reject\nthe same pack because they enable strict index validation.\n\nGit's upload-pack normally uses pack-objects to select each reachable\nobject once before writing a response. A server can instead construct\nthat response by streaming entries from existing packs. When those\npacks overlap, the same object can appear more than once.\n\nAvoiding duplicates requires the producers to coordinate their object\nselection or track object IDs across all input packs. A duplicate can\nalso be used as a delta base, so removing it can require buffering and\nrewriting the response. Doing that work at request time gives up the\nmemory and latency benefits of streaming existing packs.\n\nAdd pack.allowDuplicateObjects so a client can accept these packs during\nits initial fetch:\n\n git clone -c pack.allowDuplicateObjects \n\nAdd --[no-]allow-duplicate-objects to override the configuration for\nindex-pack. Preserve the default rejection introduced by 68be2fea50\n(receive-pack, fetch-pack: reject bogus pack that records objects twice,\n2011-11-16). An explicit --strict or --verify remains strict regardless\nof the configuration and cannot be combined with\n--allow-duplicate-objects. Object validation, connectivity checks, and\ndelta resolution remain unchanged.\n\nSigned-off-by: Friel \n---\nApplies on top of tb/pack-with-duplicates.\n\n Documentation/config/pack.adoc | 11 ++\n Documentation/git-index-pack.adoc | 12 ++-\n builtin/index-pack.c | 41 ++++++-\n t/t5308-pack-detect-duplicates.sh | 171 ++++++++++++++++++++++++++++++\n t/t5309-pack-delta-cycles.sh | 10 ++\n 5 files changed, 241 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config/pack.adoc b/Documentation/config/pack.adoc\nindex 22384c2d2f..2229878abe 100644\n--- a/Documentation/config/pack.adoc\n+++ b/Documentation/config/pack.adoc\n@@ -39,6 +39,17 @@ is set to \"multi\", reuse parts of just the bitmapped packfile. This\n can reduce memory and CPU usage to serve fetches, but might result in\n sending a slightly larger pack. Defaults to true.\n \n+pack.allowDuplicateObjects::\n+\tAllow linkgit:git-index-pack[1] to accept a pack containing\n+\tmultiple copies of the same object while checking that the pack\n+\tis self-contained and connected. For example,\n+\t`git clone -c pack.allowDuplicateObjects ` can accept\n+\ta pack generated from overlapping existing packs. Object and\n+\tconnectivity checks are preserved. Explicit `--strict` and\n+\t`--verify` continue to reject duplicate objects.\n+\t`--no-allow-duplicate-objects` overrides this setting.\n+\tDefaults to `false`.\n+\n pack.island::\n \tAn extended regular expression configuring a set of delta\n \tislands. See \"DELTA ISLANDS\" in linkgit:git-pack-objects[1]\ndiff --git a/Documentation/git-index-pack.adoc b/Documentation/git-index-pack.adoc\nindex 18036953c0..1cb11ff898 100644\n--- a/Documentation/git-index-pack.adoc\n+++ b/Documentation/git-index-pack.adoc\n@@ -11,7 +11,9 @@ SYNOPSIS\n [verse]\n 'git index-pack' [-v] [-o ] [--[no-]rev-index] \n 'git index-pack' --stdin [--fix-thin] [--keep] [-v] [-o ]\n-\t\t [--[no-]rev-index] []\n+\t\t [--[no-]rev-index]\n+\t\t [--[no-]allow-duplicate-objects]\n+\t\t []\n \n \n DESCRIPTION\n@@ -97,6 +99,14 @@ default and \"Indexing objects\" when `--stdin` is specified.\n --check-self-contained-and-connected::\n \tDie if the pack contains broken links. For internal use only.\n \n+--allow-duplicate-objects::\n+--no-allow-duplicate-objects::\n+\tAllow or reject multiple copies of the same object while checking\n+\tthat the pack is self-contained and connected. The default is\n+\tcontrolled by `pack.allowDuplicateObjects`. The command-line\n+\toption overrides the configuration. `--allow-duplicate-objects`\n+\tcannot be combined with `--strict` or `--verify`.\n+\n --fsck-objects[==...]::\n \tDie if the pack contains broken objects, but unlike `--strict`, don't\n \tchoke on broken links. If the pack contains a tree pointing to a\ndiff --git a/builtin/index-pack.c b/builtin/index-pack.c\nindex bc86925ad0..4ffcbb6c28 100644\n--- a/builtin/index-pack.c\n+++ b/builtin/index-pack.c\n@@ -33,7 +33,7 @@\n #include \"strvec.h\"\n \n static const char index_pack_usage[] =\n-\"git index-pack [-v] [-o ] [--keep | --keep=] [--[no-]rev-index] [--verify] [--strict[==...]] [--fsck-objects[==...]] ( | --stdin [--fix-thin] [])\";\n+\"git index-pack [-v] [-o ] [--keep | --keep=] [--[no-]rev-index] [--verify] [--strict[==...]] [--[no-]allow-duplicate-objects] [--fsck-objects[==...]] ( | --stdin [--fix-thin] [])\";\n \n struct object_entry {\n \tstruct pack_idx_entry idx;\n@@ -135,6 +135,11 @@ static int nr_threads;\n \n static int from_stdin;\n static int strict;\n+static enum {\n+\tDUPLICATE_OBJECTS_REJECT = 0,\n+\tDUPLICATE_OBJECTS_ALLOW_CONFIG,\n+\tDUPLICATE_OBJECTS_ALLOW_OPTION,\n+} allow_duplicate_objects;\n static int do_fsck_object;\n static struct fsck_options fsck_options;\n static int verbose;\n@@ -1673,6 +1678,12 @@ static int git_index_pack_config(const char *k, const char *v,\n \t\t\tdie(_(\"bad pack.indexVersion=%\"PRIu32), opts->version);\n \t\treturn 0;\n \t}\n+\tif (!strcmp(k, \"pack.allowduplicateobjects\")) {\n+\t\tallow_duplicate_objects = git_config_bool(k, v) ?\n+\t\t\tDUPLICATE_OBJECTS_ALLOW_CONFIG :\n+\t\t\tDUPLICATE_OBJECTS_REJECT;\n+\t\treturn 0;\n+\t}\n \tif (!strcmp(k, \"pack.threads\")) {\n \t\tnr_threads = git_config_int(k, v, ctx->kvi);\n \t\tif (nr_threads < 0)\n@@ -1889,6 +1900,7 @@ int cmd_index_pack(int argc,\n \t\t struct repository *repo UNUSED)\n {\n \tint i, fix_thin_pack = 0, verify = 0, stat_only = 0, rev_index;\n+\tint write_idx_strict;\n \tconst char *curr_index;\n \tchar *curr_rev_index = NULL;\n \tconst char *index_name = NULL, *pack_name = NULL, *rev_index_name = NULL;\n@@ -1941,8 +1953,11 @@ int cmd_index_pack(int argc,\n \t\t\t\tstrict = 1;\n \t\t\t\tdo_fsck_object = 1;\n \t\t\t\tfsck_set_msg_types(&fsck_options, arg);\n+\t\t\t} else if (!strcmp(arg, \"--allow-duplicate-objects\")) {\n+\t\t\t\tallow_duplicate_objects = DUPLICATE_OBJECTS_ALLOW_OPTION;\n+\t\t\t} else if (!strcmp(arg, \"--no-allow-duplicate-objects\")) {\n+\t\t\t\tallow_duplicate_objects = DUPLICATE_OBJECTS_REJECT;\n \t\t\t} else if (!strcmp(arg, \"--check-self-contained-and-connected\")) {\n-\t\t\t\tstrict = 1;\n \t\t\t\tcheck_self_contained_and_connected = 1;\n \t\t\t} else if (skip_to_optional_arg(arg, \"--fsck-objects\", &arg)) {\n \t\t\t\tdo_fsck_object = 1;\n@@ -2022,6 +2037,26 @@ int cmd_index_pack(int argc,\n \t\tusage(index_pack_usage);\n \tif (fix_thin_pack && !from_stdin)\n \t\tdie(_(\"the option '%s' requires '%s'\"), \"--fix-thin\", \"--stdin\");\n+\n+\t/*\n+\t * Connectivity checks require strict object traversal, but may allow\n+\t * duplicate entries in the pack index.\n+\t */\n+\twrite_idx_strict = strict;\n+\tif (check_self_contained_and_connected) {\n+\t\tstrict = 1;\n+\t\tif (allow_duplicate_objects == DUPLICATE_OBJECTS_REJECT)\n+\t\t\twrite_idx_strict = 1;\n+\t}\n+\n+\tif (write_idx_strict &&\n+\t allow_duplicate_objects == DUPLICATE_OBJECTS_ALLOW_OPTION)\n+\t\tdie(_(\"options '%s' and '%s' cannot be used together\"),\n+\t\t \"--allow-duplicate-objects\", \"--strict\");\n+\tif (verify &&\n+\t allow_duplicate_objects == DUPLICATE_OBJECTS_ALLOW_OPTION)\n+\t\tdie(_(\"options '%s' and '%s' cannot be used together\"),\n+\t\t \"--allow-duplicate-objects\", \"--verify\");\n \tif (promisor_msg && pack_name)\n \t\tdie(_(\"--promisor cannot be used with a pack name\"));\n \tif (from_stdin && !startup_info->have_repository)\n@@ -2055,7 +2090,7 @@ int cmd_index_pack(int argc,\n \t\tread_idx_option(&opts, index_name);\n \t\topts.flags |= WRITE_IDX_VERIFY | WRITE_IDX_STRICT;\n \t}\n-\tif (strict)\n+\tif (write_idx_strict)\n \t\topts.flags |= WRITE_IDX_STRICT;\n \n \tif (HAVE_THREADS && !nr_threads) {\ndiff --git a/t/t5308-pack-detect-duplicates.sh b/t/t5308-pack-detect-duplicates.sh\nindex c6273a1aeb..95c81fa7b4 100755\n--- a/t/t5308-pack-detect-duplicates.sh\n+++ b/t/t5308-pack-detect-duplicates.sh\n@@ -139,4 +139,175 @@ test_expect_success 'index-pack can reject packs with duplicates' '\n \ttest_expect_code 1 git cat-file -e $LO_SHA1\n '\n \n+test_expect_success 'connectivity check rejects duplicate objects by default' '\n+\tclear_packs &&\n+\tcreate_pack dups.pack 2 &&\n+\ttest_must_fail git index-pack \\\n+\t\t--check-self-contained-and-connected --stdin err &&\n+\ttest_grep \"appears twice in the pack\" err\n+'\n+\n+test_expect_success 'configured allowance does not relax explicit strict mode' '\n+\tclear_packs &&\n+\tcreate_pack dups.pack 2 &&\n+\ttest_must_fail git -c pack.allowDuplicateObjects=true index-pack \\\n+\t\t--strict --stdin err &&\n+\ttest_grep \"appears twice in the pack\" err\n+'\n+\n+test_expect_success 'explicit strict mode cannot allow duplicate objects' '\n+\tclear_packs &&\n+\tcreate_pack dups.pack 2 &&\n+\ttest_must_fail git index-pack --strict --allow-duplicate-objects \\\n+\t\t--stdin err &&\n+\ttest_grep \"cannot be used together\" err\n+'\n+\n+test_expect_success 'configured allowance does not relax verification' '\n+\tclear_packs &&\n+\tcreate_pack dups.pack 2 &&\n+\tgit index-pack --check-self-contained-and-connected \\\n+\t\t--allow-duplicate-objects --stdin err &&\n+\ttest_grep \"appears twice in the pack\" err\n+'\n+\n+test_expect_success 'verify cannot allow duplicate objects' '\n+\ttest_must_fail git index-pack --verify --allow-duplicate-objects \\\n+\t\t.git/objects/pack/pack-*.pack 2>err &&\n+\ttest_grep \"cannot be used together\" err\n+'\n+\n+test_expect_success 'configured allowance preserves connectivity checks' '\n+\tclear_packs &&\n+\ttree=$(git mktree not-self-contained.pack &&\n+\tpack_trailer not-self-contained.pack &&\n+\trm .git/objects/$(test_oid_to_path $commit) &&\n+\ttest_expect_code 1 git -c pack.allowDuplicateObjects index-pack \\\n+\t\t--check-self-contained-and-connected \\\n+\t\t--stdin bad-commit <<-EOF &&\n+\ttree $tree\n+\tauthor A U Thor 1234567890 +0000\n+\tcommitter C O Mitter 1234567890 +0000\n+\n+\tmessage\n+\tEOF\n+\tcommit=$(git hash-object --literally -t commit -w --stdin \\\n+\t\tbad-objects.pack &&\n+\tpack_trailer bad-objects.pack &&\n+\trm .git/objects/$(test_oid_to_path $commit) &&\n+\ttest_must_fail git index-pack \\\n+\t\t--check-self-contained-and-connected --fsck-objects \\\n+\t\t--allow-duplicate-objects --stdin err &&\n+\ttest_grep \"missingEmail\" err\n+'\n+\n+test_expect_success 'set up upload-pack that serves duplicate objects' '\n+\ttest_commit clone-duplicates &&\n+\tcommit=$(git rev-parse HEAD) &&\n+\ttree=$(git rev-parse HEAD^{tree}) &&\n+\tblob=$(git rev-parse HEAD:clone-duplicates.t) &&\n+\t{\n+\t\tpack_header 4 &&\n+\t\tpack_obj \"$commit\" &&\n+\t\tpack_obj \"$tree\" &&\n+\t\tpack_obj \"$blob\" &&\n+\t\tpack_obj \"$blob\"\n+\t} >clone-duplicates.pack &&\n+\tpack_trailer clone-duplicates.pack &&\n+\twrite_script .git/duplicate-pack-hook <<-EOF\n+\tcat >/dev/null &&\n+\tcat \"$TRASH_DIRECTORY/clone-duplicates.pack\"\n+\tEOF\n+'\n+\n+test_expect_success 'clone rejects duplicate objects by default' '\n+\ttest_must_fail git clone --no-local \\\n+\t\t-u \"git -c uploadpack.packObjectsHook=./duplicate-pack-hook upload-pack\" \\\n+\t\t. clone-reject 2>err &&\n+\ttest_grep \"appears twice in the pack\" err\n+'\n+\n+test_expect_success 'shallow clone already accepts duplicate objects' '\n+\tgit clone --no-local --depth=1 \\\n+\t\t-u \"git -c uploadpack.packObjectsHook=./duplicate-pack-hook upload-pack\" \\\n+\t\t. clone-shallow &&\n+\ttest \"$(git -C clone-shallow rev-parse HEAD)\" = \"$commit\" &&\n+\tgit -C clone-shallow fsck --full\n+'\n+\n+test_expect_success 'filtered clone already accepts duplicate objects' '\n+\tgit clone --no-local --filter=blob:none \\\n+\t\t-u \"git -c uploadpack.allowFilter=true -c uploadpack.packObjectsHook=./duplicate-pack-hook upload-pack\" \\\n+\t\t. clone-filter &&\n+\ttest \"$(git -C clone-filter config remote.origin.partialclonefilter)\" = blob:none &&\n+\ttest \"$(git -C clone-filter rev-parse HEAD)\" = \"$commit\" &&\n+\tgit -C clone-filter fsck --full\n+'\n+\n+test_expect_success 'clone -c pack.allowDuplicateObjects accepts duplicate objects' '\n+\tgit clone --no-local -c pack.allowDuplicateObjects \\\n+\t\t-u \"git -c uploadpack.packObjectsHook=./duplicate-pack-hook upload-pack\" \\\n+\t\t. clone-allow &&\n+\ttest \"$(git -C clone-allow config --bool pack.allowDuplicateObjects)\" = true &&\n+\ttest \"$(git -C clone-allow rev-parse HEAD)\" = \"$commit\" &&\n+\tgit -C clone-allow fsck --full\n+'\n+\n test_done\ndiff --git a/t/t5309-pack-delta-cycles.sh b/t/t5309-pack-delta-cycles.sh\nindex f613950e38..fb5ed81ca3 100755\n--- a/t/t5309-pack-delta-cycles.sh\n+++ b/t/t5309-pack-delta-cycles.sh\n@@ -217,6 +217,16 @@ test_expect_success 'failover after a tail into a three-object delta cycle' '\n \tcheck_blob \"$T\" tail\n '\n \n+test_expect_success 'configured connectivity check recovers duplicate delta bases' '\n+\tclear_packs &&\n+\tgit -c pack.allowDuplicateObjects index-pack --fix-thin \\\n+\t\t--check-self-contained-and-connected \\\n+\t\t--stdin expect.duplicate-base &&\n+\tcheck_blob \"$A\" expect.duplicate-base &&\n+\tgit cat-file -e \"$B\"\n+'\n+\n test_expect_success 'index-pack works with thin pack A->B->C with B on disk' '\n \tgit init server &&\n \t(\n\nbase-commit: fd2739b159d075cd4c6fa69b2cd876ba1caa5c88\n"},{"id":"549179","messageId":"amk7T6N5XhArUQwo@com-79390","threadId":"66076","inReplyTo":"20260728042550.91133-2-friel@openai.com","subject":"Re: [RFC PATCH] index-pack: optionally allow duplicate objects","fromName":"Taylor Blau","fromEmail":"ttaylorr@openai.com","sentAt":"2026-07-28T23:29:19Z","receivedAt":"2026-07-28T23:29:24Z","isPatch":true,"body":"On Mon, Jul 27, 2026 at 09:25:32PM -0700, friel@openai.com wrote:\n> Git's upload-pack normally uses pack-objects to select each reachable\n> object once before writing a response. A server can instead construct\n> that response by streaming entries from existing packs. When those\n> packs overlap, the same object can appear more than once.\n>\n> Avoiding duplicates requires the producers to coordinate their object\n> selection or track object IDs across all input packs. A duplicate can\n> also be used as a delta base, so removing it can require buffering and\n> rewriting the response. Doing that work at request time gives up the\n> memory and latency benefits of streaming existing packs.\n\nRight. An out-of-tree implementation of upload-pack may choose to stitch\nmultiple individual packs together by concatenating them, trading some\npack generation time for a pack which may contain duplicate objects.\n\nWhile this series is primarily motivated by that use-case, I suspect\nthat there are optimizations we could make within Git's implementation\nof upload-pack that would take advantage of environments where clients\nare prepared to accept packs that contain duplicate objects.\n\nThat's not a goal of this patch, of course, but something to keep in\nmind as others review this.\n\n> Applies on top of tb/pack-with-duplicates.\n>\n> Documentation/config/pack.adoc | 11 ++\n> Documentation/git-index-pack.adoc | 12 ++-\n> builtin/index-pack.c | 41 ++++++-\n> t/t5308-pack-detect-duplicates.sh | 171 ++++++++++++++++++++++++++++++\n> t/t5309-pack-delta-cycles.sh | 10 ++\n> 5 files changed, 241 insertions(+), 4 deletions(-)\n>\n> diff --git a/Documentation/config/pack.adoc b/Documentation/config/pack.adoc\n> index 22384c2d2f..2229878abe 100644\n> --- a/Documentation/config/pack.adoc\n> +++ b/Documentation/config/pack.adoc\n> @@ -39,6 +39,17 @@ is set to \"multi\", reuse parts of just the bitmapped packfile. This\n> can reduce memory and CPU usage to serve fetches, but might result in\n> sending a slightly larger pack. Defaults to true.\n>\n> +pack.allowDuplicateObjects::\n> +\tAllow linkgit:git-index-pack[1] to accept a pack containing\n> +\tmultiple copies of the same object while checking that the pack\n> +\tis self-contained and connected. For example,\n> +\t`git clone -c pack.allowDuplicateObjects ` can accept\n> +\ta pack generated from overlapping existing packs. Object and\n> +\tconnectivity checks are preserved. Explicit `--strict` and\n> +\t`--verify` continue to reject duplicate objects.\n> +\t`--no-allow-duplicate-objects` overrides this setting.\n> +\tDefaults to `false`.\n> +\n\nA couple of brief thoughts here:\n\n - Is \"while checking that the pack is self-contained and connected\"\n true in all cases? Certainly if we give the option\n '--check-self-contained-any-connected' to 'index-pack'. But if\n a user invokes \"git -c pack.allowDuplicateObjects index-pack ...\",\n we will not bother to perform the same checks.\n\n - The \"For example [...]\" may be unnecessary here. I don't have a\n strong feeling here either way, but it feels somewhat specific to\n 'git-clone(1)' so perhaps belongs there instead?\n\n - \"Explicit `--strict` and `--verify` [...]\" and the following\n sentence. I think that this means to suggest that `--strict` and\n `--verify` both continue to behave as-is, but setting this\n configuration option allows them to conditionally accept\n otherwise-good packs that happen to contain duplicate objects.\n\n I wonder if these couple of sentences may be combined like: \"When\n `true`, linkgit:git-index-pack[1] will accept otherwise-valid packs\n containing duplicate objects under `--strict` or `--verify`.\" But\n reading further, I don't think that that's actually what this option\n does. More below.\n\n> pack.island::\n> \tAn extended regular expression configuring a set of delta\n> \tislands. See \"DELTA ISLANDS\" in linkgit:git-pack-objects[1]\n> diff --git a/Documentation/git-index-pack.adoc b/Documentation/git-index-pack.adoc\n> index 18036953c0..1cb11ff898 100644\n> --- a/Documentation/git-index-pack.adoc\n> +++ b/Documentation/git-index-pack.adoc\n> @@ -11,7 +11,9 @@ SYNOPSIS\n> [verse]\n> 'git index-pack' [-v] [-o ] [--[no-]rev-index] \n> 'git index-pack' --stdin [--fix-thin] [--keep] [-v] [-o ]\n> -\t\t [--[no-]rev-index] []\n> +\t\t [--[no-]rev-index]\n> +\t\t [--[no-]allow-duplicate-objects]\n> +\t\t []\n\nNot the fault of this patch, but the synopsis and usage string\n(`index_pack_usage`) do not agree, hence the 'index-pack' entry in\nt/t0450/adoc-help-mismatches. So putting this on a new line is OK, but I\nthink it's fine to keep this and \"[]\" on the same line as\n\"[--[no-]rev-index]\" in the pre-image of this patch.\n\n> DESCRIPTION\n> @@ -97,6 +99,14 @@ default and \"Indexing objects\" when `--stdin` is specified.\n> --check-self-contained-and-connected::\n> \tDie if the pack contains broken links. For internal use only.\n>\n> +--allow-duplicate-objects::\n> +--no-allow-duplicate-objects::\n> +\tAllow or reject multiple copies of the same object while checking\n> +\tthat the pack is self-contained and connected. The default is\n> +\tcontrolled by `pack.allowDuplicateObjects`. The command-line\n> +\toption overrides the configuration. `--allow-duplicate-objects`\n> +\tcannot be combined with `--strict` or `--verify`.\n\nHmm. This suggests something other than what I gathered when reading the\ncorresponding git-config(1) entry.\n\nAre there cases where we would want want to allow duplicate object,s but\nretain the other \"--strict\" behavior of dying when the pack contains\nbroken objects, or links off to objects that we don't have? I would\nimagine that 'git clone' would want to do just this. I imagine that such\na use-case would expect that even if we are cloning from a source that\nis known to produce packs with duplicate objects we would still want to\nverify that none of the objects it references are missing, etc.\n\nI think that suggests something more along the lines of having this\noption opt you out of this specific portion of \"--strict\"'s behavior, as\nin \"git index-pack --strict --allow-duplicate-objects\". I may be missing\nsomething here.\n\n> @@ -135,6 +135,11 @@ static int nr_threads;\n>\n> static int from_stdin;\n> static int strict;\n> +static enum {\n> +\tDUPLICATE_OBJECTS_REJECT = 0,\n> +\tDUPLICATE_OBJECTS_ALLOW_CONFIG,\n> +\tDUPLICATE_OBJECTS_ALLOW_OPTION,\n> +} allow_duplicate_objects;\n\nI was initially a little surprised to see a new enum value here for what\nI imagined would be a true/false value. But looking at the diff below, I\nthink that this is to silently ignore a \"true\" value for the config\noption 'pack.allowDuplicateObjects' in the presence of \"--strict\".\n\nSo I think that this tri-state is fine in that sense. But I imagine that\nmuch of this goes away if we take this option to instead carve out one\nspecific behavior of --strict instead of being incompatible with it\nentirely.\n\n> +\tif (write_idx_strict &&\n> +\t allow_duplicate_objects == DUPLICATE_OBJECTS_ALLOW_OPTION)\n> +\t\tdie(_(\"options '%s' and '%s' cannot be used together\"),\n> +\t\t \"--allow-duplicate-objects\", \"--strict\");\n> +\tif (verify &&\n> +\t allow_duplicate_objects == DUPLICATE_OBJECTS_ALLOW_OPTION)\n> +\t\tdie(_(\"options '%s' and '%s' cannot be used together\"),\n> +\t\t \"--allow-duplicate-objects\", \"--verify\");\n\nIf you end up keeping the existing meaning and need to declare this\nincompatible with write_idx_strict and verify, there is a helper for\nthis case:\n\n die_for_incompatible_opt2(allow_duplicate_objects == DUPLICATE_OBJECTS_ALLOW_OPTION,\n \"--allow-duplicate-objects\",\n write_idx_strict, \"--strict\");\n\n die_for_incompatible_opt2(allow_duplicate_objects == DUPLICATE_OBJECTS_ALLOW_OPTION,\n \"--allow-duplicate-objects\",\n verify, \"--verify\");\n\nAlternatively, since writing \"allow_duplicate_objects == DUPLICATE_OBJECTS_ALLOW_OPTION\"\nis kind of a mouthful, you could instead write this abomination:\n\n if (allow_duplicate_objects == DUPLICATE_OBJECTS_ALLOW_OPTION) {\n die_for_incompatible_opt2(1, \"--allow-duplicate-objects\",\n write_idx_strict, \"--strict\");\n die_for_incompatible_opt2(1, \"--allow-duplicate-objects\",\n verify, \"--verify\");\n }\n\n;-)\n\n> @@ -2055,7 +2090,7 @@ int cmd_index_pack(int argc,\n> \t\tread_idx_option(&opts, index_name);\n> \t\topts.flags |= WRITE_IDX_VERIFY | WRITE_IDX_STRICT;\n> \t}\n> -\tif (strict)\n> +\tif (write_idx_strict)\n> \t\topts.flags |= WRITE_IDX_STRICT;\n>\n> \tif (HAVE_THREADS && !nr_threads) {\n\nOK. Since we aren't treating this as a carve-out, we don't have any\nfurther changes in pack-write.c. Makes sense, though I am curious about\nyour thoughts on whether the alternate interface makes more or less\nsense.\n\n> diff --git a/t/t5308-pack-detect-duplicates.sh b/t/t5308-pack-detect-duplicates.sh\n> index c6273a1aeb..95c81fa7b4 100755\n> --- a/t/t5308-pack-detect-duplicates.sh\n> +++ b/t/t5308-pack-detect-duplicates.sh\n\nI haven't read the tests carefully (under the assumption that they may\nchange substantively if the meaning of \"--allow-duplicate-objects\" is\naltered). But from skimming, I wonder if there is some room to shrink\nthe number of tests.\n\nWhen working with an agent, I typically ask it to implement the minimal\nnumber of tests, along with a prompt that it must demonstrate that those\ntests still exercise all interesting behavior. Often I will repeat this\na number of times until I am similarly convinced.\n\nThanks,\nTaylor\n"},{"id":"549185","messageId":"xmqqik5ybmi9.fsf@gitster.g","threadId":"66076","inReplyTo":"20260728042550.91133-2-friel@openai.com","subject":"Re: [RFC PATCH] index-pack: optionally allow duplicate objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-29T01:41:34Z","receivedAt":"2026-07-29T01:41:37Z","isPatch":true,"body":"friel@openai.com writes:\n\n> Signed-off-by: Friel \n> ---\n> Applies on top of tb/pack-with-duplicates.\n\nI am really reluctant to take us in this direction. The last time I\nhad a deep discussion on this was with Shawn Pearce (so those who\nknew him can tell how long ago that was), and the essence of his\nsuggestion was that allowing malformed or invalid packfiles is a\nslippery slope. They complicate everything, from delta cycle\ndetection to ensuring that repository data stays healthy.\n\nChanges that help us detect such a broken pack as early as possible\nand prevent it from entering your repository are very much welcome.\nChanges that accept such a broken pack as if nothing were wrong, not\nso much.\n\nThanks.\n"},{"id":"549228","messageId":"ampR7FkErK3CQPyC@com-79390","threadId":"66076","inReplyTo":"xmqqik5ybmi9.fsf@gitster.g","subject":"Re: [RFC PATCH] index-pack: optionally allow duplicate objects","fromName":"Taylor Blau","fromEmail":"ttaylorr@openai.com","sentAt":"2026-07-29T19:18:04Z","receivedAt":"2026-07-29T19:18:10Z","isPatch":true,"body":"On Tue, Jul 28, 2026 at 06:41:34PM -0700, Junio C Hamano wrote:\n> friel@openai.com writes:\n>\n> > Signed-off-by: Friel \n> > ---\n> > Applies on top of tb/pack-with-duplicates.\n>\n> I am really reluctant to take us in this direction. The last time I\n> had a deep discussion on this was with Shawn Pearce (so those who\n> knew him can tell how long ago that was), and the essence of his\n> suggestion was that allowing malformed or invalid packfiles is a\n> slippery slope. They complicate everything, from delta cycle\n> detection to ensuring that repository data stays healthy.\n\nI share the concern, but I am not sure to what extent historical\ndiscussions should decide our stance on whether or not we want to\nprovide support for packs containing duplicate objects.\n\nA lot of this discussion pre-dates my involvement with the project, but\nthe closest thread I could find on the topic with Shawn was from back in\n[1]. AFAICT the main concern at the time was that binary searching the\npack would not handle duplicates well.\n\nYour comment here around delta cycle detection is something that came up\nin the series I wrote (upon which this patch is based) trying to harden\nGit's handling of packs containing duplicate objects. The main thing I\nfound when writing that series that Git did *not* already handle well\nwas navigating cycles during delta resolution, in particular with\nREF_DELTAs. One of the patches in that series teaches Git how to handle\nthis case in a way that (a) doesn't adversely impact the performance of\ndelta resolution in packs that do *not* contain duplicates, and (b) is\nnot especially complex.\n\nMy sense is that many of the things the project was concerned with at\nthe time have since been hardened, and that Git is more well-equipped to\ndeal with duplicate object-containing packs than we may give it credit\nfor.\n\nA couple of thoughts on why I think this direction is useful:\n\n - I think that Peff makes a good point in [2], which effectively boils\n down to, \"OK, maybe these packs are buggy, but if they contain the\n sole copy of an object we care about, we must be able to read them.\"\n As I understand it, that is effectively why we disambiguate between\n \"index-pack\" and \"index-pack --strict\".\n\n I think a version of this patch that carves out duplicates as OK even\n in \"--strict\" mode results in better behavior for packs that contain\n duplicate objects (whereas before we had to drop all of the\n additional checks that \"--strict\" requires in order to index such a\n pack).\n\n - There are genuinely useful scenarios by which a sender may wish to\n consolidate two or more packs together by appending (as opposed to\n repacking) them, in a way that is extremely cheap to do.\n\n Friel may have a better example here, but a useful mental model for\n me has been: if an upload-pack implementation knows that it can serve\n a request by sending the objects from some known subset of all packs,\n it may make sense to simply combine those packs by appending them\n rather than generating a new pack with the union of their objects.\n\n That trade-off is useful IMHO for serving fetches and clones for very\n large and fast-moving repositories where repacking may not always be\n able to keep up.\n\nI am not proposing that we make packs containing duplicate objects the\nnorm. But mine and Friel's approach here is to first demonstrate (via\n'tb/pack-with-duplicates') that Git has good handling for packs\ncontaining duplicate objects, and subsequently (via this patch) to make\nit possible to index such packs.\n\nIf we can find useful ways to combine the ideas above with Git's in-tree\nimplementation of upload-pack, one could imagine that Git itself may\neventually send packs containing duplicate copies of some object(s)\nbehind a capability. In other words, for clients that know how to\nprocess such a pack, the server may wish to ask the client to do just\nthat in the name of saving some CPU cycles necessary to generate a pack\nthat doesn't have any duplicate objects.\n\n> Changes that help us detect such a broken pack as early as possible\n> and prevent it from entering your repository are very much welcome.\n> Changes that accept such a broken pack as if nothing were wrong, not\n> so much.\n\nI agree that the RFC as written makes the exception look broader than it\nshould.\n\nA reroll should make `--allow-duplicate-objects` carve out only the\nduplicate-OID part of `--strict`, rather than make the two options\nincompatible. `index-pack` should still recompute every object ID,\nresolve every delta, perform the usual object and link checks, and check\nconnectivity when requested. Only `WRITE_IDX_STRICT`'s requirement that\neach OID have one physical entry would be relaxed. The default should,\nIMHO, remain that we reject such packs.\n\nBut I would note that having packs containing duplicate objects is not a\nnew repository state for Git. Non-strict `index-pack` accepts duplicate\nentries today, and shallow and filtered clones can store the same pack.\nMy series in 'tb/pack-with-duplicates' attempts to fix the known-broken\nassumptions in reverse indexes, delta resolution, MIDX verification, and\nbitmap reuse because those packs can already exist.\n\nIf duplicate entries are to be forbidden entirely in order for a pack to\nbe considered valid, then I think we should reject them in every\n`index-pack` mode, including the shallow and filtered clone paths, and\ndiagnose existing packs in `fsck`. Otherwise, a default-off opt-in for a\nknown producer is a cleaner way IMHO to express that duplicate objects\nare OK, rather than relying on whether or not we performed a\nshallow/partial clone (and thus did not invoke index-pack with\n\"--strict\").\n\nI think that narrower version is worth pursuing.\n\nThanks,\nTaylor\n\n[1]: https://lore.kernel.org/git/CAJo=hJs3mM7=LcOop-WD=bipA=Wx-7MDh6ObQwFUE38tjurvcw@mail.gmail.com/\n[2]: https://lore.kernel.org/git/20140830131649.GA26833@peff.net/\n"},{"id":"549230","messageId":"xmqqtspho7tk.fsf@gitster.g","threadId":"66076","inReplyTo":"ampR7FkErK3CQPyC@com-79390","subject":"Re: [RFC PATCH] index-pack: optionally allow duplicate objects","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-29T20:32:39Z","receivedAt":"2026-07-29T20:32:41Z","isPatch":true,"body":"Taylor Blau writes:\n\n> If we can find useful ways to combine the ideas above with Git's in-tree\n> implementation of upload-pack, one could imagine that Git itself may\n> eventually send packs containing duplicate copies of some object(s)\n> behind a capability. In other words, for clients that know how to\n> process such a pack, the server may wish to ask the client to do just\n> that in the name of saving some CPU cycles necessary to generate a pack\n> that doesn't have any duplicate objects.\n\nI can live with such an extension as long as we teach the receiving\nend to deduplicate the extra copy. Leaving packs with duplicate\nobjects on disk is a completely different story, as it will become a\nsource of spreading such broken packs elsewhere, though.\n\n> But I would note that having packs containing duplicate objects is not a\n> new repository state for Git. Non-strict `index-pack` accepts duplicate\n> entries today, and shallow and filtered clones can store the same pack.\n\nThe same as what???\n\n> My series in 'tb/pack-with-duplicates' attempts to fix the known-broken\n> assumptions in reverse indexes, delta resolution, MIDX verification, and\n> bitmap reuse because those packs can already exist.\n>\n> If duplicate entries are to be forbidden entirely in order for a pack to\n> be considered valid, then I think we should reject them in every\n> `index-pack` mode, including the shallow and filtered clone paths, and\n> diagnose existing packs in `fsck`.\n\nYup, I think that would be a sensible longer-term direction. We may\nneed a bit more tool support to \"fix\" by reindexing at the receiving\nend, though. As you say, \"cancatenate two packs, damn the duplicates\"\nmay be a cheap way for server side to give union of objects contained\nin these two packs, but doing so without even measuring how much they\nare duplicating cannot go on forever unchecked. Somebody needs to\nremove these duplicates, and the time the downloader indexes the\nincoming pack would be the best place to do so. It needs to read\neach and every object in the pack stream to make the .idx file out\nof the stream anyway.\n"},{"id":"549231","messageId":"20260729211716.40166-1-friel@openai.com","threadId":"66076","inReplyTo":"xmqqtspho7tk.fsf@gitster.g","subject":"Re: [RFC PATCH] index-pack: optionally allow duplicate objects","fromName":"","fromEmail":"friel@openai.com","sentAt":"2026-07-29T21:17:16Z","receivedAt":"2026-07-29T21:17:20Z","isPatch":true,"body":"From: Friel \n\nOn Wed, Jul 29, 2026 at 01:32:39PM -0700, Junio C Hamano wrote:\n\n> I can live with such an extension as long as we teach the receiving\n> end to deduplicate the extra copy.\n\nThat makes sense. We don't want packs containing duplicate objects to\nbecome a persistent source of duplicate objects in other repositories.\n\nFor our server, duplicate objects would be an exceptional consequence of\nan optimization, not normal operation. We have not seen duplicates in\npractice yet. But preventing them imposes a cost on every upload-pack\nrequest even when duplicates are rare.\n\nI'll talk with Taylor about whether the client should repack when it\ndetects duplicates, or whether Git already has a way to mark such a pack\nas dirty for reuse or retransmission until it has been cleaned up.\nThe intent would be to pay that cost only when duplicates actually\noccur.\n\nIn all humility, thank you for reviewing and considering the RFC PATCH.\nI'm still getting acquainted with mailing list and the history of Git,\nand I'm happy to have Taylor & Ted's help.\n\nCheers,\nFriel\n"},{"id":"549233","messageId":"ampvWrDaNqmNdlUm@com-79390","threadId":"66076","inReplyTo":"xmqqtspho7tk.fsf@gitster.g","subject":"Re: [RFC PATCH] index-pack: optionally allow duplicate objects","fromName":"Taylor Blau","fromEmail":"ttaylorr@openai.com","sentAt":"2026-07-29T21:23:38Z","receivedAt":"2026-07-29T21:23:42Z","isPatch":true,"body":"On Wed, Jul 29, 2026 at 01:32:39PM -0700, Junio C Hamano wrote:\n> Taylor Blau writes:\n>\n> > If we can find useful ways to combine the ideas above with Git's in-tree\n> > implementation of upload-pack, one could imagine that Git itself may\n> > eventually send packs containing duplicate copies of some object(s)\n> > behind a capability. In other words, for clients that know how to\n> > process such a pack, the server may wish to ask the client to do just\n> > that in the name of saving some CPU cycles necessary to generate a pack\n> > that doesn't have any duplicate objects.\n>\n> I can live with such an extension as long as we teach the receiving\n> end to deduplicate the extra copy. Leaving packs with duplicate\n> objects on disk is a completely different story, as it will become a\n> source of spreading such broken packs elsewhere, though.\n\nI am trying to nudge us in the direction of reconsidering whether a\npack containing duplicate objects *is* broken. After reading some of the\nhistorical discussions on the list, the only \"broken\" portion here is\nclient-side support, which is what my series is trying to address.\n\n> > But I would note that having packs containing duplicate objects is not a\n> > new repository state for Git. Non-strict `index-pack` accepts duplicate\n> > entries today, and shallow and filtered clones can store the same pack.\n>\n> The same as what???\n\nThe same pack meaning the one which contains duplicate objects. As I\nunderstand, Friel configured clients to have a shallow depth equal to\nthe 32-bit unsigned maximum value (which is gross), but does cause us to\nrun \"index-pack\" without \"--strict\".\n\nThanks,\nTaylor\n"},{"id":"549234","messageId":"ampwoImYYKeYzkw7@com-79390","threadId":"66076","inReplyTo":"20260729211716.40166-1-friel@openai.com","subject":"Re: [RFC PATCH] index-pack: optionally allow duplicate objects","fromName":"Taylor Blau","fromEmail":"ttaylorr@openai.com","sentAt":"2026-07-29T21:29:04Z","receivedAt":"2026-07-29T21:29:09Z","isPatch":true,"body":"On Wed, Jul 29, 2026 at 02:17:16PM -0700, friel@openai.com wrote:\n> From: Friel \n>\n> On Wed, Jul 29, 2026 at 01:32:39PM -0700, Junio C Hamano wrote:\n>\n> > I can live with such an extension as long as we teach the receiving\n> > end to deduplicate the extra copy.\n>\n> That makes sense. We don't want packs containing duplicate objects to\n> become a persistent source of duplicate objects in other repositories.\n>\n> For our server, duplicate objects would be an exceptional consequence of\n> an optimization, not normal operation. We have not seen duplicates in\n> practice yet. But preventing them imposes a cost on every upload-pack\n> request even when duplicates are rare.\n\nTo Junio's point about \"these cannot go forever unchecked\", I agree, and\nI think this is an important internal detail which I may not have made\nclear. We don't expect to send packs containing duplicate objects as a\ngeneral case, but this patch and my series are a defensive measure to\nmake sure clients don't immediately choke on them.\n\n> I'll talk with Taylor about whether the client should repack when it\n> detects duplicates, or whether Git already has a way to mark such a pack\n> as dirty for reuse or retransmission until it has been cleaned up.\n> The intent would be to pay that cost only when duplicates actually\n> occur.\n\nLet me think a little bit more about this. It would be a shame to have\nto repack the entirety of the pack when there are only a few (or zero)\nduplicate object entries. But if we can efficiently pluck them out of the stream\nwhen fetching/cloning, that may be worthwhile.\n\nFWIW, (and I'm biased, but) I still think the series this patch is based\non is worth picking up. I think it makes sense to have better support\nwhen we *do* happen to see duplicate objects (e.g., to recover the only\ngood copy of some object you have, similar to Peff's argument quoted\nearlier in the thread).\n\nBut \"better support\" can coexist with \"...you still shouldn't do this\".\n\n> In all humility, thank you for reviewing and considering the RFC PATCH.\n> I'm still getting acquainted with mailing list and the history of Git,\n> and I'm happy to have Taylor & Ted's help.\n\nThanks for saying that. Welcome to the list :-).\n\nThanks,\nTaylor\n"}]}