{"thread":{"id":"66747","subject":"[PATCH] rev-list: ignore replacements in the connectivity check","startedAt":"2026-11-02T15:47:09Z","lastAt":"2026-11-03T18:36:55Z","messageCount":9,"participants":["Kristofer Karlsson via GitGitGadget","Junio C Hamano","Kristofer Karlsson","Karthik Nayak","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"557197","messageId":"pull.2279.git.1793634429929.gitgitgadget@gmail.com","threadId":"66747","inReplyTo":null,"subject":"[PATCH] rev-list: ignore replacements in the connectivity check","fromName":"Kristofer Karlsson via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-11-02T15:47:09Z","receivedAt":"2026-11-02T15:47:09Z","isPatch":true,"sender":{"key":"krka@spotify.com","avatar":"https://avatars.githubusercontent.com/u/5349266?v=4"},"body":"From: Kristofer Karlsson <krka@spotify.com>\n\nThe connectivity check follows replace refs, but other operations\nthat depend on the repository being complete, such as fsck, prune,\npack-objects and upload-pack, do not. They were taught to ignore\nreplacements in dae556bdb1 (environment: add global variable to\ndisable replacement, 2009-01-23), as they must work with the\noriginal objects, and check_connected() came later without doing\nthe same.\n\nTeach the connectivity check to ignore replacements as well, so\nthat it verifies the same objects that the commands relying on its\nresult will read. This is not a problem seen in practice, but the\ntwo can disagree. If a replace ref names an object that is not yet\npresent locally, the check follows the replacement instead of that\nobject when it arrives, and accepts an incoming pack that lacks\nobjects below it. fsck and repack then fail. Such a replace ref can\nbe the result of fetching refs/replace/* without the branch that\ncontains the replaced commit, which fsck does not complain about,\nbut it still takes a sender that sends an incomplete pack.\n\ncheck_connected() passes --connectivity-check to its rev-list\nsubprocess, so make rev-list disable replacements when that option\nis given, for both the full and the incremental check. Also drop\nOBJECT_INFO_LOOKUP_REPLACE from the object reads in tree-verify.c,\nused by the incremental check, to match. Disabling replacements\nalready overrides the flag, but it suggests the opposite intent.\n\nThe t5413 test that expected the check to accept a commit whose\noriginal tree is incomplete because a replacement is complete now\nexpects a rejection, and the test that disabled replacements\nexplicitly to get one is redundant.\n\nSigned-off-by: Kristofer Karlsson <krka@spotify.com>\n---\n    rev-list: ignore replacements in the connectivity check\n    \n    I noticed this while working on the incremental connectivity check [1].\n    I could not find any discussion of why check_connected() follows replace\n    refs when fsck, prune, pack-objects and upload-pack do not, and the\n    history suggests that it was never a deliberate choice.\n    \n    This is meant both as a question and a proposed patch: should the\n    connectivity check ignore replacements like those commands do, or is\n    there a reason for it to respect them? If there is a good reason to keep\n    the current behavior, I don't have any issue with dropping the patch\n    directly -- you would not even have to look at the code :) If not, I\n    think aligning them is a reasonable thing to do.\n    \n    The case described in the commit message, where the two disagree, is\n    tricky to trigger in practice, so I do not expect it to be a common\n    problem. The main benefit is logical consistency.\n    \n    Shallow boundaries are not affected, since they use the graft mechanism\n    internally, not replace refs.\n    \n    [1]\n    https://lore.kernel.org/git/pull.2211.v4.git.1792581744.gitgitgadget@gmail.com/\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2279%2Fspkrka%2Freplace-connectivity-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2279/spkrka/replace-connectivity-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2279\n\n Documentation/rev-list-options.adoc |  1 +\n builtin/rev-list.c                  |  3 +++\n t/t5413-connectivity-check.sh       | 11 +----------\n tree-verify.c                       |  2 --\n 4 files changed, 5 insertions(+), 12 deletions(-)\n\ndiff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc\nindex 285fb3809f..b2c0d394ab 100644\n--- a/Documentation/rev-list-options.adoc\n+++ b/Documentation/rev-list-options.adoc\n@@ -1107,6 +1107,7 @@ It is an error to use `--missing-only` without `--missing=print` or\n \t(For internal use only.)  Used by `check_connected()` to\n \tselect the connectivity check algorithm; see\n \t`transfer.connectivityCheck` in linkgit:git-config[1].\n+\tReplacement objects are ignored, as with `--no-replace-objects`.\n endif::git-rev-list[]\n \n `--no-walk[=(sorted|unsorted)]`::\ndiff --git a/builtin/rev-list.c b/builtin/rev-list.c\nindex 071a0e3cb5..bf66d2c9ae 100644\n--- a/builtin/rev-list.c\n+++ b/builtin/rev-list.c\n@@ -30,6 +30,7 @@\n #include \"strbuf.h\"\n #include \"tree-verify.h\"\n #include \"parse.h\"\n+#include \"replace-object.h\"\n \n #define INCREMENTAL_MAX_COMMITS 1000\n \n@@ -807,6 +808,8 @@ int cmd_rev_list(int argc,\n \t\t\trevs.exclude_promisor_objects = 1;\n \t\t} else if (skip_prefix(arg, \"--connectivity-check=\", &arg)) {\n \t\t\tparse_connectivity_check(arg, &connectivity_check);\n+\t\t\t/* Check the original objects, like fsck and pack-objects do. */\n+\t\t\tdisable_replace_refs();\n \t\t} else if (skip_prefix(arg, \"--missing=\", &arg)) {\n \t\t\tparse_missing_action_value(repo, arg);\n \t\t} else if (!strcmp(arg, \"--missing-only\")) {\ndiff --git a/t/t5413-connectivity-check.sh b/t/t5413-connectivity-check.sh\nindex 60edf99b63..341638a433 100755\n--- a/t/t5413-connectivity-check.sh\n+++ b/t/t5413-connectivity-check.sh\n@@ -427,18 +427,9 @@ test_expect_success \"$mode: handles directory-to-file transition\" '\n \n # Replacement objects.\n \n-test_expect_success \"$mode: accepts with replacement objects\" '\n+test_expect_success \"$mode: ignores replacement objects\" '\n \t(\n \t\tcd replace-test &&\n-\t\tcheck_connected \"$original_oid\"\n-\t)\n-'\n-\n-test_expect_success \"$mode: rejects without replacement objects\" '\n-\t(\n-\t\tcd replace-test &&\n-\t\tGIT_NO_REPLACE_OBJECTS=1 &&\n-\t\texport GIT_NO_REPLACE_OBJECTS &&\n \t\ttest_expect_code 128 check_connected \\\n \t\t\t\"$original_oid\" 2>err &&\n \t\ttest_grep \"missing blob object.*$orig_blob_oid\" err\ndiff --git a/tree-verify.c b/tree-verify.c\nindex 25db76d30b..c228235150 100644\n--- a/tree-verify.c\n+++ b/tree-verify.c\n@@ -133,7 +133,6 @@ static int verify_blob(struct repository *repo,\n \tvs->blobs_checked++;\n \toi.typep = &type;\n \tif (odb_read_object_info_extended(repo->objects, oid, &oi,\n-\t\t\t\t\t  OBJECT_INFO_LOOKUP_REPLACE |\n \t\t\t\t\t  OBJECT_INFO_SKIP_FETCH_OBJECT)) {\n \t\tif (vs->exclude_promisor_objects &&\n \t\t    is_promisor_object(repo, oid))\n@@ -163,7 +162,6 @@ static int read_tree_object(struct object_database *odb,\n \toi.sizep = sizep;\n \toi.contentp = bufp;\n \tif (odb_read_object_info_extended(odb, oid, &oi,\n-\t\t\t\t\t  OBJECT_INFO_LOOKUP_REPLACE |\n \t\t\t\t\t  OBJECT_INFO_SKIP_FETCH_OBJECT))\n \t\t*bufp = NULL;\n \tif (*bufp && type != OBJ_TREE) {\n\nbase-commit: ebdc087dc99d68bb340c282b2ba5f722def28d19\n-- \ngitgitgadget\n\n"},{"id":"557242","messageId":"xmqq4idyhotf.fsf@gitster.g","threadId":"66747","inReplyTo":"pull.2279.git.1793634429929.gitgitgadget@gmail.com","subject":"Re: [PATCH] rev-list: ignore replacements in the connectivity check","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-11-02T22:05:16Z","receivedAt":"2026-11-02T22:05:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Kristofer Karlsson via GitGitGadget\" <gitgitgadget@gmail.com>\nwrites:\n\n> The connectivity check follows replace refs, but other operations\n> that depend on the repository being complete, such as fsck, prune,\n> pack-objects and upload-pack, do not. They were taught to ignore\n> replacements in dae556bdb1 (environment: add global variable to\n> disable replacement, 2009-01-23), as they must work with the\n> original objects, and check_connected() came later without doing\n> the same.\n>\n> Teach the connectivity check to ignore replacements as well, so\n> that it verifies the same objects that the commands relying on its\n> result will read.\n\nSounds like absolutely the right thing to do.  We might want to say\nthat we check both the original and the replacement, i.e., instead\nof ignoring replace refs, we ensure that the commits, trees, and\nblobs reachable from the replacement objects all exist by treating\nthose objects as if they were also at the tips of the connectivity\ngraph.\n\n> This is not a problem seen in practice, but the\n> two can disagree. If a replace ref names an object that is not yet\n> present locally, the check follows the replacement instead of that\n> object when it arrives, and accepts an incoming pack that lacks\n> objects below it. fsck and repack then fail. Such a replace ref can\n> be the result of fetching refs/replace/* without the branch that\n> contains the replaced commit, which fsck does not complain about,\n> but it still takes a sender that sends an incomplete pack.\n\n...\n\n>     The case described in the commit message, where the two disagree, is\n>     tricky to trigger in practice, so I do not expect it to be a common\n>     problem. The main benefit is logical consistency.\n\nGood.\n\n>     Shallow boundaries are not affected, since they use the graft mechanism\n>     internally, not replace refs.\n\nIn other words, they do not introduce more links that need to be\nchecked.  They just declare \"behind this commit you do not have to\ncheck as nobody is allowed to traverse past it\".\n\n> diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc\n> index 285fb3809f..b2c0d394ab 100644\n> --- a/Documentation/rev-list-options.adoc\n> +++ b/Documentation/rev-list-options.adoc\n> @@ -1107,6 +1107,7 @@ It is an error to use `--missing-only` without `--missing=print` or\n>  \t(For internal use only.)  Used by `check_connected()` to\n>  \tselect the connectivity check algorithm; see\n>  \t`transfer.connectivityCheck` in linkgit:git-config[1].\n> +\tReplacement objects are ignored, as with `--no-replace-objects`.\n>  endif::git-rev-list[]\n>  \n>  `--no-walk[=(sorted|unsorted)]`::\n> diff --git a/builtin/rev-list.c b/builtin/rev-list.c\n> index 071a0e3cb5..bf66d2c9ae 100644\n> --- a/builtin/rev-list.c\n> +++ b/builtin/rev-list.c\n> @@ -30,6 +30,7 @@\n>  #include \"strbuf.h\"\n>  #include \"tree-verify.h\"\n>  #include \"parse.h\"\n> +#include \"replace-object.h\"\n>  \n>  #define INCREMENTAL_MAX_COMMITS 1000\n>  \n> @@ -807,6 +808,8 @@ int cmd_rev_list(int argc,\n>  \t\t\trevs.exclude_promisor_objects = 1;\n>  \t\t} else if (skip_prefix(arg, \"--connectivity-check=\", &arg)) {\n>  \t\t\tparse_connectivity_check(arg, &connectivity_check);\n> +\t\t\t/* Check the original objects, like fsck and pack-objects do. */\n> +\t\t\tdisable_replace_refs();\n\nOK.  This is straightforward \"we want to verify the underlying, real\nobjects\".\n\n>  \t\t} else if (skip_prefix(arg, \"--missing=\", &arg)) {\n>  \t\t\tparse_missing_action_value(repo, arg);\n>  \t\t} else if (!strcmp(arg, \"--missing-only\")) {\n\n> diff --git a/tree-verify.c b/tree-verify.c\n> index 25db76d30b..c228235150 100644\n> --- a/tree-verify.c\n> +++ b/tree-verify.c\n> @@ -133,7 +133,6 @@ static int verify_blob(struct repository *repo,\n>  \tvs->blobs_checked++;\n>  \toi.typep = &type;\n>  \tif (odb_read_object_info_extended(repo->objects, oid, &oi,\n> -\t\t\t\t\t  OBJECT_INFO_LOOKUP_REPLACE |\n>  \t\t\t\t\t  OBJECT_INFO_SKIP_FETCH_OBJECT)) {\n>  \t\tif (vs->exclude_promisor_objects &&\n>  \t\t    is_promisor_object(repo, oid))\n> @@ -163,7 +162,6 @@ static int read_tree_object(struct object_database *odb,\n>  \toi.sizep = sizep;\n>  \toi.contentp = bufp;\n>  \tif (odb_read_object_info_extended(odb, oid, &oi,\n> -\t\t\t\t\t  OBJECT_INFO_LOOKUP_REPLACE |\n>  \t\t\t\t\t  OBJECT_INFO_SKIP_FETCH_OBJECT))\n>  \t\t*bufp = NULL;\n>  \tif (*bufp && type != OBJ_TREE) {\n\nSo are these two other hunks.\n\nWe can address the \"shouldn't we make sure that replacement objects\nused when the resulting history is in use are backed by objects\nreachable from them?\" issue separately.\n\nThanks.\n\n"},{"id":"557276","messageId":"CAL71e4M7yFnTzYD0eVVGRmzkzSXbU0KZ-JScQOyB4-dB5xkAHA@mail.gmail.com","threadId":"66747","inReplyTo":"xmqq4idyhotf.fsf@gitster.g","subject":"Re: [PATCH] rev-list: ignore replacements in the connectivity check","fromName":"Kristofer Karlsson","fromEmail":"krka@spotify.com","sentAt":"2026-11-03T08:35:07Z","receivedAt":"2026-11-03T08:35:07Z","isPatch":true,"sender":{"key":"krka@spotify.com","avatar":"https://avatars.githubusercontent.com/u/5349266?v=4"},"body":"On Mon, 2 Nov 2026 at 23:05, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> \"Kristofer Karlsson via GitGitGadget\" <gitgitgadget@gmail.com>\n> writes:\n>\n> > Teach the connectivity check to ignore replacements as well, so\n> > that it verifies the same objects that the commands relying on its\n> > result will read.\n>\n> Sounds like absolutely the right thing to do.  We might want to say\n> that we check both the original and the replacement, i.e., instead\n> of ignoring replace refs, we ensure that the commits, trees, and\n> blobs reachable from the replacement objects all exist by treating\n> those objects as if they were also at the tips of the connectivity\n> graph.\n>\n> [snip]\n> We can address the \"shouldn't we make sure that replacement objects\n> used when the resulting history is in use are backed by objects\n> reachable from them?\" issue separately.\n\nI have not fully thought this through yet, but I'm happy\nto follow up with further strengthening if there's a gap here.\n\nMy preliminary thinking is that since replacements are\nimplemented as regular refs (refs/replace/OLD -> NEW),\ntheir reachable objects should already be covered by\nordinary connectivity checks when those refs are fetched\nor pushed.\n\nAnd thanks for the review!\nKristofer\n\n"},{"id":"557283","messageId":"CAOLa=ZTKaJhWHuKnSxL1pYN_JXdrr1Eohy8RepqFRc=Qph92Yg@mail.gmail.com","threadId":"66747","inReplyTo":"pull.2279.git.1793634429929.gitgitgadget@gmail.com","subject":"Re: [PATCH] rev-list: ignore replacements in the connectivity check","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2026-11-03T09:54:57Z","receivedAt":"2026-11-03T09:54:57Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"\"Kristofer Karlsson via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc\n> index 285fb3809f..b2c0d394ab 100644\n> --- a/Documentation/rev-list-options.adoc\n> +++ b/Documentation/rev-list-options.adoc\n> @@ -1107,6 +1107,7 @@ It is an error to use `--missing-only` without `--missing=print` or\n>  \t(For internal use only.)  Used by `check_connected()` to\n>  \tselect the connectivity check algorithm; see\n>  \t`transfer.connectivityCheck` in linkgit:git-config[1].\n> +\tReplacement objects are ignored, as with `--no-replace-objects`.\n>  endif::git-rev-list[]\n>\n>  `--no-walk[=(sorted|unsorted)]`::\n\nNit: Should we also modify 'Documentation/git-replace.adoc', which says:\n\n  Replacement references will be used by default by all Git commands\n  except those doing reachability traversal (prune, pack transfer and\n  fsck).\n"},{"id":"557285","messageId":"aum0rJ6T-OFWrMqW@pks.im","threadId":"66747","inReplyTo":"CAL71e4M7yFnTzYD0eVVGRmzkzSXbU0KZ-JScQOyB4-dB5xkAHA@mail.gmail.com","subject":"Re: [PATCH] rev-list: ignore replacements in the connectivity check","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-11-03T10:17:16Z","receivedAt":"2026-11-03T10:17:16Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Nov 03, 2026 at 09:35:07AM +0100, Kristofer Karlsson wrote:\n> On Mon, 2 Nov 2026 at 23:05, Junio C Hamano <gitster@pobox.com> wrote:\n> >\n> > \"Kristofer Karlsson via GitGitGadget\" <gitgitgadget@gmail.com>\n> > writes:\n> >\n> > > Teach the connectivity check to ignore replacements as well, so\n> > > that it verifies the same objects that the commands relying on its\n> > > result will read.\n> >\n> > Sounds like absolutely the right thing to do.  We might want to say\n> > that we check both the original and the replacement, i.e., instead\n> > of ignoring replace refs, we ensure that the commits, trees, and\n> > blobs reachable from the replacement objects all exist by treating\n> > those objects as if they were also at the tips of the connectivity\n> > graph.\n> >\n> > [snip]\n> > We can address the \"shouldn't we make sure that replacement objects\n> > used when the resulting history is in use are backed by objects\n> > reachable from them?\" issue separately.\n\nYeah, I was wondering whether we shouldn't verify both original and\nreplacement object. Previously we checked the latter, now we check the\nformer. But is either of those variants the correct thing to do?\n\n> I have not fully thought this through yet, but I'm happy\n> to follow up with further strengthening if there's a gap here.\n> \n> My preliminary thinking is that since replacements are\n> implemented as regular refs (refs/replace/OLD -> NEW),\n> their reachable objects should already be covered by\n> ordinary connectivity checks when those refs are fetched\n> or pushed.\n\nThat's fair, I guess. The question though is whether we can rely on that\nproperty when the replace ref was already written locally. But I'm not\nsure that we even _should_ care about that. It's arguably the\nresponsibility of the user that wrote the replace ref to make sure that\nthe new object is valid. And in the context of the connectivity check\ndone as part of a push we always assume that existing refs are fully\nconnected anyway.\n\nSo overall I'm aligned with this change.\n\nPatrick\n\n"},{"id":"557286","messageId":"aum0sTUAqc27DU_W@pks.im","threadId":"66747","inReplyTo":"pull.2279.git.1793634429929.gitgitgadget@gmail.com","subject":"Re: [PATCH] rev-list: ignore replacements in the connectivity check","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-11-03T10:17:21Z","receivedAt":"2026-11-03T10:17:21Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Nov 02, 2026 at 03:47:09PM +0000, Kristofer Karlsson via GitGitGadget wrote:\n[snip]\n> The t5413 test that expected the check to accept a commit whose\n> original tree is incomplete because a replacement is complete now\n> expects a rejection, and the test that disabled replacements\n> explicitly to get one is redundant.\n\nThis here makes me pause a bit. Do we know whether this specific test\nwas introduced intentionally to make a specific edge case work?\n\n> diff --git a/tree-verify.c b/tree-verify.c\n> index 25db76d30b..c228235150 100644\n> --- a/tree-verify.c\n> +++ b/tree-verify.c\n> @@ -133,7 +133,6 @@ static int verify_blob(struct repository *repo,\n>  \tvs->blobs_checked++;\n>  \toi.typep = &type;\n>  \tif (odb_read_object_info_extended(repo->objects, oid, &oi,\n> -\t\t\t\t\t  OBJECT_INFO_LOOKUP_REPLACE |\n>  \t\t\t\t\t  OBJECT_INFO_SKIP_FETCH_OBJECT)) {\n>  \t\tif (vs->exclude_promisor_objects &&\n>  \t\t    is_promisor_object(repo, oid))\n> @@ -163,7 +162,6 @@ static int read_tree_object(struct object_database *odb,\n>  \toi.sizep = sizep;\n>  \toi.contentp = bufp;\n>  \tif (odb_read_object_info_extended(odb, oid, &oi,\n> -\t\t\t\t\t  OBJECT_INFO_LOOKUP_REPLACE |\n>  \t\t\t\t\t  OBJECT_INFO_SKIP_FETCH_OBJECT))\n>  \t\t*bufp = NULL;\n>  \tif (*bufp && type != OBJ_TREE) {\n\nNit: this might have warranted a separate commit as it isn't even\nrelated to the connectivity check, is it? Did you also vet all the other\nsites that that verify objects?\n\nPatrick\n\n"},{"id":"557293","messageId":"CAL71e4M4qBJ6aHu-vr9Ex53LQsrpxjn9GRtkgNa8KSOSYWo4Lw@mail.gmail.com","threadId":"66747","inReplyTo":"aum0sTUAqc27DU_W@pks.im","subject":"Re: [PATCH] rev-list: ignore replacements in the connectivity check","fromName":"Kristofer Karlsson","fromEmail":"krka@spotify.com","sentAt":"2026-11-03T10:38:38Z","receivedAt":"2026-11-03T10:38:38Z","isPatch":true,"sender":{"key":"krka@spotify.com","avatar":"https://avatars.githubusercontent.com/u/5349266?v=4"},"body":"On Tue, 3 Nov 2026 at 11:17, Patrick Steinhardt <ps@pks.im> wrote:\n>\n> On Mon, Nov 02, 2026 at 03:47:09PM +0000, Kristofer Karlsson via GitGitGadget wrote:\n> [snip]\n> > The t5413 test that expected the check to accept a commit whose\n> > original tree is incomplete because a replacement is complete now\n> > expects a rejection, and the test that disabled replacements\n> > explicitly to get one is redundant.\n>\n> This here makes me pause a bit. Do we know whether this specific test\n> was introduced intentionally to make a specific edge case work?\n\nBoth the t5413 test and tree-verify.c were introduced as part\nof my (recent) incremental connectivity check series.  I\nnoticed the replacement \"oddity\" back then, but deliberately\nkept the existing behavior -- changing the semantics should be\nseparate and intentional.\n\n>\n> > diff --git a/tree-verify.c b/tree-verify.c\n> > index 25db76d30b..c228235150 100644\n> > --- a/tree-verify.c\n> > +++ b/tree-verify.c\n> > @@ -133,7 +133,6 @@ static int verify_blob(struct repository *repo,\n> >       vs->blobs_checked++;\n> >       oi.typep = &type;\n> >       if (odb_read_object_info_extended(repo->objects, oid, &oi,\n> > -                                       OBJECT_INFO_LOOKUP_REPLACE |\n> >                                         OBJECT_INFO_SKIP_FETCH_OBJECT)) {\n> >               if (vs->exclude_promisor_objects &&\n> >                   is_promisor_object(repo, oid))\n> > @@ -163,7 +162,6 @@ static int read_tree_object(struct object_database *odb,\n> >       oi.sizep = sizep;\n> >       oi.contentp = bufp;\n> >       if (odb_read_object_info_extended(odb, oid, &oi,\n> > -                                       OBJECT_INFO_LOOKUP_REPLACE |\n> >                                         OBJECT_INFO_SKIP_FETCH_OBJECT))\n> >               *bufp = NULL;\n> >       if (*bufp && type != OBJ_TREE) {\n>\n> Nit: this might have warranted a separate commit as it isn't even\n> related to the connectivity check, is it? Did you also vet all the other\n> sites that that verify objects?\n\ntree-verify.c is only used by rev-list\n--connectivity-check=incremental, so the flag removal is part\nof the same change: without it, the incremental check would\nstill nominally ask for replacements while the process has them\ndisabled.  The behavior would be the same, but it would look\nconfusing (why ask for replacements if they are always globally\ndisabled when this path is reached?)\n\nThis patch is intentionally limited to the connectivity check.\nI did review the other users of OBJECT_INFO_LOOKUP_REPLACE, but\nnone of them seem related to it.  Longer term it would be nice\nif each caller stated whether it wants replacements, rather\nthan relying on the global switch, but that is a much larger\nchange.\n\nOne of them does look like a separate issue, though: mktree\nchecks that each entry exists, following replacements, but\nwrites the original oid into the tree, so a missing object can\npass the check without --missing if it has a replace ref.  That\ncould be looked at separately as a bugfix maybe?  I have not\nlooked into it more though.\n\nThanks,\nKristofer\n\n"},{"id":"557295","messageId":"CAL71e4NYpaOqeXD2ZxgzM7g=7HiwEWoqBsPSTHd8NYJuPCHc0g@mail.gmail.com","threadId":"66747","inReplyTo":"CAOLa=ZTKaJhWHuKnSxL1pYN_JXdrr1Eohy8RepqFRc=Qph92Yg@mail.gmail.com","subject":"Re: [PATCH] rev-list: ignore replacements in the connectivity check","fromName":"Kristofer Karlsson","fromEmail":"krka@spotify.com","sentAt":"2026-11-03T10:51:30Z","receivedAt":"2026-11-03T10:51:30Z","isPatch":true,"sender":{"key":"krka@spotify.com","avatar":"https://avatars.githubusercontent.com/u/5349266?v=4"},"body":"On Tue, 3 Nov 2026 at 10:54, Karthik Nayak <karthik.188@gmail.com> wrote:\n>\n> Nit: Should we also modify 'Documentation/git-replace.adoc', which says:\n>\n>   Replacement references will be used by default by all Git commands\n>   except those doing reachability traversal (prune, pack transfer and\n>   fsck).\n\nI think that sentence is actually more accurate with this patch\nthan without it.  The connectivity check runs as part of pack\ntransfer (fetch and push), so until now that part of \"pack\ntransfer\" did follow replacements, contrary to what the\ndocumentation says.\n\nThe list was never exhaustive either: index-pack,\nunpack-objects, commit-graph and multi-pack-index also disable\nreplacements, so we can see it as a few examples of internal\noperations rather than a complete list.\n\nThanks,\nKristofer\n\n"},{"id":"557349","messageId":"xmqq7bit92yg.fsf@gitster.g","threadId":"66747","inReplyTo":"CAL71e4M7yFnTzYD0eVVGRmzkzSXbU0KZ-JScQOyB4-dB5xkAHA@mail.gmail.com","subject":"Re: [PATCH] rev-list: ignore replacements in the connectivity check","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-11-03T18:36:55Z","receivedAt":"2026-11-03T18:36:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Kristofer Karlsson <krka@spotify.com> writes:\n\n> My preliminary thinking is that since replacements are\n> implemented as regular refs (refs/replace/OLD -> NEW),\n> their reachable objects should already be covered by\n> ordinary connectivity checks when those refs are fetched\n> or pushed.\n\nYou are right.  So no need to do anything extra to worry about\n\"doing both\".  It just \"ignore replacements\" had a wrong connotation\nthat we ignore the replacing objects and only pay attention to the\nconnection among underlying objects that misled me.\n\nThanks.\n\n"}]}