{"thread":{"id":"66200","subject":"[PATCH] send-pack: avoid sending the whole tree when pushing from a shallow clone","startedAt":"2026-08-21T06:55:54Z","lastAt":"2026-09-06T07:25:12Z","messageCount":18,"participants":["Elijah Newren via GitGitGadget","Patrick Steinhardt","Elijah Newren","Derrick Stolee"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"550994","messageId":"pull.2208.git.1787295352016.gitgitgadget@gmail.com","threadId":"66200","inReplyTo":null,"subject":"[PATCH] send-pack: avoid sending the whole tree when pushing from a shallow clone","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-21T06:55:51Z","receivedAt":"2026-08-21T06:55:54Z","isPatch":true,"body":"From: Elijah Newren <newren@gmail.com>\n\nWhen pushing from a shallow clone, even if we only have made a small\none-line change to a tiny file, we often push the entire toplevel tree\nof files.  For large repositories, this could be gigabytes instead of\nkilobytes.\n\nThe reason for this is that the push likely lacks the commits the\nreceiver has advertised, so it walks back to its shallow grafts.  Since\nit doesn't know that the server has anything, it sends the entire tree\nfor the graft.  It would also send the parents of the shallow graft,\nexcept the shallow clone doesn't have those by construction.  We thus\nare forced to assume that the server has the parents of the shallow\ngraft -- if it doesn't, the server's receive-pack will reject the push.\n\nBut that raises the obvious question: if we're going to assume the\nserver has the parents of the shallow graft, why not just assume the\nserver has the shallow graft itself -- which this clone almost certainly\nreceived from the server when the shallow clone was created?  As noted\nabove, receive-pack already has a builtin connectivity check that\npredates pushing from a shallow clone by years[*], so even if a client\nis pushing to a different server than it cloned from, the worst that\nhappens is a rejected push.  And by assuming the server has the shallow\ngraft commits, then for large repositories (those most likely to use\nshallow clone) we can avoid transferring (and perhaps re-compressing)\ngigabytes of file contents that the server already has.\n\n[*] Compare 5dbd76760181 (receive/send-pack: support pushing from a\n    shallow clone, 2013-12-05) and 52fed6e1ce07 (receive-pack: check\n    connectivity before concluding \"git push\", 2011-09-02)\n\nFix this by finding the shallow grafts behind the history we're pushing\nand adding them to the pack boundary as uninteresting (negative) tips,\nso the generated pack leaves out everything underneath them.  We only\nuse grafts that the pushed commits can actually reach; excluding every\ngraft in the repository would be simpler, but it could drop an object we\nreally do need to send -- for example, a new blob we're pushing that\nalso happens to sit under some unrelated shallow root pulled from a\ndifferent remote.\n\nWe can also stop early at any commit we and the server both have --\none the server advertised, or that push negotiation found in common.\nSuch a commit already marks the edge of what we need to send, so\nthere's no reason to keep walking down to a graft below it.  For\ndeeper clones the server usually has a commit close by, which keeps\nthis walk short; we only reach a graft when we and the server share no\nhistory that we know about.\n\nOne very rare (and non-default) workflow genuinely needs the larger\npush: seeding a receiver willing to adopt new shallow roots\n(receive.shallowUpdate; see 5dbd76760181 (receive/send-pack: support\npushing from a shallow clone, 2013-12-05) and 0a1bc12b6e40\n(receive-pack: allow pushes that update .git/shallow, 2013-12-05)).\nWhen the server sets receive.shallowUpdate, it is willing to accept\npushes despite lacking ancestors of the pushed commits.  But it expects\nus to send all tree objects so it can graft a new shallow root.  For\nthat case, add a sender-side config, push.shallowExcludeBoundary,\ndefaulting to true (the optimization), while allowing users to set it to\nfalse to restore the previous behavior needed for that rare case.\n\nUpdate the existing shallow-seeding tests in t5538 to set\npush.shallowExcludeBoundary=false, since they exercise that\nreceive.shallowUpdate path.  Add tests for the optimized default and the\nopt-out, that a rejected ref does not cause an accepted ref to be\nover-excluded, and that a shallowUpdate receiver still rejects a\nrootless snapshot by default.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n    send-pack: avoid sending the whole tree when pushing from a shallow\n    clone\n    \n    Maintainer note: The base for this series is\n    ps/odb-pluggable-pack-generation; that series' removal of feed_object()\n    conflicted with my original version of this patch, so I rebased on that\n    series and fixed up the conflict.\n    \n    Users can work around the problem described in this patch with\n    push.negotiate=true, but while we can educate some users to set that,\n    trying to get them all to do so is quite unlikely. Let's help users by\n    providing sane default behavior.\n    \n    One alternative I considered here is making the new\n    push.shallowExcludeBoundary config a tri-state: true, false, or abort,\n    and default to abort. If abort, then when shallow grafts are reached by\n    send-pack, simply abort the push on the client side and tell the user to\n    set push.shallowExcludeBoundary to either true or false. That'd be the\n    more traditional backward compatibility approach of introducing an error\n    period before changing the default. But since the \"traditional\" case\n    seems extraordinarily rare to me and already requires additional special\n    configuration (receive.shallowUpdate=true on any relevant server), I\n    thought the transition period wasn't warranted in this case. Let me know\n    if you disagree.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2208%2Fnewren%2Favoid-expensive-shallow-pushes-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2208/newren/avoid-expensive-shallow-pushes-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/2208\n\n Documentation/config/push.adoc |  12 +++\n send-pack.c                    |  95 ++++++++++++++++++++\n t/t5538-push-shallow.sh        | 156 ++++++++++++++++++++++++++++++++-\n 3 files changed, 260 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config/push.adoc b/Documentation/config/push.adoc\nindex 28132eedfe..9fd6a956a8 100644\n--- a/Documentation/config/push.adoc\n+++ b/Documentation/config/push.adoc\n@@ -134,6 +134,18 @@ This will result in only b (a and c are cleared).\n \trely solely on the server's ref advertisement to find commits\n \tin common.\n \n+`push.shallowExcludeBoundary`::\n+\tWhen pushing from a shallow repository (see linkgit:git-clone[1]\n+\t`--depth`), Git normally assumes that the receiving end already\n+\thas the pushing repository's shallow grafts, and omits those\n+\tobjects from the generated pack rather than resending the full\n+\ttoplevel tree of those grafts. This is safe because the\n+\treceiving end rejects a push that references objects it does not\n+\thave. Set this to `false` to send those objects anyway; this is\n+\tonly needed for the highly unusual case of using a push to seed\n+\ta receiver that adopts new shallow roots (i.e. a receiver that\n+\thas explicitly set `receive.shallowUpdate`). Default is `true`.\n+\n `push.useBitmaps`::\n \tIf set to `false`, disable use of bitmaps for `git push` even if\n \t`pack.useBitmaps` is `true`, without preventing other git operations\ndiff --git a/send-pack.c b/send-pack.c\nindex f20460fbf4..9a035d7403 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -14,6 +14,7 @@\n #include \"transport.h\"\n #include \"version.h\"\n #include \"oid-array.h\"\n+#include \"oidset.h\"\n #include \"gpg-interface.h\"\n #include \"shallow.h\"\n #include \"parse-options.h\"\n@@ -55,6 +56,86 @@ static void append_negative_object(struct repository *r,\n \toid_array_append(haves, oid);\n }\n \n+static int check_to_send_update(const struct ref *ref, const struct send_pack_args *args);\n+\n+/*\n+ * Add the shallow grafts (nr_parent == -1), which are reachable from the\n+ * refs being pushed, to the pack boundary (\"haves\") as uninteresting\n+ * (negative) tips so the generated pack leaves out everything beneath them.\n+ *\n+ * Walk only from the pushed tips, and only until a graft: using a graft\n+ * that does not bound the pushed history could exclude an object we are\n+ * genuinely sending (if it is also reachable from that unrelated graft).\n+ * Stop early at any commit the peer already has, since it is a negative\n+ * the peer can use and the graft beneath it would be redundant.\n+ */\n+static void append_reachable_shallow_grafts(struct repository *r,\n+\t\t\t\t\t    struct ref *refs,\n+\t\t\t\t\t    struct oid_array *advertised,\n+\t\t\t\t\t    struct oid_array *negotiated,\n+\t\t\t\t\t    struct send_pack_args *args,\n+\t\t\t\t\t    struct oid_array *haves)\n+{\n+\tstruct commit_list *pending = NULL;\n+\tstruct oidset seen = OIDSET_INIT;\n+\tstruct oidset known = OIDSET_INIT;\n+\tstruct ref *ref;\n+\tsize_t i;\n+\n+\tfor (i = 0; i < advertised->nr; i++)\n+\t\toidset_insert(&known, &advertised->oid[i]);\n+\tfor (i = 0; i < negotiated->nr; i++)\n+\t\toidset_insert(&known, &negotiated->oid[i]);\n+\tfor (ref = refs; ref; ref = ref->next)\n+\t\tif (!is_null_oid(&ref->old_oid))\n+\t\t\toidset_insert(&known, &ref->old_oid);\n+\n+\tfor (ref = refs; ref; ref = ref->next) {\n+\t\tstruct commit *commit;\n+\n+\t\tif (is_null_oid(&ref->new_oid))\n+\t\t\tcontinue;\n+\t\tif (check_to_send_update(ref, args))\n+\t\t\tcontinue;\n+\t\tcommit = lookup_commit_reference_gently(r, &ref->new_oid, 1);\n+\t\tif (commit)\n+\t\t\tcommit_list_insert(commit, &pending);\n+\t}\n+\n+\twhile (pending) {\n+\t\tstruct commit *commit = pop_commit(&pending);\n+\t\tconst struct object_id *oid = &commit->object.oid;\n+\t\tstruct commit_graft *graft;\n+\t\tstruct commit_list *parent;\n+\n+\t\tif (oidset_insert(&seen, oid))\n+\t\t\tcontinue;\n+\n+\t\t/*\n+\t\t * A commit the peer already has bounds the pushed history\n+\t\t * with a negative it can use, so stop here rather than\n+\t\t * descend to a graft that would only be redundant.\n+\t\t */\n+\t\tif (oidset_contains(&known, oid) &&\n+\t\t    odb_has_object(r->objects, oid, 0))\n+\t\t\tcontinue;\n+\n+\t\tgraft = lookup_commit_graft(r, oid);\n+\t\tif (graft && graft->nr_parent == -1) {\n+\t\t\tappend_negative_object(r, haves, oid);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (repo_parse_commit(r, commit))\n+\t\t\tcontinue;\n+\t\tfor (parent = commit->parents; parent; parent = parent->next)\n+\t\t\tcommit_list_insert(parent->item, &pending);\n+\t}\n+\n+\toidset_clear(&seen);\n+\toidset_clear(&known);\n+}\n+\n /*\n  * Make a pack stream and spit it out into file descriptor fd\n  */\n@@ -88,6 +169,20 @@ static int pack_objects(struct repository *r,\n \tfor (size_t i = 0; i < negotiated->nr; i++)\n \t\tappend_negative_object(r, &opts.haves, &negotiated->oid[i]);\n \n+\t/*\n+\t * When pushing from a shallow repository, avoid re-pushing the\n+\t * entire toplevel tree.\n+\t */\n+\tif (is_repository_shallow(r)) {\n+\t\tint exclude_boundary = 1;\n+\t\trepo_config_get_bool(r, \"push.shallowexcludeboundary\",\n+\t\t\t\t     &exclude_boundary);\n+\t\tif (exclude_boundary)\n+\t\t\tappend_reachable_shallow_grafts(r, refs, advertised,\n+\t\t\t\t\t\t\tnegotiated, args,\n+\t\t\t\t\t\t\t&opts.haves);\n+\t}\n+\n \twhile (refs) {\n \t\tif (!is_null_oid(&refs->old_oid))\n \t\t\tappend_negative_object(r, &opts.haves, &refs->old_oid);\ndiff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh\nindex afab456b32..6b0425bdbc 100755\n--- a/t/t5538-push-shallow.sh\n+++ b/t/t5538-push-shallow.sh\n@@ -64,7 +64,8 @@ EOF\n test_expect_success 'push from shallow clone, with grafted roots' '\n \t(\n \tcd shallow2 &&\n-\ttest_must_fail git push ../.git +main:refs/remotes/shallow2/main 2>err &&\n+\ttest_must_fail git -c push.shallowExcludeBoundary=false \\\n+\t\tpush ../.git +main:refs/remotes/shallow2/main 2>err &&\n \ttest_grep \"shallow2/main.*shallow update not allowed\" err\n \t) &&\n \ttest_must_fail git rev-parse shallow2/main &&\n@@ -75,7 +76,8 @@ test_expect_success 'add new shallow root with receive.updateshallow on' '\n \ttest_config receive.shallowupdate true &&\n \t(\n \tcd shallow2 &&\n-\tgit push ../.git +main:refs/remotes/shallow2/main\n+\tgit -c push.shallowExcludeBoundary=false \\\n+\t\tpush ../.git +main:refs/remotes/shallow2/main\n \t) &&\n \tgit log --format=%s shallow2/main >actual &&\n \tgit fsck &&\n@@ -90,7 +92,8 @@ test_expect_success 'push from shallow to shallow' '\n \t(\n \tcd shallow &&\n \tgit --git-dir=../shallow2/.git config receive.shallowupdate true &&\n-\tgit push ../shallow2/.git +main:refs/remotes/shallow/main &&\n+\tgit -c push.shallowExcludeBoundary=false \\\n+\t\tpush ../shallow2/.git +main:refs/remotes/shallow/main &&\n \tgit --git-dir=../shallow2/.git config receive.shallowupdate false\n \t) &&\n \t(\n@@ -164,4 +167,151 @@ test_expect_success 'push new commit from shallow clone has good deltas' '\n \ttest_region pack-objects path-walk config-push.txt\n '\n \n+test_expect_success 'shallow push only pushes what is necessary' '\n+\tgit init adv-origin &&\n+\t# The shallow grafts are intentionally untagged so that no\n+\t# advertised ref points at them.\n+\ttest_commit --no-tag -C adv-origin a &&\n+\ttest_commit --no-tag -C adv-origin b &&\n+\n+\tgit clone --depth=1 \"file://$(pwd)/adv-origin\" adv-client &&\n+\n+\t# The remote branch advances past the history we have, so its\n+\t# advertised tip is something we cannot use as a negative tip;\n+\t# only the shallow graft lets us exclude the full tree.\n+\ttest_commit --no-tag -C adv-origin c &&\n+\n+\tgit -C adv-client checkout -b topic &&\n+\ttest_commit --no-tag -C adv-client new &&\n+\tGIT_PROGRESS_DELAY=0 git -C adv-client push --progress origin topic 2>err &&\n+\n+\t# Only the new commit, its tree, and the new blob are sent; sending\n+\t# the full tree is avoided by excluding the shallow graft.\n+\ttest_grep \"Enumerating objects: 4, done.\" err\n+'\n+\n+test_expect_success 'push.shallowExcludeBoundary=false sends full tree' '\n+\tgit init adv-origin2 &&\n+\ttest_commit --no-tag -C adv-origin2 a &&\n+\ttest_commit --no-tag -C adv-origin2 b &&\n+\n+\tgit clone --depth=1 \"file://$(pwd)/adv-origin2\" adv-client2 &&\n+\ttest_commit --no-tag -C adv-origin2 c &&\n+\n+\tgit -C adv-client2 checkout -b topic &&\n+\ttest_commit --no-tag -C adv-client2 new &&\n+\tGIT_PROGRESS_DELAY=0 git -C adv-client2 \\\n+\t\t-c push.shallowExcludeBoundary=false \\\n+\t\tpush --progress origin topic 2>err &&\n+\n+\t# With the optimization disabled and no advertised ref pointing at\n+\t# the shallow graft, the full snapshot down to the shallow graft is\n+\t# resent, including its full tree.\n+\ttest_grep \"Enumerating objects: 7, done.\" err\n+'\n+\n+# A rejected ref must not over-exclude objects that another, accepted ref\n+# legitimately needs in the pack.  Set up a testcase using two independent\n+# shallow roots.\n+#\n+#   origin: two unrelated histories; only branch A carries blob O (sh=shared)\n+#       A:  A0---A1     (A0, A1 trees contain sh=O)\n+#       B:  B0---B1     (no \"shared\" blob)\n+#\n+#   receiver: seeded from branch B only, under both ref names; lacks blob O\n+#       refs/heads/B -> B1\n+#       refs/heads/A -> B1     (makes our A push a non-fast-forward)\n+#\n+#   client: \"clone --depth=1 --no-single-branch\" gives a graft at each tip\n+#           and a copy of blob O under A1   (x = cut parents = shallow graft)\n+#           x        x\n+#           |        |\n+#          A1       B1\n+#           |        |\n+#          cX     topic=cY     (cY re-adds sh=O, which the receiver lacks)\n+#\n+#   push \"A topic\" (non-atomic):\n+#     A     -> a non-fast-forward vs receiver A=B1, so its ref update is\n+#              rejected locally and never applied.  It still takes part in\n+#              the shared pack computation, and the buggy code also walked\n+#              back from it to graft A1 (which owns O).\n+#     topic -> accepted; cY grafts onto B1 and needs blob O.\n+#\n+#   Using the shallow graft A1 (an ancestor of A) to trim the pack, even\n+#   though our push of A is rejected locally, would omit blob O from topic's\n+#   pack -- yet topic needs O.  We want to ensure that when topic is pushed,\n+#   O is sent along with it despite A being rejected.\n+test_expect_success 'shallow push does not over-exclude for an accepted ref via a rejected one' '\n+\t# origin\n+\tgit init tworoot-origin &&\n+\tgit -C tworoot-origin checkout -b A &&\n+\ttest_commit -C tworoot-origin --no-tag has-shared sh shared &&\n+\ttest_commit -C tworoot-origin --no-tag A1 &&\n+\tgit -C tworoot-origin switch --orphan B &&\n+\ttest_commit -C tworoot-origin --no-tag B0 &&\n+\ttest_commit -C tworoot-origin --no-tag B1 &&\n+\n+\t# receiver: branch B only, exposed as both B and A\n+\tgit init --bare tworoot-receiver.git &&\n+\tgit -C tworoot-origin push \"file://$(pwd)/tworoot-receiver.git\" \\\n+\t\tB:refs/heads/B B:refs/heads/A &&\n+\n+\t# client: a shallow graft at each branch tip\n+\tgit clone --depth=1 --no-single-branch \\\n+\t\t\"file://$(pwd)/tworoot-origin\" tworoot-client &&\n+\n+\t# branch A gets commit cX; including A in the push gives us a\n+\t# locally-rejected ref whose graft A1 the buggy code walked to.  The A\n+\t# ref update is a non-fast-forward, so it is rejected and never applied.\n+\tgit -C tworoot-client checkout A &&\n+\ttest_commit -C tworoot-client --no-tag cX &&\n+\n+\t# branch topic is what we actually send, reintroducing blob O on B1\n+\tgit -C tworoot-client checkout -b topic B &&\n+\ttest_commit -C tworoot-client --no-tag reintroduce sh shared &&\n+\n+\t# push both in one command: they share a single pack computation, so a\n+\t# graft reached from the rejected A can strip objects that topic needs.\n+\t# The A ref update is rejected locally (non-fast-forward); the shared\n+\t# pack must still contain blob O for topic to land on the receiver.\n+\ttest_must_fail git -C tworoot-client push \\\n+\t\t\"file://$(pwd)/tworoot-receiver.git\" A topic &&\n+\tgit --git-dir=tworoot-receiver.git rev-parse --verify topic\n+'\n+\n+# push.shallowExcludeBoundary (default true) omits the shallow boundary\n+# snapshot from the pack, since an ordinary receiver already has it.  The\n+# exception is a receiver willing to adopt a *new* shallow root\n+# (receive.shallowUpdate): it genuinely needs that snapshot, so the default\n+# optimization leaves it unable to graft the new root.  Verify the receiver\n+# rejects such a push (rather than corrupting itself), and that setting the\n+# config to false restores the full snapshot and lets the push succeed.  This\n+# is the tradeoff that motivates the config knob.\n+test_expect_success 'default push to a shallowUpdate receiver rejects a rootless snapshot' '\n+\tgit init seed-origin &&\n+\ttest_commit -C seed-origin s1 &&\n+\ttest_commit -C seed-origin s2 &&\n+\ttest_commit -C seed-origin s3 &&\n+\n+\t# depth-2: a shallow graft at s2, pushing s3 on top of it\n+\tgit clone --depth=2 \"file://$(pwd)/seed-origin\" seed-client &&\n+\n+\tgit init --bare seed-receiver.git &&\n+\tgit --git-dir=seed-receiver.git config receive.shallowUpdate true &&\n+\n+\t# Default (optimization on): the s2 boundary snapshot is withheld, so\n+\t# the receiver cannot graft the new root and rejects the push, leaving\n+\t# the ref uncreated.\n+\ttest_must_fail git -C seed-client push \\\n+\t\t\"file://$(pwd)/seed-receiver.git\" HEAD:refs/heads/seeded 2>err &&\n+\ttest_grep \"remote rejected\" err &&\n+\ttest_must_fail git --git-dir=seed-receiver.git rev-parse --verify seeded &&\n+\n+\t# Opt-out: the full snapshot is sent, so the same push now succeeds and\n+\t# the new shallow root is grafted.\n+\tgit -C seed-client -c push.shallowExcludeBoundary=false push \\\n+\t\t\"file://$(pwd)/seed-receiver.git\" HEAD:refs/heads/seeded &&\n+\tgit --git-dir=seed-receiver.git rev-parse --verify seeded\n+'\n+\n test_done\n\nbase-commit: 96650039a0dff984f3575568aea85af68474f5c3\n-- \ngitgitgadget\n"},{"id":"551016","messageId":"aohP7GMx9oX3ZCsQ@pks.im","threadId":"66200","inReplyTo":"pull.2208.git.1787295352016.gitgitgadget@gmail.com","subject":"Re: [PATCH] send-pack: avoid sending the whole tree when pushing from a shallow clone","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-21T13:17:32Z","receivedAt":"2026-08-21T13:17:47Z","isPatch":true,"body":"On Fri, Aug 21, 2026 at 06:55:51AM +0000, Elijah Newren via GitGitGadget wrote:\n> From: Elijah Newren <newren@gmail.com>\n> \n> When pushing from a shallow clone, even if we only have made a small\n> one-line change to a tiny file, we often push the entire toplevel tree\n> of files.  For large repositories, this could be gigabytes instead of\n> kilobytes.\n\nOh yeah, that issue. It's a common foot gun indeed, and the common\nadvice here is to never clone with \"--depth=1\", but always with\n\"--depth=2\" so that there is at least one non-grafted commit available\non the client so that they can indeed perform proper negotiation with a\nserver. But over the years I had to explain this again and again, so it\nis clear that this common knowledge might only be commonly known to\npeople who have spent way too much time in the Git codebase.\n\n> The reason for this is that the push likely lacks the commits the\n> receiver has advertised, so it walks back to its shallow grafts.  Since\n> it doesn't know that the server has anything, it sends the entire tree\n> for the graft.  It would also send the parents of the shallow graft,\n> except the shallow clone doesn't have those by construction.  We thus\n> are forced to assume that the server has the parents of the shallow\n> graft -- if it doesn't, the server's receive-pack will reject the push.\n> \n> But that raises the obvious question: if we're going to assume the\n> server has the parents of the shallow graft, why not just assume the\n> server has the shallow graft itself -- which this clone almost certainly\n> received from the server when the shallow clone was created?\n\nIt's a good question to ask. In theory though, can't it happen that the\nclient changes the commit in question locally, e.g. via `git commit\n--amend`, and then pushes? If we now assume that the local commit exists\non the remote side then we'd be insufficient information to the server.\n\n> As noted\n> above, receive-pack already has a builtin connectivity check that\n> predates pushing from a shallow clone by years[*], so even if a client\n> is pushing to a different server than it cloned from, the worst that\n> happens is a rejected push.  And by assuming the server has the shallow\n> graft commits, then for large repositories (those most likely to use\n> shallow clone) we can avoid transferring (and perhaps re-compressing)\n> gigabytes of file contents that the server already has.\n\nRight, the server would catch that case and abort the push. But it\nhighlights the need for an escape hatch, and it makes me wonder what the\ncurrent behaviour is when the grafted commit got modified. I guess\nnothing good comes out of it.\n\nThere's another question though: can we properly determine whether the\ntree of the grafted commit matches a tree that the remote side has, for\nexample example by including the tree in the reference negotiation? I\nhave no idea whether that would break git-recieve-pack(1) or any other\nclients out there, as I don't think we ever negotiated down to trees\nuntil now. But in theory, there isn't really much of a reason why we\ncannot do so.\n\n[snip]\n> Update the existing shallow-seeding tests in t5538 to set\n> push.shallowExcludeBoundary=false, since they exercise that\n> receive.shallowUpdate path.  Add tests for the optimized default and the\n> opt-out, that a rejected ref does not cause an accepted ref to be\n> over-excluded, and that a shallowUpdate receiver still rejects a\n> rootless snapshot by default.\n\nDo we have tests that modify the grafted commit? It would be good to\nlearn how such pushes behave right now, and how the proposed change\nmodifies it.\n\n[snip]\n>     Users can work around the problem described in this patch with\n>     push.negotiate=true, but while we can educate some users to set that,\n>     trying to get them all to do so is quite unlikely. Let's help users by\n>     providing sane default behavior.\n\nMakes me wonder whether the default is something that we should adjust\nso that this defaults to enabled. Are there any downsides to doing so?\n\n> diff --git a/send-pack.c b/send-pack.c\n> index f20460fbf4..9a035d7403 100644\n> --- a/send-pack.c\n> +++ b/send-pack.c\n> @@ -55,6 +56,86 @@ static void append_negative_object(struct repository *r,\n>  \toid_array_append(haves, oid);\n>  }\n>  \n> +static int check_to_send_update(const struct ref *ref, const struct send_pack_args *args);\n> +\n> +/*\n> + * Add the shallow grafts (nr_parent == -1), which are reachable from the\n> + * refs being pushed, to the pack boundary (\"haves\") as uninteresting\n> + * (negative) tips so the generated pack leaves out everything beneath them.\n> + *\n> + * Walk only from the pushed tips, and only until a graft: using a graft\n> + * that does not bound the pushed history could exclude an object we are\n> + * genuinely sending (if it is also reachable from that unrelated graft).\n> + * Stop early at any commit the peer already has, since it is a negative\n> + * the peer can use and the graft beneath it would be redundant.\n> + */\n> +static void append_reachable_shallow_grafts(struct repository *r,\n> +\t\t\t\t\t    struct ref *refs,\n> +\t\t\t\t\t    struct oid_array *advertised,\n> +\t\t\t\t\t    struct oid_array *negotiated,\n> +\t\t\t\t\t    struct send_pack_args *args,\n> +\t\t\t\t\t    struct oid_array *haves)\n\nNit: it might make sense to mark those parameters as `const` that are\nonly used as input.\n\n> +{\n> +\tstruct commit_list *pending = NULL;\n> +\tstruct oidset seen = OIDSET_INIT;\n> +\tstruct oidset known = OIDSET_INIT;\n> +\tstruct ref *ref;\n> +\tsize_t i;\n> +\n> +\tfor (i = 0; i < advertised->nr; i++)\n> +\t\toidset_insert(&known, &advertised->oid[i]);\n> +\tfor (i = 0; i < negotiated->nr; i++)\n> +\t\toidset_insert(&known, &negotiated->oid[i]);\n> +\tfor (ref = refs; ref; ref = ref->next)\n> +\t\tif (!is_null_oid(&ref->old_oid))\n> +\t\t\toidset_insert(&known, &ref->old_oid);\n\nOkay, here we assemble the list of all objects that the remote is\nsupposed to know about.\n\n> +\tfor (ref = refs; ref; ref = ref->next) {\n> +\t\tstruct commit *commit;\n> +\n> +\t\tif (is_null_oid(&ref->new_oid))\n> +\t\t\tcontinue;\n> +\t\tif (check_to_send_update(ref, args))\n> +\t\t\tcontinue;\n> +\t\tcommit = lookup_commit_reference_gently(r, &ref->new_oid, 1);\n> +\t\tif (commit)\n> +\t\t\tcommit_list_insert(commit, &pending);\n> +\t}\n\nHm. Why do we loop through the refs twice? Wouldn't it be possible to\ncombine both loops?\n\n> +\twhile (pending) {\n> +\t\tstruct commit *commit = pop_commit(&pending);\n> +\t\tconst struct object_id *oid = &commit->object.oid;\n> +\t\tstruct commit_graft *graft;\n> +\t\tstruct commit_list *parent;\n> +\n> +\t\tif (oidset_insert(&seen, oid))\n> +\t\t\tcontinue;\n> +\n> +\t\t/*\n> +\t\t * A commit the peer already has bounds the pushed history\n> +\t\t * with a negative it can use, so stop here rather than\n> +\t\t * descend to a graft that would only be redundant.\n> +\t\t */\n> +\t\tif (oidset_contains(&known, oid) &&\n> +\t\t    odb_has_object(r->objects, oid, 0))\n> +\t\t\tcontinue;\n\nWe abort the walk whenever we hit any of the objects in our walk that\nthe remote supposedly already knows about.\n\n> +\t\tgraft = lookup_commit_graft(r, oid);\n> +\t\tif (graft && graft->nr_parent == -1) {\n> +\t\t\tappend_negative_object(r, haves, oid);\n> +\t\t\tcontinue;\n> +\t\t}\n\nAnd when hitting a graft we explicitly add that graf to the negative\nobjects, too, so that we include the graft itself and its tree.\nLogic-wise this make sense, pending the above questions around whether a\ngraft can be modified locally.\n\n> +\t\tif (repo_parse_commit(r, commit))\n> +\t\t\tcontinue;\n> +\t\tfor (parent = commit->parents; parent; parent = parent->next)\n> +\t\t\tcommit_list_insert(parent->item, &pending);\n> +\t}\n> +\n> +\toidset_clear(&seen);\n> +\toidset_clear(&known);\n> +}\n\nInstead of doing a manual walk like this, shouldn't we use higher-level\ninterfaces like `repo_is_descendant_of()` that can make use of commit\ngraphs? That might be overkill though as we can assume that in most\nshallow repositories we won't have deep commit history anyway.\n\nI guess the answer is \"no\" though, as you don't only want to check\nreachability, but also whether any commit in between is part of the\ncommits that either we or the server has advertised.\n\nThanks!\n\nPatrick\n"},{"id":"551035","messageId":"CABPp-BHJj-b=ieva3-=zaCAyvn5UtNQqNT0Q76YCpqZAjO-8VQ@mail.gmail.com","threadId":"66200","inReplyTo":"aohP7GMx9oX3ZCsQ@pks.im","subject":"Re: [PATCH] send-pack: avoid sending the whole tree when pushing from a shallow clone","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-08-21T17:36:04Z","receivedAt":"2026-08-21T17:36:17Z","isPatch":true,"body":"On Fri, Aug 21, 2026 at 6:17 AM Patrick Steinhardt <ps@pks.im> wrote:\n>\n> On Fri, Aug 21, 2026 at 06:55:51AM +0000, Elijah Newren via GitGitGadget wrote:\n> > From: Elijah Newren <newren@gmail.com>\n> >\n> > When pushing from a shallow clone, even if we only have made a small\n> > one-line change to a tiny file, we often push the entire toplevel tree\n> > of files.  For large repositories, this could be gigabytes instead of\n> > kilobytes.\n>\n> Oh yeah, that issue. It's a common foot gun indeed, and the common\n> advice here is to never clone with \"--depth=1\", but always with\n> \"--depth=2\" so that there is at least one non-grafted commit available\n> on the client so that they can indeed perform proper negotiation with a\n> server. But over the years I had to explain this again and again, so it\n> is clear that this common knowledge might only be commonly known to\n> people who have spent way too much time in the Git codebase.\n\nI don't think --depth=2 actually helps here.  What enables real\nnegotiation is push.negotiate, not the extra commit, and\npush.negotiate works just as well at --depth=1.\n\nWithout push.negotiate, send-pack's only negatives come from the refs\nthe server advertised filtered by what we actually have.  In the\nfoot-gun scenario -- clone shallow, server advances, then push, using\ndepth of 2 just walks one commit further to the graft and then\nre-sends the whole tree anyway.  Running the four combinations (server\nadvanced after clone, optimization disabled) in a small test repo:\n\n    depth=1, push.negotiate=false:  Enumerating objects: 205\n    depth=2, push.negotiate=false:  Enumerating objects: 208\n    depth=1, push.negotiate=true:   Enumerating objects: 4\n    depth=2, push.negotiate=true:   Enumerating objects: 4\n\n--depth=2 without negotiation is if anything a hair worse, while\nnegotiation fixes it regardless of depth (the negotiator offers the\nshallow graft commit itself as a \"have\", and the server ACKs it).\n\n--depth=2 can in rare cases help, but only in the lucky/accidental\ncase where some advertised ref happens to point at the extra commit\nyou now have.\n\n> It's a good question to ask. In theory though, can't it happen that the\n> client changes the commit in question locally, e.g. via `git commit\n> --amend`, and then pushes? If we now assume that the local commit exists\n> on the remote side then we'd be insufficient information to the server.\n\nOh, wow, I had never thought to amend a shallow graft.  As soon as you\nasked, I assumed it'd create a corrupt repo -- a commit that wasn't\nitself a shallow graft but had parents we didn't know about.  I got\nsurprised in a different way, though: commit --amend treats a shallow\ngraft as a parent-less commit, and thus creates a new root commit.\nThat does avoid corruption, but only by providing a different kind of\nfoot-gun.  (If users really wanted a new root commit, `git\n{switch,checkout} --orphan` is the tool to do that.)\n\nSince we've got another place where commit --amend can serve as a\nfoot-gun that I've long meant to fix up, I'll submit a separate series\nthat'll make it throw errors for both cases.\n\n> There's another question though: can we properly determine whether the\n> tree of the grafted commit matches a tree that the remote side has, for\n> example example by including the tree in the reference negotiation? I\n> have no idea whether that would break git-recieve-pack(1) or any other\n> clients out there, as I don't think we ever negotiated down to trees\n> until now. But in theory, there isn't really much of a reason why we\n> cannot do so.\n\nInteresting idea...but doesn't this happen too late to help?  Without\npush.negotiate=true, I _think_ (double check me) that the flow is:\n\n  * server blindly speaks first, advertising the refs it has\n  * client responds, including its shallow <oid> lines and then sending the pack\n  * server reports status\n\nIf I'm right about that, the server doesn't know about the client's\nshallow grafts until too late, so it'd have to advertise the toplevel\ntree of every commit it has if it wanted the client to be able to take\nadvantage of them.\n\nAlternatively, we could change the protocol, but we already have\npush.negotiate=true that is implemented and is more thorough than\nsharing the common tree (the common commit contains the shared\ntoplevel tree).  The only place I think a tree negotiation could win\nover commit negotiation is when you keep a tree the server already has\nbut under a commit it doesn't -- e.g. you rewrite the grafted commit\nbut leave its tree (or part of it) unchanged. And if you changed the\ntop-level tree, you'd have to recurse and share each unchanged subtree\nto avoid re-sending common history. That's a lot of machinery for a\nnarrow, contrived case, so I'm not sure it leads to a helpful path.\n\n> [snip]\n> > Update the existing shallow-seeding tests in t5538 to set\n> > push.shallowExcludeBoundary=false, since they exercise that\n> > receive.shallowUpdate path.  Add tests for the optimized default and the\n> > opt-out, that a rejected ref does not cause an accepted ref to be\n> > over-excluded, and that a shallowUpdate receiver still rejects a\n> > rootless snapshot by default.\n>\n> Do we have tests that modify the grafted commit? It would be good to\n> learn how such pushes behave right now, and how the proposed change\n> modifies it.\n\nAs noted above, modified commits are actually root commits and do not\nhave a shallow history, and thus aren't really part of shallow push\ntesting.  I think it's a bug that modified commits become root\ncommits, but one that really is tangential to this patch.  I'll submit\na separate series with a fix.\n\n> [snip]\n> >     Users can work around the problem described in this patch with\n> >     push.negotiate=true, but while we can educate some users to set that,\n> >     trying to get them all to do so is quite unlikely. Let's help users by\n> >     providing sane default behavior.\n>\n> Makes me wonder whether the default is something that we should adjust\n> so that this defaults to enabled. Are there any downsides to doing so?\n\nThe only one I can think of is that it adds a round-trip to every\npush, which increases latency in order to sometimes reduce bandwidth\nand cpu.\n\nIt can dramatically reduce bandwidth and cpu, but not always (single\nperson projects would probably never see a benefit, for example, nor\nwould anyone interacting with a fetch v0 server), and it always\nincreases latency.\n\n> > +static void append_reachable_shallow_grafts(struct repository *r,\n> > +                                         struct ref *refs,\n> > +                                         struct oid_array *advertised,\n> > +                                         struct oid_array *negotiated,\n> > +                                         struct send_pack_args *args,\n> > +                                         struct oid_array *haves)\n>\n> Nit: it might make sense to mark those parameters as `const` that are\n> only used as input.\n\nGood point; will fix.\n\n> > +     for (ref = refs; ref; ref = ref->next)\n> > +             if (!is_null_oid(&ref->old_oid))\n> > +                     oidset_insert(&known, &ref->old_oid);\n>\n> Okay, here we assemble the list of all objects that the remote is\n> supposed to know about.\n>\n> > +     for (ref = refs; ref; ref = ref->next) {\n> > +             struct commit *commit;\n> > +\n> > +             if (is_null_oid(&ref->new_oid))\n> > +                     continue;\n> > +             if (check_to_send_update(ref, args))\n> > +                     continue;\n> > +             commit = lookup_commit_reference_gently(r, &ref->new_oid, 1);\n> > +             if (commit)\n> > +                     commit_list_insert(commit, &pending);\n> > +     }\n>\n> Hm. Why do we loop through the refs twice? Wouldn't it be possible to\n> combine both loops?\n\nOops, good catch.  Will fix.\n\n> Instead of doing a manual walk like this, shouldn't we use higher-level\n> interfaces like `repo_is_descendant_of()` that can make use of commit\n> graphs? That might be overkill though as we can assume that in most\n> shallow repositories we won't have deep commit history anyway.\n>\n> I guess the answer is \"no\" though, as you don't only want to check\n> reachability, but also whether any commit in between is part of the\n> commits that either we or the server has advertised.\n\nRight, that's the reason: I need to stop at commits the peer already\nhas and pick out graft boundaries along the way, which a descendant\ncheck doesn't give me.  Shallow histories tend to be short, so the\nexplicit walk is likely cheap.\n"},{"id":"551036","messageId":"CABPp-BHWz_cugSO0EezqjHhDGok-xGuVQqWLqf=jMUvVM-Vpyw@mail.gmail.com","threadId":"66200","inReplyTo":"CABPp-BHJj-b=ieva3-=zaCAyvn5UtNQqNT0Q76YCpqZAjO-8VQ@mail.gmail.com","subject":"Re: [PATCH] send-pack: avoid sending the whole tree when pushing from a shallow clone","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-08-21T18:21:50Z","receivedAt":"2026-08-21T18:22:03Z","isPatch":true,"body":"On Fri, Aug 21, 2026 at 10:36 AM Elijah Newren <newren@gmail.com> wrote:\n>\n> On Fri, Aug 21, 2026 at 6:17 AM Patrick Steinhardt <ps@pks.im> wrote:\n> >\n> > On Fri, Aug 21, 2026 at 06:55:51AM +0000, Elijah Newren via GitGitGadget wrote:\n> > > From: Elijah Newren <newren@gmail.com>\n> > >\n> > > When pushing from a shallow clone, even if we only have made a small\n> > > one-line change to a tiny file, we often push the entire toplevel tree\n> > > of files.  For large repositories, this could be gigabytes instead of\n> > > kilobytes.\n> >\n> > Oh yeah, that issue. It's a common foot gun indeed, and the common\n> > advice here is to never clone with \"--depth=1\", but always with\n> > \"--depth=2\" so that there is at least one non-grafted commit available\n> > on the client so that they can indeed perform proper negotiation with a\n> > server. But over the years I had to explain this again and again, so it\n> > is clear that this common knowledge might only be commonly known to\n> > people who have spent way too much time in the Git codebase.\n>\n> I don't think --depth=2 actually helps here.  What enables real\n> negotiation is push.negotiate, not the extra commit, and\n> push.negotiate works just as well at --depth=1.\n>\n> Without push.negotiate, send-pack's only negatives come from the refs\n> the server advertised filtered by what we actually have.  In the\n> foot-gun scenario -- clone shallow, server advances, then push, using\n> depth of 2 just walks one commit further to the graft and then\n> re-sends the whole tree anyway.  Running the four combinations (server\n> advanced after clone, optimization disabled) in a small test repo:\n>\n>     depth=1, push.negotiate=false:  Enumerating objects: 205\n>     depth=2, push.negotiate=false:  Enumerating objects: 208\n>     depth=1, push.negotiate=true:   Enumerating objects: 4\n>     depth=2, push.negotiate=true:   Enumerating objects: 4\n>\n> --depth=2 without negotiation is if anything a hair worse, while\n> negotiation fixes it regardless of depth (the negotiator offers the\n> shallow graft commit itself as a \"have\", and the server ACKs it).\n>\n> --depth=2 can in rare cases help, but only in the lucky/accidental\n> case where some advertised ref happens to point at the extra commit\n> you now have.\n\nI guess I should add that --depth=2 is not really \"luck\" for some\nusers, but may be guaranteed by their workflow:\n  - customers of forges\n  - assuming those forges (make refs for merge/pull requests AND\nadvertise those refs from receive-pack) OR (keep a branch pointing at\nthe tip of the {pull,merge} request)\n  - assuming those users never push directly to their main branch\n(instead only updating it via merge requests or pull requests)\n  - assuming those users don't use squash merges or rebases on their\nmerge request/pull requests, but do actual merges\n\nIf all the conditions above are met, forges should have a ref pointing\nto the tip of the now-merged {merge,pull} request, which will never\nchange since it was merged, and thus a --depth=2 clone will pick up\nsuch a commit and have some common history it discovers.\n\nGitHub includes refs/pull/ in receive.hiderefs, and users often delete\nbranches upon merge, so neither half of that second condition holds\nfor us and this wouldn't help our customers.  Further, even if we did\nchange the ref advertisement, we have a number of big repositories who\ndon't satisfy the other conditions (e.g. some customers make heavy use\nof squash merges), so it still wouldn't help them.\n"},{"id":"551106","messageId":"aovW5bxu1F8jYKYl@pks.im","threadId":"66200","inReplyTo":"CABPp-BHJj-b=ieva3-=zaCAyvn5UtNQqNT0Q76YCpqZAjO-8VQ@mail.gmail.com","subject":"Re: [PATCH] send-pack: avoid sending the whole tree when pushing from a shallow clone","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2026-08-24T05:30:13Z","receivedAt":"2026-08-24T05:30:21Z","isPatch":true,"body":"On Fri, Aug 21, 2026 at 10:36:04AM -0700, Elijah Newren wrote:\n> On Fri, Aug 21, 2026 at 6:17 AM Patrick Steinhardt <ps@pks.im> wrote:\n> >\n> > On Fri, Aug 21, 2026 at 06:55:51AM +0000, Elijah Newren via GitGitGadget wrote:\n> > > From: Elijah Newren <newren@gmail.com>\n> > >\n> > > When pushing from a shallow clone, even if we only have made a small\n> > > one-line change to a tiny file, we often push the entire toplevel tree\n> > > of files.  For large repositories, this could be gigabytes instead of\n> > > kilobytes.\n> >\n> > Oh yeah, that issue. It's a common foot gun indeed, and the common\n> > advice here is to never clone with \"--depth=1\", but always with\n> > \"--depth=2\" so that there is at least one non-grafted commit available\n> > on the client so that they can indeed perform proper negotiation with a\n> > server. But over the years I had to explain this again and again, so it\n> > is clear that this common knowledge might only be commonly known to\n> > people who have spent way too much time in the Git codebase.\n> \n> I don't think --depth=2 actually helps here.  What enables real\n> negotiation is push.negotiate, not the extra commit, and\n> push.negotiate works just as well at --depth=1.\n> \n> Without push.negotiate, send-pack's only negatives come from the refs\n> the server advertised filtered by what we actually have.  In the\n> foot-gun scenario -- clone shallow, server advances, then push, using\n> depth of 2 just walks one commit further to the graft and then\n> re-sends the whole tree anyway.  Running the four combinations (server\n> advanced after clone, optimization disabled) in a small test repo:\n> \n>     depth=1, push.negotiate=false:  Enumerating objects: 205\n>     depth=2, push.negotiate=false:  Enumerating objects: 208\n>     depth=1, push.negotiate=true:   Enumerating objects: 4\n>     depth=2, push.negotiate=true:   Enumerating objects: 4\n> \n> --depth=2 without negotiation is if anything a hair worse, while\n> negotiation fixes it regardless of depth (the negotiator offers the\n> shallow graft commit itself as a \"have\", and the server ACKs it).\n> \n> --depth=2 can in rare cases help, but only in the lucky/accidental\n> case where some advertised ref happens to point at the extra commit\n> you now have.\n\nTIL, thanks. I don't think I was even aware of \"push.negotiate\", and I\nmostly went by the folklore of \"just clone with --depth=2\" that I saw\nrepeated on many sites.\n\nBut this and all of your other answers make me lean strongly into the\ndirection that the fix is at the wrong level, and the proper fix really\nis to enable \"push.negotiate\" by default.\n\n> > It's a good question to ask. In theory though, can't it happen that the\n> > client changes the commit in question locally, e.g. via `git commit\n> > --amend`, and then pushes? If we now assume that the local commit exists\n> > on the remote side then we'd be insufficient information to the server.\n> \n> Oh, wow, I had never thought to amend a shallow graft.  As soon as you\n> asked, I assumed it'd create a corrupt repo -- a commit that wasn't\n> itself a shallow graft but had parents we didn't know about.  I got\n> surprised in a different way, though: commit --amend treats a shallow\n> graft as a parent-less commit, and thus creates a new root commit.\n> That does avoid corruption, but only by providing a different kind of\n> foot-gun.  (If users really wanted a new root commit, `git\n> {switch,checkout} --orphan` is the tool to do that.)\n> \n> Since we've got another place where commit --amend can serve as a\n> foot-gun that I've long meant to fix up, I'll submit a separate series\n> that'll make it throw errors for both cases.\n\nThat makes sense.\n\n> > [snip]\n> > >     Users can work around the problem described in this patch with\n> > >     push.negotiate=true, but while we can educate some users to set that,\n> > >     trying to get them all to do so is quite unlikely. Let's help users by\n> > >     providing sane default behavior.\n> >\n> > Makes me wonder whether the default is something that we should adjust\n> > so that this defaults to enabled. Are there any downsides to doing so?\n> \n> The only one I can think of is that it adds a round-trip to every\n> push, which increases latency in order to sometimes reduce bandwidth\n> and cpu.\n> \n> It can dramatically reduce bandwidth and cpu, but not always (single\n> person projects would probably never see a benefit, for example, nor\n> would anyone interacting with a fetch v0 server), and it always\n> increases latency.\n\nThat's all fair, but it does dramatically help in the case of shallow\nclones. And the number of times I've seen this question come up hints\nthat this is a very common scenario.\n\nWe could be clever about it: if \"push.negotiate\" is very likely to help\nin shallow clones but mostly just adds latency in full clones, then why\ndon't we introduce a new \"push.negotiate=shallow\" option that enables\nthis feature automatically for shallow clones and make it the default?\nThat to me sounds like a low-hanging fruit, and I would prefer such a\nfix compared to introducing new logic.\n\nPatrick\n"},{"id":"551165","messageId":"CABPp-BHwa7QM=XDuO=9xqm-OL8dn8uGf1=rv+sgBRQ9hHKMFuQ@mail.gmail.com","threadId":"66200","inReplyTo":"aovW5bxu1F8jYKYl@pks.im","subject":"Re: [PATCH] send-pack: avoid sending the whole tree when pushing from a shallow clone","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-08-25T05:00:18Z","receivedAt":"2026-08-25T05:00:31Z","isPatch":true,"body":"On Sun, Aug 23, 2026 at 10:30 PM Patrick Steinhardt <ps@pks.im> wrote:\n>\n[...]\n> TIL, thanks. I don't think I was even aware of \"push.negotiate\", and I\n> mostly went by the folklore of \"just clone with --depth=2\" that I saw\n> repeated on many sites.\n>\n> But this and all of your other answers make me lean strongly into the\n> direction that the fix is at the wrong level, and the proper fix really\n> is to enable \"push.negotiate\" by default.\n\nI don't think that fixes the problem, though:\n\n  a) Users can do a shallow clone of a specific branch for a specific\npull-request/merge-request.  Then the pull-request/merge-request is\nrebased, and sensitive data removed due to a leaked secret.  The\nshallow graft is no longer common.  Pushing from the shallow clone\nshould fail, but it shouldn't have to send several gigabytes of data\nin order to get the failure message.\n  b) (Very similar to a) Users can do a shallow clone of one repo (a\nlocal repository cache?) and then push to another; the shallow graft\nthus may not be common.  An error is expected, but sending gigabytes\nof data to get the error isn't.\n  c) Users set push.negotiate=false explicitly.  In my opinion, they\nshouldn't get this bug just for opting out of that kind-of-related\nfeature.\n  d) push.negotiate=true silently fails for some setups\n\nI think case (d) is particularly interesting: For push.negotiate to\nwork, fetch v2 must be working.  For http, that's not a big deal.\nWhen using ssh, it requires the client to send GIT_PROTOCOL=version=2\nenvironment variable and for the server to accept it:\n  * Server side:\n    * Some self-hosting forges may not automatically support receiving\nthe environment variable.  My searches suggest BitBucket always uses\nv0 for ssh, and GitLab depends on the installation method -- either\nthe Linux package or self-compile installs requiring manual action\n(only Helm and the all-in-one Docker image are preconfigured)\n    * Some corporate setups may specifically want to disallow sending\nany environment variables over ssh (perhaps through an \"upstream stock\nconfiguration only\" policy?)\n  * Client side:\n    * git only requests v2 when it decides the client is OpenSSH (I\nthink that maps to the command being named ssh/ssh.exe, or an\nauto-probe succeeds)\n    * plink / putty / tortoiseplink appear to not allow sending this\nenvironment variable, so many Windows users may be cut out\n    * Some corporate setups might restrict sending environment\nvariables over ssh on the client side as well\n\nWhen it's not supported, it falls back to v0 with a simple warning,\ndoes no negotiation, and runs into the old bug.\n\nSo, while I support the idea of moving towards push.negotiate=true or\neven adding push.negotiate=shallow, because they would provide other\nbenefits, I don't think they fix the problem at hand and thus believe\nthat this patch is still important.\n\n[...]\n> > Since we've got another place where commit --amend can serve as a\n> > foot-gun that I've long meant to fix up, I'll submit a separate series\n> > that'll make it throw errors for both cases.\n>\n> That makes sense.\n\nTurns out there's a bunch of additional stuff on the shallow side, so\nI think I'm going to split it into two series; a single patch for\nrebase/revert/am, and five or so patches for shallow graft handling\nacross a variety of commands.\n\n> That's all fair, but it does dramatically help in the case of shallow\n> clones. And the number of times I've seen this question come up hints\n> that this is a very common scenario.\n>\n> We could be clever about it: if \"push.negotiate\" is very likely to help\n> in shallow clones but mostly just adds latency in full clones, then why\n> don't we introduce a new \"push.negotiate=shallow\" option that enables\n> this feature automatically for shallow clones and make it the default?\n> That to me sounds like a low-hanging fruit, and I would prefer such a\n> fix compared to introducing new logic.\n\nI kind of like the idea of somehow making push.negotiate default on\nfor big repos in general (not just shallow), but while push.negotiate\nhas lots of other benefits and has the side effect of solving most\ncases of this problem for some users, it falls short of actually fully\nsolving the problem.  This patch, or something like it, is still\nneeded.\n"},{"id":"551230","messageId":"pull.2208.v2.git.1787684776048.gitgitgadget@gmail.com","threadId":"66200","inReplyTo":"pull.2208.git.1787295352016.gitgitgadget@gmail.com","subject":"[PATCH v2] send-pack: avoid sending the whole tree when pushing from a shallow clone","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-08-25T19:06:16Z","receivedAt":"2026-08-25T19:06:18Z","isPatch":true,"body":"From: Elijah Newren <newren@gmail.com>\n\nWhen pushing from a shallow clone, even if we only have made a small\none-line change to a tiny file, we often push the entire toplevel tree\nof files.  For large repositories, this could be gigabytes instead of\nkilobytes.\n\nThe reason for this is that the push likely lacks the commits the\nreceiver has advertised, so it walks back to its shallow grafts.  Since\nit doesn't know that the server has anything, it sends the entire tree\nfor the graft.  It would also send the parents of the shallow graft,\nexcept the shallow clone doesn't have those by construction.  We thus\nare forced to assume that the server has the parents of the shallow\ngraft -- if it doesn't, the server's receive-pack will reject the push.\n\nBut that raises the obvious question: if we're going to assume the\nserver has the parents of the shallow graft, why not just assume the\nserver has the shallow graft itself -- which this clone almost certainly\nreceived from the server when the shallow clone was created?  As noted\nabove, receive-pack already has a builtin connectivity check that\npredates pushing from a shallow clone by years[*], so even if a client\nis pushing to a different server than it cloned from, the worst that\nhappens is a rejected push.  And by assuming the server has the shallow\ngraft commits, then for large repositories (those most likely to use\nshallow clone) we can avoid transferring (and perhaps re-compressing)\ngigabytes of file contents that the server already has.\n\n[*] Compare 5dbd76760181 (receive/send-pack: support pushing from a\n    shallow clone, 2013-12-05) and 52fed6e1ce07 (receive-pack: check\n    connectivity before concluding \"git push\", 2011-09-02)\n\nFix this by finding the shallow grafts behind the history we're pushing\nand adding them to the pack boundary as uninteresting (negative) tips,\nso the generated pack leaves out everything underneath them.  We only\nuse grafts that the pushed commits can actually reach; excluding every\ngraft in the repository would be simpler, but it could drop an object we\nreally do need to send -- for example, a new blob we're pushing that\nalso happens to sit under some unrelated shallow root pulled from a\ndifferent remote.\n\nWe can also stop early at any commit we and the server both have --\none the server advertised, or that push negotiation found in common.\nSuch a commit already marks the edge of what we need to send, so\nthere's no reason to keep walking down to a graft below it.  For\ndeeper clones the server usually has a commit close by, which keeps\nthis walk short; we only reach a graft when we and the server share no\nhistory that we know about.\n\nOne very rare (and non-default) workflow genuinely needs the larger\npush: seeding a receiver willing to adopt new shallow roots\n(receive.shallowUpdate; see 5dbd76760181 (receive/send-pack: support\npushing from a shallow clone, 2013-12-05) and 0a1bc12b6e40\n(receive-pack: allow pushes that update .git/shallow, 2013-12-05)).\nWhen the server sets receive.shallowUpdate, it is willing to accept\npushes despite lacking ancestors of the pushed commits.  But it expects\nus to send all tree objects so it can graft a new shallow root.  For\nthat case, add a sender-side config, push.shallowExcludeBoundary,\ndefaulting to true (the optimization), while allowing users to set it to\nfalse to restore the previous behavior needed for that rare case.\n\nUpdate the existing shallow-seeding tests in t5538 to set\npush.shallowExcludeBoundary=false, since they exercise that\nreceive.shallowUpdate path.  Add tests for the optimized default and the\nopt-out, that a rejected ref does not cause an accepted ref to be\nover-excluded, and that a shallowUpdate receiver still rejects a\nrootless snapshot by default.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n    send-pack: avoid sending the whole tree when pushing from a shallow\n    clone\n    \n    Changes since v1:\n    \n     * Fixed two small code style issues\n     * Updated the cover letter below to point out that push.negotiate=true\n       doesn't work for everyone, and even when negotiation does work it\n       doesn't solve all cases.\n     * Updated to latest ps/odb-pluggable-pack-generation branch.\n    \n    Maintainer note: The base for this series is\n    ps/odb-pluggable-pack-generation; that series' removal of feed_object()\n    conflicted with my original version of this patch, so I rebased on that\n    series and fixed up the conflict.\n    \n    Some users can work around the problem described in this patch with\n    push.negotiate=true. Even if we were to make that the default, though,\n    (a) negotiation doesn't work for some people (depending on other server\n    and client settings and programs), and (b) even for those for whom\n    negotiation does happen, that doesn't solve all cases. Let's help users\n    by providing sane default behavior.\n    \n    One alternative I considered here is making the new\n    push.shallowExcludeBoundary config a tri-state: true, false, or abort,\n    and default to abort. If abort, then when shallow grafts are reached by\n    send-pack, simply abort the push on the client side and tell the user to\n    set push.shallowExcludeBoundary to either true or false. That'd be the\n    more traditional backward compatibility approach of introducing an error\n    period before changing the default. But since the \"traditional\" case\n    seems extraordinarily rare to me and already requires additional special\n    configuration (receive.shallowUpdate=true on any relevant server), I\n    thought the transition period wasn't warranted in this case. Let me know\n    if you disagree.\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2208%2Fnewren%2Favoid-expensive-shallow-pushes-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2208/newren/avoid-expensive-shallow-pushes-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/2208\n\nRange-diff vs v1:\n\n 1:  649efa1c5f ! 1:  d4501a5c23 send-pack: avoid sending the whole tree when pushing from a shallow clone\n     @@ send-pack.c: static void append_negative_object(struct repository *r,\n      + * the peer can use and the graft beneath it would be redundant.\n      + */\n      +static void append_reachable_shallow_grafts(struct repository *r,\n     -+\t\t\t\t\t    struct ref *refs,\n     -+\t\t\t\t\t    struct oid_array *advertised,\n     -+\t\t\t\t\t    struct oid_array *negotiated,\n     -+\t\t\t\t\t    struct send_pack_args *args,\n     ++\t\t\t\t\t    const struct ref *refs,\n     ++\t\t\t\t\t    const struct oid_array *advertised,\n     ++\t\t\t\t\t    const struct oid_array *negotiated,\n     ++\t\t\t\t\t    const struct send_pack_args *args,\n      +\t\t\t\t\t    struct oid_array *haves)\n      +{\n      +\tstruct commit_list *pending = NULL;\n      +\tstruct oidset seen = OIDSET_INIT;\n      +\tstruct oidset known = OIDSET_INIT;\n     -+\tstruct ref *ref;\n     ++\tconst struct ref *ref;\n      +\tsize_t i;\n      +\n      +\tfor (i = 0; i < advertised->nr; i++)\n      +\t\toidset_insert(&known, &advertised->oid[i]);\n      +\tfor (i = 0; i < negotiated->nr; i++)\n      +\t\toidset_insert(&known, &negotiated->oid[i]);\n     -+\tfor (ref = refs; ref; ref = ref->next)\n     -+\t\tif (!is_null_oid(&ref->old_oid))\n     -+\t\t\toidset_insert(&known, &ref->old_oid);\n      +\n     ++\t/*\n     ++\t * Record every commit the peer is known to have as a boundary for\n     ++\t * the walk, and seed the walk from the tips we are actually sending.\n     ++\t * The walk below does not begin until \"known\" is fully populated.\n     ++\t */\n      +\tfor (ref = refs; ref; ref = ref->next) {\n      +\t\tstruct commit *commit;\n      +\n     ++\t\tif (!is_null_oid(&ref->old_oid))\n     ++\t\t\toidset_insert(&known, &ref->old_oid);\n     ++\n      +\t\tif (is_null_oid(&ref->new_oid))\n      +\t\t\tcontinue;\n      +\t\tif (check_to_send_update(ref, args))\n\n\n Documentation/config/push.adoc |  12 +++\n send-pack.c                    | 100 +++++++++++++++++++++\n t/t5538-push-shallow.sh        | 156 ++++++++++++++++++++++++++++++++-\n 3 files changed, 265 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config/push.adoc b/Documentation/config/push.adoc\nindex 28132eedfe..9fd6a956a8 100644\n--- a/Documentation/config/push.adoc\n+++ b/Documentation/config/push.adoc\n@@ -134,6 +134,18 @@ This will result in only b (a and c are cleared).\n \trely solely on the server's ref advertisement to find commits\n \tin common.\n \n+`push.shallowExcludeBoundary`::\n+\tWhen pushing from a shallow repository (see linkgit:git-clone[1]\n+\t`--depth`), Git normally assumes that the receiving end already\n+\thas the pushing repository's shallow grafts, and omits those\n+\tobjects from the generated pack rather than resending the full\n+\ttoplevel tree of those grafts. This is safe because the\n+\treceiving end rejects a push that references objects it does not\n+\thave. Set this to `false` to send those objects anyway; this is\n+\tonly needed for the highly unusual case of using a push to seed\n+\ta receiver that adopts new shallow roots (i.e. a receiver that\n+\thas explicitly set `receive.shallowUpdate`). Default is `true`.\n+\n `push.useBitmaps`::\n \tIf set to `false`, disable use of bitmaps for `git push` even if\n \t`pack.useBitmaps` is `true`, without preventing other git operations\ndiff --git a/send-pack.c b/send-pack.c\nindex f20460fbf4..5e1ac9dd89 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -14,6 +14,7 @@\n #include \"transport.h\"\n #include \"version.h\"\n #include \"oid-array.h\"\n+#include \"oidset.h\"\n #include \"gpg-interface.h\"\n #include \"shallow.h\"\n #include \"parse-options.h\"\n@@ -55,6 +56,91 @@ static void append_negative_object(struct repository *r,\n \toid_array_append(haves, oid);\n }\n \n+static int check_to_send_update(const struct ref *ref, const struct send_pack_args *args);\n+\n+/*\n+ * Add the shallow grafts (nr_parent == -1), which are reachable from the\n+ * refs being pushed, to the pack boundary (\"haves\") as uninteresting\n+ * (negative) tips so the generated pack leaves out everything beneath them.\n+ *\n+ * Walk only from the pushed tips, and only until a graft: using a graft\n+ * that does not bound the pushed history could exclude an object we are\n+ * genuinely sending (if it is also reachable from that unrelated graft).\n+ * Stop early at any commit the peer already has, since it is a negative\n+ * the peer can use and the graft beneath it would be redundant.\n+ */\n+static void append_reachable_shallow_grafts(struct repository *r,\n+\t\t\t\t\t    const struct ref *refs,\n+\t\t\t\t\t    const struct oid_array *advertised,\n+\t\t\t\t\t    const struct oid_array *negotiated,\n+\t\t\t\t\t    const struct send_pack_args *args,\n+\t\t\t\t\t    struct oid_array *haves)\n+{\n+\tstruct commit_list *pending = NULL;\n+\tstruct oidset seen = OIDSET_INIT;\n+\tstruct oidset known = OIDSET_INIT;\n+\tconst struct ref *ref;\n+\tsize_t i;\n+\n+\tfor (i = 0; i < advertised->nr; i++)\n+\t\toidset_insert(&known, &advertised->oid[i]);\n+\tfor (i = 0; i < negotiated->nr; i++)\n+\t\toidset_insert(&known, &negotiated->oid[i]);\n+\n+\t/*\n+\t * Record every commit the peer is known to have as a boundary for\n+\t * the walk, and seed the walk from the tips we are actually sending.\n+\t * The walk below does not begin until \"known\" is fully populated.\n+\t */\n+\tfor (ref = refs; ref; ref = ref->next) {\n+\t\tstruct commit *commit;\n+\n+\t\tif (!is_null_oid(&ref->old_oid))\n+\t\t\toidset_insert(&known, &ref->old_oid);\n+\n+\t\tif (is_null_oid(&ref->new_oid))\n+\t\t\tcontinue;\n+\t\tif (check_to_send_update(ref, args))\n+\t\t\tcontinue;\n+\t\tcommit = lookup_commit_reference_gently(r, &ref->new_oid, 1);\n+\t\tif (commit)\n+\t\t\tcommit_list_insert(commit, &pending);\n+\t}\n+\n+\twhile (pending) {\n+\t\tstruct commit *commit = pop_commit(&pending);\n+\t\tconst struct object_id *oid = &commit->object.oid;\n+\t\tstruct commit_graft *graft;\n+\t\tstruct commit_list *parent;\n+\n+\t\tif (oidset_insert(&seen, oid))\n+\t\t\tcontinue;\n+\n+\t\t/*\n+\t\t * A commit the peer already has bounds the pushed history\n+\t\t * with a negative it can use, so stop here rather than\n+\t\t * descend to a graft that would only be redundant.\n+\t\t */\n+\t\tif (oidset_contains(&known, oid) &&\n+\t\t    odb_has_object(r->objects, oid, 0))\n+\t\t\tcontinue;\n+\n+\t\tgraft = lookup_commit_graft(r, oid);\n+\t\tif (graft && graft->nr_parent == -1) {\n+\t\t\tappend_negative_object(r, haves, oid);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (repo_parse_commit(r, commit))\n+\t\t\tcontinue;\n+\t\tfor (parent = commit->parents; parent; parent = parent->next)\n+\t\t\tcommit_list_insert(parent->item, &pending);\n+\t}\n+\n+\toidset_clear(&seen);\n+\toidset_clear(&known);\n+}\n+\n /*\n  * Make a pack stream and spit it out into file descriptor fd\n  */\n@@ -88,6 +174,20 @@ static int pack_objects(struct repository *r,\n \tfor (size_t i = 0; i < negotiated->nr; i++)\n \t\tappend_negative_object(r, &opts.haves, &negotiated->oid[i]);\n \n+\t/*\n+\t * When pushing from a shallow repository, avoid re-pushing the\n+\t * entire toplevel tree.\n+\t */\n+\tif (is_repository_shallow(r)) {\n+\t\tint exclude_boundary = 1;\n+\t\trepo_config_get_bool(r, \"push.shallowexcludeboundary\",\n+\t\t\t\t     &exclude_boundary);\n+\t\tif (exclude_boundary)\n+\t\t\tappend_reachable_shallow_grafts(r, refs, advertised,\n+\t\t\t\t\t\t\tnegotiated, args,\n+\t\t\t\t\t\t\t&opts.haves);\n+\t}\n+\n \twhile (refs) {\n \t\tif (!is_null_oid(&refs->old_oid))\n \t\t\tappend_negative_object(r, &opts.haves, &refs->old_oid);\ndiff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh\nindex afab456b32..6b0425bdbc 100755\n--- a/t/t5538-push-shallow.sh\n+++ b/t/t5538-push-shallow.sh\n@@ -64,7 +64,8 @@ EOF\n test_expect_success 'push from shallow clone, with grafted roots' '\n \t(\n \tcd shallow2 &&\n-\ttest_must_fail git push ../.git +main:refs/remotes/shallow2/main 2>err &&\n+\ttest_must_fail git -c push.shallowExcludeBoundary=false \\\n+\t\tpush ../.git +main:refs/remotes/shallow2/main 2>err &&\n \ttest_grep \"shallow2/main.*shallow update not allowed\" err\n \t) &&\n \ttest_must_fail git rev-parse shallow2/main &&\n@@ -75,7 +76,8 @@ test_expect_success 'add new shallow root with receive.updateshallow on' '\n \ttest_config receive.shallowupdate true &&\n \t(\n \tcd shallow2 &&\n-\tgit push ../.git +main:refs/remotes/shallow2/main\n+\tgit -c push.shallowExcludeBoundary=false \\\n+\t\tpush ../.git +main:refs/remotes/shallow2/main\n \t) &&\n \tgit log --format=%s shallow2/main >actual &&\n \tgit fsck &&\n@@ -90,7 +92,8 @@ test_expect_success 'push from shallow to shallow' '\n \t(\n \tcd shallow &&\n \tgit --git-dir=../shallow2/.git config receive.shallowupdate true &&\n-\tgit push ../shallow2/.git +main:refs/remotes/shallow/main &&\n+\tgit -c push.shallowExcludeBoundary=false \\\n+\t\tpush ../shallow2/.git +main:refs/remotes/shallow/main &&\n \tgit --git-dir=../shallow2/.git config receive.shallowupdate false\n \t) &&\n \t(\n@@ -164,4 +167,151 @@ test_expect_success 'push new commit from shallow clone has good deltas' '\n \ttest_region pack-objects path-walk config-push.txt\n '\n \n+test_expect_success 'shallow push only pushes what is necessary' '\n+\tgit init adv-origin &&\n+\t# The shallow grafts are intentionally untagged so that no\n+\t# advertised ref points at them.\n+\ttest_commit --no-tag -C adv-origin a &&\n+\ttest_commit --no-tag -C adv-origin b &&\n+\n+\tgit clone --depth=1 \"file://$(pwd)/adv-origin\" adv-client &&\n+\n+\t# The remote branch advances past the history we have, so its\n+\t# advertised tip is something we cannot use as a negative tip;\n+\t# only the shallow graft lets us exclude the full tree.\n+\ttest_commit --no-tag -C adv-origin c &&\n+\n+\tgit -C adv-client checkout -b topic &&\n+\ttest_commit --no-tag -C adv-client new &&\n+\tGIT_PROGRESS_DELAY=0 git -C adv-client push --progress origin topic 2>err &&\n+\n+\t# Only the new commit, its tree, and the new blob are sent; sending\n+\t# the full tree is avoided by excluding the shallow graft.\n+\ttest_grep \"Enumerating objects: 4, done.\" err\n+'\n+\n+test_expect_success 'push.shallowExcludeBoundary=false sends full tree' '\n+\tgit init adv-origin2 &&\n+\ttest_commit --no-tag -C adv-origin2 a &&\n+\ttest_commit --no-tag -C adv-origin2 b &&\n+\n+\tgit clone --depth=1 \"file://$(pwd)/adv-origin2\" adv-client2 &&\n+\ttest_commit --no-tag -C adv-origin2 c &&\n+\n+\tgit -C adv-client2 checkout -b topic &&\n+\ttest_commit --no-tag -C adv-client2 new &&\n+\tGIT_PROGRESS_DELAY=0 git -C adv-client2 \\\n+\t\t-c push.shallowExcludeBoundary=false \\\n+\t\tpush --progress origin topic 2>err &&\n+\n+\t# With the optimization disabled and no advertised ref pointing at\n+\t# the shallow graft, the full snapshot down to the shallow graft is\n+\t# resent, including its full tree.\n+\ttest_grep \"Enumerating objects: 7, done.\" err\n+'\n+\n+# A rejected ref must not over-exclude objects that another, accepted ref\n+# legitimately needs in the pack.  Set up a testcase using two independent\n+# shallow roots.\n+#\n+#   origin: two unrelated histories; only branch A carries blob O (sh=shared)\n+#       A:  A0---A1     (A0, A1 trees contain sh=O)\n+#       B:  B0---B1     (no \"shared\" blob)\n+#\n+#   receiver: seeded from branch B only, under both ref names; lacks blob O\n+#       refs/heads/B -> B1\n+#       refs/heads/A -> B1     (makes our A push a non-fast-forward)\n+#\n+#   client: \"clone --depth=1 --no-single-branch\" gives a graft at each tip\n+#           and a copy of blob O under A1   (x = cut parents = shallow graft)\n+#           x        x\n+#           |        |\n+#          A1       B1\n+#           |        |\n+#          cX     topic=cY     (cY re-adds sh=O, which the receiver lacks)\n+#\n+#   push \"A topic\" (non-atomic):\n+#     A     -> a non-fast-forward vs receiver A=B1, so its ref update is\n+#              rejected locally and never applied.  It still takes part in\n+#              the shared pack computation, and the buggy code also walked\n+#              back from it to graft A1 (which owns O).\n+#     topic -> accepted; cY grafts onto B1 and needs blob O.\n+#\n+#   Using the shallow graft A1 (an ancestor of A) to trim the pack, even\n+#   though our push of A is rejected locally, would omit blob O from topic's\n+#   pack -- yet topic needs O.  We want to ensure that when topic is pushed,\n+#   O is sent along with it despite A being rejected.\n+test_expect_success 'shallow push does not over-exclude for an accepted ref via a rejected one' '\n+\t# origin\n+\tgit init tworoot-origin &&\n+\tgit -C tworoot-origin checkout -b A &&\n+\ttest_commit -C tworoot-origin --no-tag has-shared sh shared &&\n+\ttest_commit -C tworoot-origin --no-tag A1 &&\n+\tgit -C tworoot-origin switch --orphan B &&\n+\ttest_commit -C tworoot-origin --no-tag B0 &&\n+\ttest_commit -C tworoot-origin --no-tag B1 &&\n+\n+\t# receiver: branch B only, exposed as both B and A\n+\tgit init --bare tworoot-receiver.git &&\n+\tgit -C tworoot-origin push \"file://$(pwd)/tworoot-receiver.git\" \\\n+\t\tB:refs/heads/B B:refs/heads/A &&\n+\n+\t# client: a shallow graft at each branch tip\n+\tgit clone --depth=1 --no-single-branch \\\n+\t\t\"file://$(pwd)/tworoot-origin\" tworoot-client &&\n+\n+\t# branch A gets commit cX; including A in the push gives us a\n+\t# locally-rejected ref whose graft A1 the buggy code walked to.  The A\n+\t# ref update is a non-fast-forward, so it is rejected and never applied.\n+\tgit -C tworoot-client checkout A &&\n+\ttest_commit -C tworoot-client --no-tag cX &&\n+\n+\t# branch topic is what we actually send, reintroducing blob O on B1\n+\tgit -C tworoot-client checkout -b topic B &&\n+\ttest_commit -C tworoot-client --no-tag reintroduce sh shared &&\n+\n+\t# push both in one command: they share a single pack computation, so a\n+\t# graft reached from the rejected A can strip objects that topic needs.\n+\t# The A ref update is rejected locally (non-fast-forward); the shared\n+\t# pack must still contain blob O for topic to land on the receiver.\n+\ttest_must_fail git -C tworoot-client push \\\n+\t\t\"file://$(pwd)/tworoot-receiver.git\" A topic &&\n+\tgit --git-dir=tworoot-receiver.git rev-parse --verify topic\n+'\n+\n+# push.shallowExcludeBoundary (default true) omits the shallow boundary\n+# snapshot from the pack, since an ordinary receiver already has it.  The\n+# exception is a receiver willing to adopt a *new* shallow root\n+# (receive.shallowUpdate): it genuinely needs that snapshot, so the default\n+# optimization leaves it unable to graft the new root.  Verify the receiver\n+# rejects such a push (rather than corrupting itself), and that setting the\n+# config to false restores the full snapshot and lets the push succeed.  This\n+# is the tradeoff that motivates the config knob.\n+test_expect_success 'default push to a shallowUpdate receiver rejects a rootless snapshot' '\n+\tgit init seed-origin &&\n+\ttest_commit -C seed-origin s1 &&\n+\ttest_commit -C seed-origin s2 &&\n+\ttest_commit -C seed-origin s3 &&\n+\n+\t# depth-2: a shallow graft at s2, pushing s3 on top of it\n+\tgit clone --depth=2 \"file://$(pwd)/seed-origin\" seed-client &&\n+\n+\tgit init --bare seed-receiver.git &&\n+\tgit --git-dir=seed-receiver.git config receive.shallowUpdate true &&\n+\n+\t# Default (optimization on): the s2 boundary snapshot is withheld, so\n+\t# the receiver cannot graft the new root and rejects the push, leaving\n+\t# the ref uncreated.\n+\ttest_must_fail git -C seed-client push \\\n+\t\t\"file://$(pwd)/seed-receiver.git\" HEAD:refs/heads/seeded 2>err &&\n+\ttest_grep \"remote rejected\" err &&\n+\ttest_must_fail git --git-dir=seed-receiver.git rev-parse --verify seeded &&\n+\n+\t# Opt-out: the full snapshot is sent, so the same push now succeeds and\n+\t# the new shallow root is grafted.\n+\tgit -C seed-client -c push.shallowExcludeBoundary=false push \\\n+\t\t\"file://$(pwd)/seed-receiver.git\" HEAD:refs/heads/seeded &&\n+\tgit --git-dir=seed-receiver.git rev-parse --verify seeded\n+'\n+\n test_done\n\nbase-commit: 5176dd3d057ac5cae8321508febef61fa88537aa\n-- \ngitgitgadget\n"},{"id":"551794","messageId":"f9de9449-2e32-483d-937d-45b847143b29@gmail.com","threadId":"66200","inReplyTo":"pull.2208.v2.git.1787684776048.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] send-pack: avoid sending the whole tree when pushing from a shallow clone","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-09-02T18:23:55Z","receivedAt":"2026-09-02T18:23:58Z","isPatch":true,"body":"On 8/25/2026 3:06 PM, Elijah Newren via GitGitGadget wrote:\n> From: Elijah Newren <newren@gmail.com>\n> \n> When pushing from a shallow clone, even if we only have made a small\n> one-line change to a tiny file, we often push the entire toplevel tree\n> of files.  For large repositories, this could be gigabytes instead of\n> kilobytes.\n> \n> The reason for this is that the push likely lacks the commits the\n> receiver has advertised, so it walks back to its shallow grafts.  Since\n> it doesn't know that the server has anything, it sends the entire tree\n> for the graft.  It would also send the parents of the shallow graft,\n> except the shallow clone doesn't have those by construction.  We thus\n> are forced to assume that the server has the parents of the shallow\n> graft -- if it doesn't, the server's receive-pack will reject the push.\n\nI was ready to assume this patch was fully correct, but then I asked\nan AI agent to review it and it found an interesting subtlety that\nputs the entire approach in question. It also presents an alternative\napproach that is much simpler and helps improve things immediately.\n\nThe gist is that we can attempt to push a shallow object to a remote\nthat _doesn't have that commit or its parent_. This gets rejected by\nthe remote as not allowing a shallow update.\n\nThe problem occurs when this shallow update is attempted alongside\nanother non-shallow branch being pushed that also has some \"new\"\nobjects reachable, so the \"assume the remote has the shallow\ncommit\" condition leads to novel failures due to that other ref\nupdate not having full connectivity.\n\nHere's a test for t5538 that the AI agent generated, and I\nmassaged into something more understandable/readable:\n\n# A ref that passes the client's checks can still be rejected by the receiver.\n# Its shallow graft must not trim objects needed by another ref in the shared\n# pack, since a non-atomic push should still allow that other ref to succeed.\n#\n# The client has two unrelated shallow histories (\"x\" marks a shallow graft).\n# Blob O is present in A1 and is reintroduced by cY on topic:\n#\n#                 contains O\n#                    |\n#       A0----------A1(x)---cX          refs/heads/A\n#\n#       B0----------B1(x)---cY          refs/heads/topic\n#                              \\\n#                               contains O\n#\n# The receiver has only the B history.  Both of its refs A and B point to\n# the same B1 commit as full history. It has neither A1 nor blob O in its\n# object database.\n#\n# The '--force' option lets the force-push of A from client to receiver\n# pass the client's checks, but the receiver rejects A because it will not\n# adopt A1 as a new shallow root.\ntest_expect_success 'shallow push does not over-exclude via a remotely rejected ref' '\n\t# origin: two unrelated histories; only branch A has blob \"shared\"\n\tgit init remote-reject-origin &&\n\t(\n\t\tcd remote-reject-origin &&\n\t\tgit checkout -b A &&\n\t\ttest_commit --no-tag has-shared sh shared &&\n\t\ttest_commit --no-tag A1 &&\n\t\tgit switch --orphan B &&\n\t\ttest_commit --no-tag B0 &&\n\t\ttest_commit --no-tag B1\n\t) &&\n\n\t# receiver: commit B1 is exposed as both B and A and lacks A1\n\tgit init --bare remote-reject-receiver.git &&\n\t(\n\t\tcd remote-reject-origin &&\n\t\tgit remote add receiver ../remote-reject-receiver.git &&\n\t\tgit push receiver B:refs/heads/B B:refs/heads/A\n\t) &&\n\n\t# client: each remote branch tip is a shallow graft\n\tgit clone --depth=1 --no-single-branch \\\n\t\t\"file://$(pwd)/remote-reject-origin\" remote-reject-client &&\n\n\told_a=$(cd remote-reject-receiver.git && git rev-parse A) &&\n\t(\n\t\tcd remote-reject-client &&\n\t\tgit remote add receiver ../remote-reject-receiver.git &&\n\n\t\t# Force makes A pass the client-side non-fast-forward check. The\n\t\t# receiver will reject it because A1 is a new shallow root and\n\t\t# receive.shallowUpdate is disabled.\n\t\tgit checkout A &&\n\t\ttest_commit --no-tag cX &&\n\n\t\t# topic is independently valid but needs the shared blob from A1.\n\t\tgit checkout -b topic B &&\n\t\ttest_commit --no-tag reintroduce sh shared &&\n\n\t\ttest_must_fail git push --force receiver A topic 2>err &&\n\t\ttest_grep \"remote rejected.*shallow update not allowed\" err\n\t) &&\n\n\t# The non-atomic push should reject A without affecting topic.\n\t(\n\t\tcd remote-reject-receiver.git &&\n\t\ttest \"$old_a\" = \"$(git rev-parse A)\" &&\n\t\tgit rev-parse --verify topic\n\t)\n'\n\nThis test passes before this patch, but fails after.\n\nAs I was working on this test case, the key step that will fail with the\ncurrent patch is the test_grep here:\n\n\ttest_must_fail git push --force receiver A topic 2>err &&\n\ttest_grep \"remote rejected.*shallow update not allowed\" err\n\nbecause the error that will be returned instead is more of a hard failure.\nThis failure \"at grep time\" is something I added. If this line doesn't\nexist, then the 'git rev-parse --verify topic' fails which shows that we\nare able to break the receiver repo with this push, as the second ref\nupdate is accepted even though the packfile isn't complete.\n\n> +static int check_to_send_update(const struct ref *ref, const struct send_pack_args *args);\n> +\n> +/*\n> + * Add the shallow grafts (nr_parent == -1), which are reachable from the\n> + * refs being pushed, to the pack boundary (\"haves\") as uninteresting\n> + * (negative) tips so the generated pack leaves out everything beneath them.\n\nThis \"which are reachable from the refs being pushed\" is the key problem,\nI think. We need to verify that the shallow commits are reachable from\nthe refs advertised by the remote.\n\n> + * Walk only from the pushed tips, and only until a graft: using a graft\n> + * that does not bound the pushed history could exclude an object we are\n> + * genuinely sending (if it is also reachable from that unrelated graft).\n> + * Stop early at any commit the peer already has, since it is a negative\n> + * the peer can use and the graft beneath it would be redundant.\n> + */\n> +static void append_reachable_shallow_grafts(struct repository *r,\n> +\t\t\t\t\t    const struct ref *refs,\n> +\t\t\t\t\t    const struct oid_array *advertised,\n> +\t\t\t\t\t    const struct oid_array *negotiated,\n> +\t\t\t\t\t    const struct send_pack_args *args,\n> +\t\t\t\t\t    struct oid_array *haves)\n\nWhen I asked the agent to implement something that instead cared about\nwhether the remote refs could reach the shallow commits, it deleted this\nmethod in favor of having your push.shallowexcludeboundary setting enable\npush.negotiate when the local repo is shallow:\n\n\trepo_config_get_bool(r, \"push.shallowexcludeboundary\",\n\t\t\t     &shallow_exclude_boundary);\n\tif (is_repository_shallow(r) && shallow_exclude_boundary)\n\t\tpush_negotiate = 1;\n\nThat was sufficient to pass the new test, as well as all other tests you\nadded, except one. I'm not sure if we need a new option or if we should\nrecommend push.negotiate in more places (plus these new tests).\n\nThese new tests are great:\n\n> +test_expect_success 'shallow push only pushes what is necessary' '\n> +test_expect_success 'push.shallowExcludeBoundary=false sends full tree' '\n> +test_expect_success 'shallow push does not over-exclude for an accepted ref via a rejected one' '\n\nThis test that you are adding is hinting at some of this behavior of the\nnew test I added, except the multi-ref push causes unexpected behavior:\n\n> +# push.shallowExcludeBoundary (default true) omits the shallow boundary\n> +# snapshot from the pack, since an ordinary receiver already has it.  The\n> +# exception is a receiver willing to adopt a *new* shallow root\n> +# (receive.shallowUpdate): it genuinely needs that snapshot, so the default\n> +# optimization leaves it unable to graft the new root.  Verify the receiver\n> +# rejects such a push (rather than corrupting itself), and that setting the\n> +# config to false restores the full snapshot and lets the push succeed.  This\n> +# is the tradeoff that motivates the config knob.\n> +test_expect_success 'default push to a shallowUpdate receiver rejects a rootless snapshot' '\n> +\tgit init seed-origin &&\n> +\ttest_commit -C seed-origin s1 &&\n> +\ttest_commit -C seed-origin s2 &&\n> +\ttest_commit -C seed-origin s3 &&\n> +\n> +\t# depth-2: a shallow graft at s2, pushing s3 on top of it\n> +\tgit clone --depth=2 \"file://$(pwd)/seed-origin\" seed-client &&\n> +\n> +\tgit init --bare seed-receiver.git &&\n> +\tgit --git-dir=seed-receiver.git config receive.shallowUpdate true &&\n> +\n\nHere is the chunk that doesn't work with the push.negotiate approach:\n\n> +\t# Default (optimization on): the s2 boundary snapshot is withheld, so\n> +\t# the receiver cannot graft the new root and rejects the push, leaving\n> +\t# the ref uncreated.\n> +\ttest_must_fail git -C seed-client push \\\n> +\t\t\"file://$(pwd)/seed-receiver.git\" HEAD:refs/heads/seeded 2>err &&\n> +\ttest_grep \"remote rejected\" err &&\n\nbut specifically it's because the remote doesn't reject it. The client\nmakes the appropriate adjustment.\n\n> +\ttest_must_fail git --git-dir=seed-receiver.git rev-parse --verify seeded &&\n> +\n> +\t# Opt-out: the full snapshot is sent, so the same push now succeeds and\n> +\t# the new shallow root is grafted.\n> +\tgit -C seed-client -c push.shallowExcludeBoundary=false push \\\n> +\t\t\"file://$(pwd)/seed-receiver.git\" HEAD:refs/heads/seeded &&\n> +\tgit --git-dir=seed-receiver.git rev-parse --verify seeded\n> +'\nSo the diff on your test becomes\n\n-       # Default (optimization on): the s2 boundary snapshot is withheld, so\n-       # the receiver cannot graft the new root and rejects the push, leaving\n-       # the ref uncreated.\n-       test_must_fail git -C seed-client push \\\n-               \"file://$(pwd)/seed-receiver.git\" HEAD:refs/heads/seeded 2>err &&\n-       test_grep \"remote rejected\" err &&\n-       test_must_fail git --git-dir=seed-receiver.git rev-parse --verify seeded &&\n-\n-       # Opt-out: the full snapshot is sent, so the same push now succeeds and\n-       # the new shallow root is grafted.\n-       git -C seed-client -c push.shallowExcludeBoundary=false push \\\n+       git -C seed-client rev-parse HEAD^ >expect &&\n+       git -C seed-client push \\\n                \"file://$(pwd)/seed-receiver.git\" HEAD:refs/heads/seeded &&\n-       git --git-dir=seed-receiver.git rev-parse --verify seeded\n+       git --git-dir=seed-receiver.git rev-parse --verify seeded &&\n+       test_cmp expect seed-receiver.git/shallow &&\n+       git --git-dir=seed-receiver.git fsck\n '\n\nThanks,\n-Stolee\n\n"},{"id":"551799","messageId":"1d6a4047-fa41-45cc-8097-88680e8ea67d@gmail.com","threadId":"66200","inReplyTo":"CABPp-BHwa7QM=XDuO=9xqm-OL8dn8uGf1=rv+sgBRQ9hHKMFuQ@mail.gmail.com","subject":"Re: [PATCH] send-pack: avoid sending the whole tree when pushing from a shallow clone","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2026-09-02T19:05:50Z","receivedAt":"2026-09-02T19:05:53Z","isPatch":true,"body":"Sorry that I missed this portion of the discussion talking about\npush.negotiate. Coming back to correct that.\n\nOn 8/25/2026 1:00 AM, Elijah Newren wrote:\n> On Sun, Aug 23, 2026 at 10:30 PM Patrick Steinhardt <ps@pks.im> wrote:\n>>\n> [...]\n>> TIL, thanks. I don't think I was even aware of \"push.negotiate\", and I\n>> mostly went by the folklore of \"just clone with --depth=2\" that I saw\n>> repeated on many sites.\n>>\n>> But this and all of your other answers make me lean strongly into the\n>> direction that the fix is at the wrong level, and the proper fix really\n>> is to enable \"push.negotiate\" by default.\n> \n> I don't think that fixes the problem, though:\n\nYou are right that the following cases are somewhat common.\n\n>   a) Users can do a shallow clone of a specific branch for a specific\n> pull-request/merge-request.  Then the pull-request/merge-request is\n> rebased, and sensitive data removed due to a leaked secret.  The\n> shallow graft is no longer common.  Pushing from the shallow clone\n> should fail, but it shouldn't have to send several gigabytes of data\n> in order to get the failure message.\n>   b) (Very similar to a) Users can do a shallow clone of one repo (a\n> local repository cache?) and then push to another; the shallow graft\n> thus may not be common.  An error is expected, but sending gigabytes\n> of data to get the error isn't.\n\nFor this case (b) I can think of it as doing a shallow clone of a\nbase repo (https://github.com/git/git) and then needing to push to\na user-owned fork (https://github.com/derrickstolee/git) and the\nfork not advertising reachability to the shallow commit.\n\nI think the difficulties here is that your approach is assuming\nsomething about how \"non-advertised\" objects may exist due to either\n\n a) delayed garbage collection, or\n b) shared object databases across a fork network.\n\nI don't think these are reasonable assumptions to have by default,\nso we need to be really clear about the reason to use this setting.\n\nAs your test demonstrates, some amount of \"our assumption was wrong\"\nis built in, so we should have a way for users to respond quickly\nor automatically (retry without the setting?).\n\nThe multi-push case that I brought up is tricky, though. It may\nbe very narrow, and HTTP servers would be protected, but we should\navoid allowing corruption over file:// protocol.\n\nThanks,\n-Stolee\n\n"},{"id":"551808","messageId":"CABPp-BFekYtzXo7BscEw=6CvPve-shA5ZvXkQaj-jcALE7Sx+w@mail.gmail.com","threadId":"66200","inReplyTo":"1d6a4047-fa41-45cc-8097-88680e8ea67d@gmail.com","subject":"Re: [PATCH] send-pack: avoid sending the whole tree when pushing from a shallow clone","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-09-02T20:57:00Z","receivedAt":"2026-09-02T20:57:15Z","isPatch":true,"body":"On Wed, Sep 2, 2026 at 12:05 PM Derrick Stolee <stolee@gmail.com> wrote:\n>\n> Sorry that I missed this portion of the discussion talking about\n> push.negotiate. Coming back to correct that.\n>\n> On 8/25/2026 1:00 AM, Elijah Newren wrote:\n> > On Sun, Aug 23, 2026 at 10:30 PM Patrick Steinhardt <ps@pks.im> wrote:\n> >>\n> > [...]\n> >> TIL, thanks. I don't think I was even aware of \"push.negotiate\", and I\n> >> mostly went by the folklore of \"just clone with --depth=2\" that I saw\n> >> repeated on many sites.\n> >>\n> >> But this and all of your other answers make me lean strongly into the\n> >> direction that the fix is at the wrong level, and the proper fix really\n> >> is to enable \"push.negotiate\" by default.\n> >\n> > I don't think that fixes the problem, though:\n>\n> You are right that the following cases are somewhat common.\n>\n> >   a) Users can do a shallow clone of a specific branch for a specific\n> > pull-request/merge-request.  Then the pull-request/merge-request is\n> > rebased, and sensitive data removed due to a leaked secret.  The\n> > shallow graft is no longer common.  Pushing from the shallow clone\n> > should fail, but it shouldn't have to send several gigabytes of data\n> > in order to get the failure message.\n> >   b) (Very similar to a) Users can do a shallow clone of one repo (a\n> > local repository cache?) and then push to another; the shallow graft\n> > thus may not be common.  An error is expected, but sending gigabytes\n> > of data to get the error isn't.\n\nI personally think (d) which you snipped out, namely\npush.negotiate=true doesn't work for some users/servers, may be more\ncommon.  I know you, Patrick, and I were all hoping that\npush.negotiate=true would be a panacea for the common case, but the\nconditions behind (d) that prevent that option from working for some\nusers would seem to be more common to me than these two conditions.\n\nFurther, my previous list for (d) was incomplete...\n\npush.negotiate=true can fail in another case both under http and ssh:\n  - repack replaces packfiles on the server with a new packfile.\n  - The client points to the shallow-graft as something it has.\n  - The server looks up that commit ID with QUICK, losing the race\nwith repacking, and reports it doesn't have it.\n  - The client doesn't have any more history further back so it can't\nfind any more shared history.\n  - Under current versions of git, the client believes it has to send\n_everything_ it has.\n\nIn the concurrent-repack discussion, upload-pack's QUICK \"have\" check\nwas deemed working-as-intended, on the grounds that a dropped \"have\"\njust means \"the client is sent more than it needs.\" For a shallow\nclone that \"bit more\" is the whole history the client has, which is\nexactly the problem this patch fixes.  I'm not trying to reopen that\nother discussion, and I admit this race is rare, but when it triggers,\nit'll defeat push.negotiate=true.  I think we need a backstop.  (And\neven if we do revisit that QUICK race, there's still the other\nconditions in my previous email under which push.negotiate=true\nfails.)\n\n[1] https://lore.kernel.org/git/20260827055743.GB189659@coredump.intra.peff.net/\n\n> For this case (b) I can think of it as doing a shallow clone of a\n> base repo (https://github.com/git/git) and then needing to push to\n> a user-owned fork (https://github.com/derrickstolee/git) and the\n> fork not advertising reachability to the shallow commit.\n\nYep, that's probably a better way to put it.\n\n> I think the difficulties here is that your approach is assuming\n> something about how \"non-advertised\" objects may exist due to either\n>\n>  a) delayed garbage collection, or\n>  b) shared object databases across a fork network.\n>\n> I don't think these are reasonable assumptions to have by default,\n> so we need to be really clear about the reason to use this setting.\n\nI don't follow.\n\nA shallow push already assumes something about how \"non-advertised\"\nobjects may exist -- it assumes the *parents* of the shallow graft\nexist on the server.  Why is it such a big leap to move from assuming\nthe server has the parents of the shallow graft to assuming it has the\nshallow graft itself?  Further, what are the consequences of assuming\nor not assuming the shallow graft exists?\n\nHere's the matrix:\n\nAssume the shallow graft exists:\n  (A) and it does -> push succeeds, and does so orders of magnitude\nfaster in large repos\n  (B) but it doesn't, nor does its parents -> push fails with error\nmessage we would have gotten anyway, and does so dramatically faster\n  (C) but it doesn't, but its parents (magically) do -> sends an error\nmessage quickly, where the push would have (eventually) previously\nsucceeded\n\nAssume the shallow graft doesn't exist:\n  (D) but it does -> push succeeds, AFTER pushing hundreds of\nmegabytes of almost certainly unnecessary data\n  (E) and it doesn't, nor does its parents -> get back an error\nmessage, AFTER pushing hundreds of megabytes of unnecessary data\n  (F) and it doesn't, but its parents (magically) do -> push succeeds,\nAFTER pushing hundreds of megabytes of mostly unnecessary data since\nwe can't determine which parts are necessary\n\nClearly, (A) and (B) are vastly superior to (D) and (E).  The only\ncase in question then is (C) vs (F).  My opinions there:\n\n(1) We already generally require folks to push from shallow clones\nback to repositories that have the parents of the shallow graft and\nextending that requirement to the shallow graft itself does not seem\nunreasonable to me.  I would much rather be told I'm pushing to the\nwrong remote than wait forever.\n(2) case C/F is incredibly unlikely (people tend to push back to the\nsame server, and even if they don't, the server likely either has the\nshallow graft and its history or is missing the parents of the graft\nas well).\n\n> As your test demonstrates, some amount of \"our assumption was wrong\"\n> is built in, so we should have a way for users to respond quickly\n> or automatically (retry without the setting?).\n>\n> The multi-push case that I brought up is tricky, though. It may\n> be very narrow, and HTTP servers would be protected, but we should\n> avoid allowing corruption over file:// protocol.\n\nAh!  I see where the disconnect may have been.  Yeah, corruption needs\nto be prevented, and if corruption was a risk then it'd override other\nconcerns.  But that isn't relevant here: receive-pack checks for\nconnectivity (regardless of protocol -- http, ssh, or file) and fails\nthe push if objects are missing.  (See commit 52fed6e1ce07\n(receive-pack: check connectivity before concluding \"git push\",\n2011-09-02)).  The multi-push case then ends up being a case of us\nfailing more refs than necessary, not a way to induce corruption.\n\n\nAlso, I didn't state this earlier, but this bug can actually be more\ncomical.  If someone clones with e.g. `git clone --depth ${N}\n--filter=blob:none --sparse ...`, then after making their changes and\ndeciding to push and the server no longer has references to one of the\ncommits in our shallow clone:\n  (i) Push notes that it knows no objects the server has -> it needs\nto send ALL trees and blobs from the shallow graft\n  (ii) It doesn't have ALL blobs from the shallow graft -> promisor\nremote handling kicks in\n  (iii) promisor remote downloads ALL blobs from the shallow graft\nfrom our origin\n  (iv) push can now push all objects to our origin\n\nAbove, you'll note that although the user started with a tiny clone,\nstep (iii) downloads huge amounts of data from origin so that step\n(iv) can upload that \"necessary\" data back to the server it just\ndownloaded it from.  My patch avoids both the unnecessary huge\ndownload and upload.\n"},{"id":"551841","messageId":"CABPp-BFi5xCg+cV-udwaq75QsV45mHmSu07ZGxTwp-1GP3YN0A@mail.gmail.com","threadId":"66200","inReplyTo":"f9de9449-2e32-483d-937d-45b847143b29@gmail.com","subject":"Re: [PATCH v2] send-pack: avoid sending the whole tree when pushing from a shallow clone","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2026-09-03T09:22:39Z","receivedAt":"2026-09-03T09:22:54Z","isPatch":true,"body":"Hi Stolee,\n\nOn Wed, Sep 2, 2026 at 11:23 AM Derrick Stolee <stolee@gmail.com> wrote:\n>\n> On 8/25/2026 3:06 PM, Elijah Newren via GitGitGadget wrote:\n> > From: Elijah Newren <newren@gmail.com>\n> >\n> > When pushing from a shallow clone, even if we only have made a small\n> > one-line change to a tiny file, we often push the entire toplevel tree\n> > of files.  For large repositories, this could be gigabytes instead of\n> > kilobytes.\n> >\n> > The reason for this is that the push likely lacks the commits the\n> > receiver has advertised, so it walks back to its shallow grafts.  Since\n> > it doesn't know that the server has anything, it sends the entire tree\n> > for the graft.  It would also send the parents of the shallow graft,\n> > except the shallow clone doesn't have those by construction.  We thus\n> > are forced to assume that the server has the parents of the shallow\n> > graft -- if it doesn't, the server's receive-pack will reject the push.\n>\n> I was ready to assume this patch was fully correct, but then I asked\n> an AI agent to review it and it found an interesting subtlety that\n> puts the entire approach in question. It also presents an alternative\n> approach that is much simpler and helps improve things immediately.\n\nThe bug you found here is a really good discovery; thanks for sending\nit along.  I think there are still some misunderstandings, though,\nwhich I think may significantly affect the resulting conclusion.\n\n> The gist is that we can attempt to push a shallow object to a remote\n> that _doesn't have that commit or its parent_. This gets rejected by\n> the remote as not allowing a shallow update.\n>\n> The problem occurs when this shallow update is attempted alongside\n> another non-shallow branch being pushed that also has some \"new\"\n> objects reachable, so the \"assume the remote has the shallow\n> commit\" condition leads to novel failures due to that other ref\n> update not having full connectivity.\n\nAh, I already had a similar test (\"does not over-exclude for an\naccepted ref via a rejected one\"), but this is a different variant I\noverlooked.  Good catch.\n\n> Here's a test for t5538 that the AI agent generated, and I\n> massaged into something more understandable/readable:\n>\n[...]\n>\n> This test passes before this patch, but fails after.\n>\n> As I was working on this test case, the key step that will fail with the\n> current patch is the test_grep here:\n>\n>         test_must_fail git push --force receiver A topic 2>err &&\n>         test_grep \"remote rejected.*shallow update not allowed\" err\n>\n> because the error that will be returned instead is more of a hard failure.\n> This failure \"at grep time\" is something I added. If this line doesn't\n> exist, then the 'git rev-parse --verify topic' fails which shows that we\n> are able to break the receiver repo with this push, as the second ref\n> update is accepted even though the packfile isn't complete.\n\nIsn't this self-contradictory?  Saying \"git rev-parse --verify topic\nfails\" means that `topic` was not created on the server.  Saying \"the\nsecond ref update is accepted\" claims it was created on the server.\n\nAlso, I'm not sure where you got \"break the receiver repo\" from.  When\nI re-run your exact testcase against the v2 patch, it is not broken:\n  - git fsck passes\n  - `A` remains unmodified\n  - `topic` was also rejected\nwhich seems to be guaranteed by 52fed6e1ce07 (receive-pack: check\nconnectivity before concluding \"git push\", 2011-09-02).\n\nIn particular, `git rev-parse --verify topic` failing here is the\n*safe* outcome which means the push was denied.  So, the case you\nprovided has no corruption.  In fact, all that has happened is that\nthis shallow push caused the pushes to fail.  A simple re-push of\nindividual refs by the user seems like the natural next step.\n\nHowever, the error message returned for this testcase is inscrutable;\nby my count the potential error messages here are about half a dozen\ndepending on the exact codepath that is triggered based on a few\ntweaks of config settings, and the unpack-objects ones are\nparticularly bad.  So we really ought to make those error messages\nbetter, and perhaps provide a hint to the user to just retry pushing\nindividual refs as a simple workaround; that'd point out to the user\nthat does hit your usecase that there's a really simple \"recovery\"\npath for them.  I've got some patches to fix that up.\n\n> When I asked the agent to implement something that instead cared about\n> whether the remote refs could reach the shallow commits, it deleted this\n> method in favor of having your push.shallowexcludeboundary setting enable\n> push.negotiate when the local repo is shallow:\n>\n>         repo_config_get_bool(r, \"push.shallowexcludeboundary\",\n>                              &shallow_exclude_boundary);\n>         if (is_repository_shallow(r) && shallow_exclude_boundary)\n>                 push_negotiate = 1;\n>\n> That was sufficient to pass the new test, as well as all other tests you\n> added, except one. I'm not sure if we need a new option or if we should\n> recommend push.negotiate in more places (plus these new tests).\n\nYeah, as noted elsewhere in this thread, there is a flowchart of\nreasons why push.negotiate=true will fail to solve the problem.  You\nhave since commented in that thread, so we can leave that discussion\nover there.\n"},{"id":"552059","messageId":"pull.2208.v3.git.1788679500.gitgitgadget@gmail.com","threadId":"66200","inReplyTo":"pull.2208.git.1787295352016.gitgitgadget@gmail.com","subject":"[PATCH v3 0/6] send-pack: avoid sending the whole tree when pushing from a shallow clone","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-06T07:24:54Z","receivedAt":"2026-09-06T07:25:03Z","isPatch":true,"body":"Changes since v2:\n\n * Rebased on master (ps/odb-pluggable-pack-generation has now been merged)\n * Completely rewrote the cover letter below\n * Fixed several suboptimal error messages related to Stolee's suggested\n   testcase (Patches [1-3]/6)\n * Made push.shallowExcludeBoundary a tri-state setting, initially keeping\n   the same default (Patch 4/6)\n * Argued for the change of default in a separate patch (Patch 5/6)\n * Added advice for retrying problematic multi-ref pushes separately. (Patch\n   6/6)\n\nChanges since v1:\n\n * Fixed two small code style issues\n * Updated the cover letter below to point out that push.negotiate=true\n   doesn't work for everyone, and even when negotiation does work it doesn't\n   solve all cases.\n * Updated to latest ps/odb-pluggable-pack-generation branch.\n\n[Note: push.negotiate=true is NOT a general solution to this problem.\nThere's too many holes it leaves open.]\n\nWhen pushing from a shallow clone, send-pack may know none of the commits\nadvertised by the receiver. Pack generation then walks back to the client's\nshallow boundary and sends the boundary commit's entire tree. A tiny change\ncan consequently result in transferring gigabytes of objects that the\nreceiver almost certainly already has.\n\nThe behavior can be even worse with a partial, sparse clone. For example, a\nrepository created with:\n\ngit clone --depth=2 --filter=blob:none --sparse ...\n\n\nmay not have the blobs from its shallow boundary locally. Before sending the\nunnecessarily large pack, Git first downloads those blobs from its promisor\nremote, only to upload them back to what is often the same server.\n\nA shallow client already omits the boundary commit's parents, which it does\nnot have, and relies on receive-pack's connectivity check to reject the push\nif the receiver lacks them. This series allows the client to make the same\nassumption about the boundary commit itself. Omitting that commit prevents\nits tree from becoming part of the generated pack.\n\nIf the receiver has the shallow graft commit, the push avoids transferring\nand recompressing its tree. If the receiver lacks both the shallow graft\ncommit and its history, the push was going to fail anyway, but now fails\nwithout first sending the large pack.\n\nThe special case to consider is the rare use of push to seed a receiver that\naccepts new shallow roots. Such a receiver already requires\nreceive.shallowUpdate=true; it must now be paired with\npush.shallowExcludeBoundary=false on the client so that the boundary\nsnapshot is sent.\n\nThe series introduces push.shallowExcludeBoundary with three values:\n\n * true omits reachable shallow boundaries from the pack;\n * false retains the historical behavior; and\n * abort refuses to choose either behavior, telling the user to specify.\n\nThe series first introduces the option while not changing the default, and\nthen argues for the change of default in a separate patch.\n\nAs highlighted by Stolee's testcase, omitting a boundary can also cause a\nshared pack for multiple refs to lack an object needed by one of those refs.\nHowever, there is no corruption -- the receiver safely rejects the affected\nupdates, and the final patch advises retrying the refs separately, allowing\nthe user to easily recover.\n\nBefore changing send-pack, the first three patches improve how receive-pack\nhandles and reports incomplete pushes. Missing objects are distinguished\nfrom type mismatches, repeated connectivity diagnostics are suppressed, and\na missing shallow boundary results in per-ref \"missing necessary objects\"\nerrors instead of receive-pack disconnecting.\n\nElijah Newren (6):\n  unpack-objects: distinguish missing objects from type mismatches\n  receive-pack: avoid repeating connectivity errors\n  shallow: reject missing boundaries without disconnecting\n  send-pack: optionally omit shallow boundaries\n  send-pack: default to excluding shallow boundaries\n  send-pack: advise splitting incomplete shallow pushes\n\n Documentation/config/advice.adoc |   5 +\n Documentation/config/push.adoc   |  23 ++++\n advice.c                         |   1 +\n advice.h                         |   1 +\n builtin/receive-pack.c           |   7 +\n builtin/unpack-objects.c         |   9 +-\n send-pack.c                      | 146 ++++++++++++++++++++-\n shallow.c                        |  16 ++-\n t/t5410-receive-pack.sh          |   6 +-\n t/t5504-fetch-receive-strict.sh  |   7 +-\n t/t5538-push-shallow.sh          | 216 ++++++++++++++++++++++++++++++-\n 11 files changed, 422 insertions(+), 15 deletions(-)\n\n\nbase-commit: 1630431f326e15fcde608827b5ff38422528eb59\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-2208%2Fnewren%2Favoid-expensive-shallow-pushes-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2208/newren/avoid-expensive-shallow-pushes-v3\nPull-Request: https://github.com/gitgitgadget/git/pull/2208\n\nRange-diff vs v2:\n\n -:  ---------- > 1:  6056689be0 unpack-objects: distinguish missing objects from type mismatches\n -:  ---------- > 2:  74a52a632e receive-pack: avoid repeating connectivity errors\n -:  ---------- > 3:  fc21ecf832 shallow: reject missing boundaries without disconnecting\n 1:  d4501a5c23 ! 4:  7a4fb38450 send-pack: avoid sending the whole tree when pushing from a shallow clone\n     @@ Metadata\n      Author: Elijah Newren <newren@gmail.com>\n      \n       ## Commit message ##\n     -    send-pack: avoid sending the whole tree when pushing from a shallow clone\n     +    send-pack: optionally omit shallow boundaries\n      \n     -    When pushing from a shallow clone, even if we only have made a small\n     -    one-line change to a tiny file, we often push the entire toplevel tree\n     -    of files.  For large repositories, this could be gigabytes instead of\n     -    kilobytes.\n     +    When the receiver advertises no commit the shallow client has, pack\n     +    generation walks to a shallow boundary and sends its entire tree. A tiny\n     +    push can consequently transfer gigabytes of objects the receiver likely\n     +    already has.\n      \n     -    The reason for this is that the push likely lacks the commits the\n     -    receiver has advertised, so it walks back to its shallow grafts.  Since\n     -    it doesn't know that the server has anything, it sends the entire tree\n     -    for the graft.  It would also send the parents of the shallow graft,\n     -    except the shallow clone doesn't have those by construction.  We thus\n     -    are forced to assume that the server has the parents of the shallow\n     -    graft -- if it doesn't, the server's receive-pack will reject the push.\n     +    The client already assumes the receiver has the boundary's parents,\n     +    which are absent from the shallow clone. Extend that option to the\n     +    boundary itself: push.shallowExcludeBoundary=true adds reachable shallow\n     +    grafts as negative tips, letting receive-pack's connectivity check\n     +    reject the push if the assumption is wrong.\n      \n     -    But that raises the obvious question: if we're going to assume the\n     -    server has the parents of the shallow graft, why not just assume the\n     -    server has the shallow graft itself -- which this clone almost certainly\n     -    received from the server when the shallow clone was created?  As noted\n     -    above, receive-pack already has a builtin connectivity check that\n     -    predates pushing from a shallow clone by years[*], so even if a client\n     -    is pushing to a different server than it cloned from, the worst that\n     -    happens is a rejected push.  And by assuming the server has the shallow\n     -    graft commits, then for large repositories (those most likely to use\n     -    shallow clone) we can avoid transferring (and perhaps re-compressing)\n     -    gigabytes of file contents that the server already has.\n     +    Only use grafts reached from refs contributing to the pack. An unrelated\n     +    graft could otherwise exclude an object another ref needs. Stop at\n     +    commits known to both sides, since they already bound the pack.\n      \n     -    [*] Compare 5dbd76760181 (receive/send-pack: support pushing from a\n     -        shallow clone, 2013-12-05) and 52fed6e1ce07 (receive-pack: check\n     -        connectivity before concluding \"git push\", 2011-09-02)\n     -\n     -    Fix this by finding the shallow grafts behind the history we're pushing\n     -    and adding them to the pack boundary as uninteresting (negative) tips,\n     -    so the generated pack leaves out everything underneath them.  We only\n     -    use grafts that the pushed commits can actually reach; excluding every\n     -    graft in the repository would be simpler, but it could drop an object we\n     -    really do need to send -- for example, a new blob we're pushing that\n     -    also happens to sit under some unrelated shallow root pulled from a\n     -    different remote.\n     -\n     -    We can also stop early at any commit we and the server both have --\n     -    one the server advertised, or that push negotiation found in common.\n     -    Such a commit already marks the edge of what we need to send, so\n     -    there's no reason to keep walking down to a graft below it.  For\n     -    deeper clones the server usually has a commit close by, which keeps\n     -    this walk short; we only reach a graft when we and the server share no\n     -    history that we know about.\n     -\n     -    One very rare (and non-default) workflow genuinely needs the larger\n     -    push: seeding a receiver willing to adopt new shallow roots\n     -    (receive.shallowUpdate; see 5dbd76760181 (receive/send-pack: support\n     -    pushing from a shallow clone, 2013-12-05) and 0a1bc12b6e40\n     -    (receive-pack: allow pushes that update .git/shallow, 2013-12-05)).\n     -    When the server sets receive.shallowUpdate, it is willing to accept\n     -    pushes despite lacking ancestors of the pushed commits.  But it expects\n     -    us to send all tree objects so it can graft a new shallow root.  For\n     -    that case, add a sender-side config, push.shallowExcludeBoundary,\n     -    defaulting to true (the optimization), while allowing users to set it to\n     -    false to restore the previous behavior needed for that rare case.\n     -\n     -    Update the existing shallow-seeding tests in t5538 to set\n     -    push.shallowExcludeBoundary=false, since they exercise that\n     -    receive.shallowUpdate path.  Add tests for the optimized default and the\n     -    opt-out, that a rejected ref does not cause an accepted ref to be\n     -    over-excluded, and that a shallowUpdate receiver still rejects a\n     -    rootless snapshot by default.\n     +    Also accept \"abort\" to make no assumption, and \"false\" to retain the\n     +    historical behavior required when seeding a receive.shallowUpdate\n     +    receiver.  Keep false as the default for now, so introducing the\n     +    mechanism does not change existing pushes.\n      \n          Signed-off-by: Elijah Newren <newren@gmail.com>\n      \n     @@ Documentation/config/push.adoc: This will result in only b (a and c are cleared)\n       \tin common.\n       \n      +`push.shallowExcludeBoundary`::\n     -+\tWhen pushing from a shallow repository (see linkgit:git-clone[1]\n     -+\t`--depth`), Git normally assumes that the receiving end already\n     -+\thas the pushing repository's shallow grafts, and omits those\n     -+\tobjects from the generated pack rather than resending the full\n     -+\ttoplevel tree of those grafts. This is safe because the\n     -+\treceiving end rejects a push that references objects it does not\n     -+\thave. Set this to `false` to send those objects anyway; this is\n     -+\tonly needed for the highly unusual case of using a push to seed\n     -+\ta receiver that adopts new shallow roots (i.e. a receiver that\n     -+\thas explicitly set `receive.shallowUpdate`). Default is `true`.\n     ++\tWhen pushing from a shallow repository, Git can omit the shallow\n     ++\tgrafts' objects from the generated pack rather than resending the\n     ++\tfull toplevel tree of those grafts.  This assumes the receiver\n     ++\talready has those objects.  If it does not, the receiver rejects\n     ++\tthe push rather than accepting incomplete history. This setting\n     ++\tcontrols that behavior and accepts three values:\n     +++\n     ++--\n     ++`abort`;;\n     ++\tIf the push reaches such a boundary, refuse it rather than\n     ++\tchoosing whether to send or omit it.\n     ++`true`;;\n     ++\tOmit the boundary objects (fast). If the receiver does not have\n     ++\tthem, the push is rejected.\n     ++`false`;;\n     ++\t(the default) Send the boundary objects, retaining the historical\n     ++\tbehavior.  This can send the boundary's entire tree, which may be\n     ++\tvery large.  This is only needed when pushing to a receiver that\n     ++\taccepts new shallow roots (i.e. one with `receive.shallowUpdate`\n     ++\tenabled), which is very rare.\n     ++--\n      +\n       `push.useBitmaps`::\n       \tIf set to `false`, disable use of bitmaps for `git push` even if\n     @@ send-pack.c: static void append_negative_object(struct repository *r,\n       \toid_array_append(haves, oid);\n       }\n       \n     -+static int check_to_send_update(const struct ref *ref, const struct send_pack_args *args);\n     ++static int check_to_send_update(const struct ref *ref,\n     ++\t\t\t\tconst struct send_pack_args *args);\n     ++\n     ++enum exclude_boundary_mode {\n     ++\tEXCLUDE_BOUNDARY_NONE = 0,\n     ++\tEXCLUDE_BOUNDARY_YES,\n     ++\tEXCLUDE_BOUNDARY_ABORT\n     ++};\n     ++\n     ++static enum exclude_boundary_mode get_exclude_boundary_mode(struct repository *r)\n     ++{\n     ++\tconst char *value;\n     ++\n     ++\tif (repo_config_get_string_tmp(r, \"push.shallowexcludeboundary\", &value))\n     ++\t\treturn EXCLUDE_BOUNDARY_NONE;\n     ++\n     ++\tswitch (git_parse_maybe_bool(value)) {\n     ++\tcase 1:\n     ++\t\treturn EXCLUDE_BOUNDARY_YES;\n     ++\tcase 0:\n     ++\t\treturn EXCLUDE_BOUNDARY_NONE;\n     ++\tdefault:\n     ++\t\tif (!strcasecmp(value, \"abort\"))\n     ++\t\t\treturn EXCLUDE_BOUNDARY_ABORT;\n     ++\t\tdie(_(\"bad push.shallowExcludeBoundary value: %s\"), value);\n     ++\t}\n     ++}\n      +\n      +/*\n     -+ * Add the shallow grafts (nr_parent == -1), which are reachable from the\n     -+ * refs being pushed, to the pack boundary (\"haves\") as uninteresting\n     -+ * (negative) tips so the generated pack leaves out everything beneath them.\n     -+ *\n     -+ * Walk only from the pushed tips, and only until a graft: using a graft\n     -+ * that does not bound the pushed history could exclude an object we are\n     -+ * genuinely sending (if it is also reachable from that unrelated graft).\n     -+ * Stop early at any commit the peer already has, since it is a negative\n     -+ * the peer can use and the graft beneath it would be redundant.\n     ++ * Append shallow grafts bounding contributing refs. Grafts from unrelated\n     ++ * history could exclude objects this push needs, while commits both sides\n     ++ * have make any graft below them irrelevant.\n      + */\n     -+static void append_reachable_shallow_grafts(struct repository *r,\n     ++static int append_reachable_shallow_grafts(struct repository *r,\n      +\t\t\t\t\t    const struct ref *refs,\n      +\t\t\t\t\t    const struct oid_array *advertised,\n      +\t\t\t\t\t    const struct oid_array *negotiated,\n     @@ send-pack.c: static void append_negative_object(struct repository *r,\n      +\tstruct oidset seen = OIDSET_INIT;\n      +\tstruct oidset known = OIDSET_INIT;\n      +\tconst struct ref *ref;\n     ++\tint found = 0;\n      +\tsize_t i;\n      +\n      +\tfor (i = 0; i < advertised->nr; i++)\n     @@ send-pack.c: static void append_negative_object(struct repository *r,\n      +\tfor (i = 0; i < negotiated->nr; i++)\n      +\t\toidset_insert(&known, &negotiated->oid[i]);\n      +\n     -+\t/*\n     -+\t * Record every commit the peer is known to have as a boundary for\n     -+\t * the walk, and seed the walk from the tips we are actually sending.\n     -+\t * The walk below does not begin until \"known\" is fully populated.\n     -+\t */\n     ++\t/* Populate \"known\" fully before starting the walk. */\n      +\tfor (ref = refs; ref; ref = ref->next) {\n      +\t\tstruct commit *commit;\n      +\n     @@ send-pack.c: static void append_negative_object(struct repository *r,\n      +\t\tif (oidset_insert(&seen, oid))\n      +\t\t\tcontinue;\n      +\n     -+\t\t/*\n     -+\t\t * A commit the peer already has bounds the pushed history\n     -+\t\t * with a negative it can use, so stop here rather than\n     -+\t\t * descend to a graft that would only be redundant.\n     -+\t\t */\n      +\t\tif (oidset_contains(&known, oid) &&\n      +\t\t    odb_has_object(r->objects, oid, 0))\n      +\t\t\tcontinue;\n     @@ send-pack.c: static void append_negative_object(struct repository *r,\n      +\t\tgraft = lookup_commit_graft(r, oid);\n      +\t\tif (graft && graft->nr_parent == -1) {\n      +\t\t\tappend_negative_object(r, haves, oid);\n     ++\t\t\tfound++;\n      +\t\t\tcontinue;\n      +\t\t}\n      +\n     @@ send-pack.c: static void append_negative_object(struct repository *r,\n      +\n      +\toidset_clear(&seen);\n      +\toidset_clear(&known);\n     ++\treturn found;\n      +}\n      +\n       /*\n     @@ send-pack.c: static int pack_objects(struct repository *r,\n       \tfor (size_t i = 0; i < negotiated->nr; i++)\n       \t\tappend_negative_object(r, &opts.haves, &negotiated->oid[i]);\n       \n     -+\t/*\n     -+\t * When pushing from a shallow repository, avoid re-pushing the\n     -+\t * entire toplevel tree.\n     -+\t */\n     -+\tif (is_repository_shallow(r)) {\n     -+\t\tint exclude_boundary = 1;\n     -+\t\trepo_config_get_bool(r, \"push.shallowexcludeboundary\",\n     -+\t\t\t\t     &exclude_boundary);\n     -+\t\tif (exclude_boundary)\n     -+\t\t\tappend_reachable_shallow_grafts(r, refs, advertised,\n     -+\t\t\t\t\t\t\tnegotiated, args,\n     -+\t\t\t\t\t\t\t&opts.haves);\n     -+\t}\n     ++\t/* Exclude reachable shallow boundaries from the pack. */\n     ++\tif (is_repository_shallow(r) &&\n     ++\t    get_exclude_boundary_mode(r) == EXCLUDE_BOUNDARY_YES)\n     ++\t\tappend_reachable_shallow_grafts(r, refs, advertised,\n     ++\t\t\t\t\t\tnegotiated, args,\n     ++\t\t\t\t\t\t&opts.haves);\n      +\n       \twhile (refs) {\n       \t\tif (!is_null_oid(&refs->old_oid))\n       \t\t\tappend_negative_object(r, &opts.haves, &refs->old_oid);\n     +@@ send-pack.c: int send_pack(struct repository *r,\n     + \t\t\tref->status = REF_STATUS_EXPECTING_REPORT;\n     + \t}\n     + \n     ++\t/* Honor ABORT before sending any ref-update commands. */\n     ++\tif (!args->dry_run && need_pack_data && is_repository_shallow(r) &&\n     ++\t    get_exclude_boundary_mode(r) == EXCLUDE_BOUNDARY_ABORT) {\n     ++\t\tstruct oid_array probe = OID_ARRAY_INIT;\n     ++\t\tint reachable = append_reachable_shallow_grafts(r, remote_refs,\n     ++\t\t\t\t\t\t\t\textra_have,\n     ++\t\t\t\t\t\t\t\t&commons, args,\n     ++\t\t\t\t\t\t\t\t&probe);\n     ++\t\toid_array_clear(&probe);\n     ++\t\tif (reachable)\n     ++\t\t\tdie(_(\"refusing to push a shallow boundary commit\\n\"\n     ++\t\t\t      \"Set push.shallowExcludeBoundary to true to omit it (fast),\\n\"\n     ++\t\t\t      \"or false to send it (needed for receive.shallowUpdate).\"));\n     ++\t}\n     ++\n     + \tif (!args->dry_run)\n     + \t\tadvertise_shallow_grafts_buf(r, &req_buf);\n     + \n      \n       ## t/t5538-push-shallow.sh ##\n     -@@ t/t5538-push-shallow.sh: EOF\n     - test_expect_success 'push from shallow clone, with grafted roots' '\n     - \t(\n     - \tcd shallow2 &&\n     --\ttest_must_fail git push ../.git +main:refs/remotes/shallow2/main 2>err &&\n     -+\ttest_must_fail git -c push.shallowExcludeBoundary=false \\\n     -+\t\tpush ../.git +main:refs/remotes/shallow2/main 2>err &&\n     - \ttest_grep \"shallow2/main.*shallow update not allowed\" err\n     - \t) &&\n     - \ttest_must_fail git rev-parse shallow2/main &&\n     -@@ t/t5538-push-shallow.sh: test_expect_success 'add new shallow root with receive.updateshallow on' '\n     - \ttest_config receive.shallowupdate true &&\n     - \t(\n     - \tcd shallow2 &&\n     --\tgit push ../.git +main:refs/remotes/shallow2/main\n     -+\tgit -c push.shallowExcludeBoundary=false \\\n     -+\t\tpush ../.git +main:refs/remotes/shallow2/main\n     - \t) &&\n     - \tgit log --format=%s shallow2/main >actual &&\n     - \tgit fsck &&\n     -@@ t/t5538-push-shallow.sh: test_expect_success 'push from shallow to shallow' '\n     - \t(\n     - \tcd shallow &&\n     - \tgit --git-dir=../shallow2/.git config receive.shallowupdate true &&\n     --\tgit push ../shallow2/.git +main:refs/remotes/shallow/main &&\n     -+\tgit -c push.shallowExcludeBoundary=false \\\n     -+\t\tpush ../shallow2/.git +main:refs/remotes/shallow/main &&\n     - \tgit --git-dir=../shallow2/.git config receive.shallowupdate false\n     - \t) &&\n     - \t(\n     -@@ t/t5538-push-shallow.sh: test_expect_success 'push new commit from shallow clone has good deltas' '\n     - \ttest_region pack-objects path-walk config-push.txt\n     +@@ t/t5538-push-shallow.sh: test_expect_success 'incomplete shallow push rejects without disconnecting' '\n     + \ttest_grep ! \"unable to parse commit\" err\n       '\n       \n     -+test_expect_success 'shallow push only pushes what is necessary' '\n     ++test_expect_success 'shallow boundary exclusion avoids sending the full tree' '\n      +\tgit init adv-origin &&\n      +\t# The shallow grafts are intentionally untagged so that no\n      +\t# advertised ref points at them.\n     @@ t/t5538-push-shallow.sh: test_expect_success 'push new commit from shallow clone\n      +\n      +\tgit -C adv-client checkout -b topic &&\n      +\ttest_commit --no-tag -C adv-client new &&\n     -+\tGIT_PROGRESS_DELAY=0 git -C adv-client push --progress origin topic 2>err &&\n     ++\tGIT_PROGRESS_DELAY=0 git -C adv-client \\\n     ++\t\t-c push.shallowExcludeBoundary=true \\\n     ++\t\tpush --progress origin topic 2>err &&\n      +\n      +\t# Only the new commit, its tree, and the new blob are sent; sending\n      +\t# the full tree is avoided by excluding the shallow graft.\n     @@ t/t5538-push-shallow.sh: test_expect_success 'push new commit from shallow clone\n      +\ttest_grep \"Enumerating objects: 7, done.\" err\n      +'\n      +\n     -+# A rejected ref must not over-exclude objects that another, accepted ref\n     -+# legitimately needs in the pack.  Set up a testcase using two independent\n     -+# shallow roots.\n     -+#\n     -+#   origin: two unrelated histories; only branch A carries blob O (sh=shared)\n     -+#       A:  A0---A1     (A0, A1 trees contain sh=O)\n     -+#       B:  B0---B1     (no \"shared\" blob)\n     -+#\n     -+#   receiver: seeded from branch B only, under both ref names; lacks blob O\n     -+#       refs/heads/B -> B1\n     -+#       refs/heads/A -> B1     (makes our A push a non-fast-forward)\n     -+#\n     -+#   client: \"clone --depth=1 --no-single-branch\" gives a graft at each tip\n     -+#           and a copy of blob O under A1   (x = cut parents = shallow graft)\n     -+#           x        x\n     -+#           |        |\n     -+#          A1       B1\n     -+#           |        |\n     -+#          cX     topic=cY     (cY re-adds sh=O, which the receiver lacks)\n     -+#\n     -+#   push \"A topic\" (non-atomic):\n     -+#     A     -> a non-fast-forward vs receiver A=B1, so its ref update is\n     -+#              rejected locally and never applied.  It still takes part in\n     -+#              the shared pack computation, and the buggy code also walked\n     -+#              back from it to graft A1 (which owns O).\n     -+#     topic -> accepted; cY grafts onto B1 and needs blob O.\n     -+#\n     -+#   Using the shallow graft A1 (an ancestor of A) to trim the pack, even\n     -+#   though our push of A is rejected locally, would omit blob O from topic's\n     -+#   pack -- yet topic needs O.  We want to ensure that when topic is pushed,\n     -+#   O is sent along with it despite A being rejected.\n     ++test_expect_success 'push.shallowExcludeBoundary=abort refuses when a graft is reached' '\n     ++\tgit init adv-origin3 &&\n     ++\ttest_commit --no-tag -C adv-origin3 a &&\n     ++\ttest_commit --no-tag -C adv-origin3 b &&\n     ++\n     ++\tgit clone --depth=1 \"file://$(pwd)/adv-origin3\" adv-client3 &&\n     ++\n     ++\t# The remote branch advances past the history we have, so its\n     ++\t# advertised tip cannot bound the walk; only the shallow graft could,\n     ++\t# which is exactly what \"abort\" refuses to rely on.\n     ++\ttest_commit --no-tag -C adv-origin3 c &&\n     ++\n     ++\tgit -C adv-client3 checkout -b topic &&\n     ++\ttest_commit --no-tag -C adv-client3 new &&\n     ++\n     ++\ttest_must_fail git -C adv-client3 \\\n     ++\t\t-c push.shallowExcludeBoundary=abort push origin topic 2>err &&\n     ++\ttest_grep \"push.shallowExcludeBoundary\" err &&\n     ++\n     ++\t# The receiver must be left untouched: no ref was created.\n     ++\ttest_must_fail git -C adv-origin3 rev-parse --verify refs/heads/topic\n     ++'\n     ++\n     ++# A and B are unrelated shallow histories. The receiver has B1 under both\n     ++# names, but lacks the \"shared\" blob from A1. The client adds cX atop A1 and\n     ++# reintroduces \"shared\" on a topic atop B1. Pushing A and topic together\n     ++# rejects A as a non-fast-forward, but A still participates in pack selection.\n     ++# Its A1 boundary must not exclude the blob needed by topic.\n      +test_expect_success 'shallow push does not over-exclude for an accepted ref via a rejected one' '\n     -+\t# origin\n      +\tgit init tworoot-origin &&\n      +\tgit -C tworoot-origin checkout -b A &&\n      +\ttest_commit -C tworoot-origin --no-tag has-shared sh shared &&\n     @@ t/t5538-push-shallow.sh: test_expect_success 'push new commit from shallow clone\n      +\ttest_commit -C tworoot-origin --no-tag B0 &&\n      +\ttest_commit -C tworoot-origin --no-tag B1 &&\n      +\n     -+\t# receiver: branch B only, exposed as both B and A\n      +\tgit init --bare tworoot-receiver.git &&\n      +\tgit -C tworoot-origin push \"file://$(pwd)/tworoot-receiver.git\" \\\n      +\t\tB:refs/heads/B B:refs/heads/A &&\n      +\n     -+\t# client: a shallow graft at each branch tip\n      +\tgit clone --depth=1 --no-single-branch \\\n      +\t\t\"file://$(pwd)/tworoot-origin\" tworoot-client &&\n      +\n     -+\t# branch A gets commit cX; including A in the push gives us a\n     -+\t# locally-rejected ref whose graft A1 the buggy code walked to.  The A\n     -+\t# ref update is a non-fast-forward, so it is rejected and never applied.\n      +\tgit -C tworoot-client checkout A &&\n      +\ttest_commit -C tworoot-client --no-tag cX &&\n      +\n     -+\t# branch topic is what we actually send, reintroducing blob O on B1\n      +\tgit -C tworoot-client checkout -b topic B &&\n      +\ttest_commit -C tworoot-client --no-tag reintroduce sh shared &&\n      +\n     -+\t# push both in one command: they share a single pack computation, so a\n     -+\t# graft reached from the rejected A can strip objects that topic needs.\n     -+\t# The A ref update is rejected locally (non-fast-forward); the shared\n     -+\t# pack must still contain blob O for topic to land on the receiver.\n     -+\ttest_must_fail git -C tworoot-client push \\\n     ++\ttest_must_fail git -C tworoot-client \\\n     ++\t\t-c push.shallowExcludeBoundary=true push \\\n      +\t\t\"file://$(pwd)/tworoot-receiver.git\" A topic &&\n      +\tgit --git-dir=tworoot-receiver.git rev-parse --verify topic\n      +'\n      +\n     -+# push.shallowExcludeBoundary (default true) omits the shallow boundary\n     -+# snapshot from the pack, since an ordinary receiver already has it.  The\n     -+# exception is a receiver willing to adopt a *new* shallow root\n     -+# (receive.shallowUpdate): it genuinely needs that snapshot, so the default\n     -+# optimization leaves it unable to graft the new root.  Verify the receiver\n     -+# rejects such a push (rather than corrupting itself), and that setting the\n     -+# config to false restores the full snapshot and lets the push succeed.  This\n     -+# is the tradeoff that motivates the config knob.\n     -+test_expect_success 'default push to a shallowUpdate receiver rejects a rootless snapshot' '\n     ++# A receive.shallowUpdate receiver needs the boundary snapshot to adopt a new\n     ++# shallow root, so omission must reject rather than create a broken ref.\n     ++test_expect_success 'push to a shallowUpdate receiver rejects a rootless snapshot' '\n      +\tgit init seed-origin &&\n      +\ttest_commit -C seed-origin s1 &&\n      +\ttest_commit -C seed-origin s2 &&\n     @@ t/t5538-push-shallow.sh: test_expect_success 'push new commit from shallow clone\n      +\tgit init --bare seed-receiver.git &&\n      +\tgit --git-dir=seed-receiver.git config receive.shallowUpdate true &&\n      +\n     -+\t# Default (optimization on): the s2 boundary snapshot is withheld, so\n     -+\t# the receiver cannot graft the new root and rejects the push, leaving\n     -+\t# the ref uncreated.\n     -+\ttest_must_fail git -C seed-client push \\\n     ++\t# Optimization on: the s2 boundary snapshot is withheld, so the\n     ++\t# receiver cannot graft the new root and rejects the push, leaving the\n     ++\t# ref uncreated.\n     ++\ttest_must_fail git -C seed-client \\\n     ++\t\t-c push.shallowExcludeBoundary=true push \\\n      +\t\t\"file://$(pwd)/seed-receiver.git\" HEAD:refs/heads/seeded 2>err &&\n      +\ttest_grep \"remote rejected\" err &&\n      +\ttest_must_fail git --git-dir=seed-receiver.git rev-parse --verify seeded &&\n -:  ---------- > 5:  afa44c6d22 send-pack: default to excluding shallow boundaries\n -:  ---------- > 6:  ae821ce078 send-pack: advise splitting incomplete shallow pushes\n\n-- \ngitgitgadget\n"},{"id":"552060","messageId":"6056689be039696d03dc67b8365300449b08676d.1788679500.git.gitgitgadget@gmail.com","threadId":"66200","inReplyTo":"pull.2208.v3.git.1788679500.gitgitgadget@gmail.com","subject":"[PATCH v3 1/6] unpack-objects: distinguish missing objects from type mismatches","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-06T07:24:55Z","receivedAt":"2026-09-06T07:25:04Z","isPatch":true,"body":"From: Elijah Newren <newren@gmail.com>\n\nWith receive.fsckObjects enabled, an incomplete pushed pack reports\n\"object of unexpected type\" when the expected object is simply absent.\nThat suggests corruption rather than identifying the missing object.\n\nUse the same diagnostics as index-pack: report \"did not receive expected\nobject\" when lookup fails, and reserve the type-mismatch message for an\nobject that exists with the wrong type.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n builtin/unpack-objects.c        | 9 +++++++--\n t/t5504-fetch-receive-strict.sh | 7 +++++--\n 2 files changed, 12 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c\nindex 351948724a..ceefeb5a49 100644\n--- a/builtin/unpack-objects.c\n+++ b/builtin/unpack-objects.c\n@@ -233,8 +233,13 @@ static int check_object(struct object *obj, enum object_type type,\n \tif (!(obj->flags & FLAG_OPEN)) {\n \t\tsize_t size;\n \t\tint type = odb_read_object_info(the_repository->objects, &obj->oid, &size);\n-\t\tif (type != obj->type || type <= 0)\n-\t\t\tdie(\"object of unexpected type\");\n+\t\tif (type <= 0)\n+\t\t\tdie(_(\"did not receive expected object %s\"),\n+\t\t\t    oid_to_hex(&obj->oid));\n+\t\tif (type != obj->type)\n+\t\t\tdie(_(\"object %s: expected type %s, found %s\"),\n+\t\t\t    oid_to_hex(&obj->oid),\n+\t\t\t    type_name(obj->type), type_name(type));\n \t\tobj->flags |= FLAG_WRITTEN;\n \t\treturn 0;\n \t}\ndiff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh\nindex 75b2b87999..0848e2da4a 100755\n--- a/t/t5504-fetch-receive-strict.sh\n+++ b/t/t5504-fetch-receive-strict.sh\n@@ -105,8 +105,11 @@ test_expect_success 'push with receive.fsckobjects' '\n \tTo dst\n \t!\trefs/heads/main:refs/heads/test\t[remote rejected] (unpacker error)\n \tEOF\n-\ttest_must_fail git push --porcelain dst main:refs/heads/test >act &&\n-\ttest_cmp exp act\n+\ttest_must_fail git push --porcelain dst main:refs/heads/test >act 2>err &&\n+\ttest_cmp exp act &&\n+\tmissing_oid=$(sed -e s%/%% S) &&\n+\ttest_grep \"did not receive expected object $missing_oid\" err &&\n+\ttest_grep ! \"object of unexpected type\" err\n '\n \n test_expect_success 'push with transfer.fsckobjects' '\n-- \ngitgitgadget\n\n"},{"id":"552061","messageId":"74a52a632e81e12a0b3fceebb50756c4fa434bb5.1788679500.git.gitgitgadget@gmail.com","threadId":"66200","inReplyTo":"pull.2208.v3.git.1788679500.gitgitgadget@gmail.com","subject":"[PATCH v3 2/6] receive-pack: avoid repeating connectivity errors","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-06T07:24:56Z","receivedAt":"2026-09-06T07:25:06Z","isPatch":true,"body":"From: Elijah Newren <newren@gmail.com>\n\nreceive-pack first checks all proposed ref tips together. If that bulk\nconnectivity check fails, it checks each tip separately to identify\nwhich ref updates need \"missing necessary objects\".\n\nThe bulk check already reports rev-list's diagnostic. The per-ref checks\nrepeat it merely as a side effect of attributing the failure,\npotentially once for every broken ref. Silence their stderr while\nretaining their exit status and the per-ref rejection.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n builtin/receive-pack.c  | 7 +++++++\n t/t5410-receive-pack.sh | 6 ++++--\n 2 files changed, 11 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex e6e54ba55f..8079901bb6 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1785,6 +1785,13 @@ static void set_connectivity_errors(struct command *commands,\n \t\t\t/* to be checked in update_shallow_ref() */\n \t\t\tcontinue;\n \n+\t\t/*\n+\t\t * The bulk check already reported rev-list's diagnostics;\n+\t\t * this per-ref pass only attributes the failure, so keep it\n+\t\t * quiet rather than repeat those errors for every ref.\n+\t\t */\n+\t\topt.quiet = 1;\n+\n \t\todb_transaction_env(transaction, &env);\n \t\topt.env = env.v;\n \ndiff --git a/t/t5410-receive-pack.sh b/t/t5410-receive-pack.sh\nindex 09d6bfd2a1..20d221044f 100755\n--- a/t/t5410-receive-pack.sh\n+++ b/t/t5410-receive-pack.sh\n@@ -68,9 +68,11 @@ test_expect_success TEE_DOES_NOT_HANG \\\n \t# Replay captured git-send-pack(1) output on new empty repository.\n \tgit init --bare remote.git &&\n \tgit receive-pack remote.git <out >actual 2>err &&\n+\tdepacketize <actual >actual.raw &&\n \n-\ttest_grep \"missing necessary objects\" actual &&\n-\ttest_grep \"fatal: Failed to traverse parents\" err &&\n+\ttest_grep \"missing necessary objects\" actual.raw &&\n+\ttest_grep \"fatal: Failed to traverse parents\" actual.raw &&\n+\ttest_must_be_empty err &&\n \ttest_must_fail git -C remote.git cat-file -e $(git -C repo rev-parse HEAD)\n '\n \n-- \ngitgitgadget\n\n"},{"id":"552062","messageId":"fc21ecf8327722ed02b656a85e76b4a60371597b.1788679500.git.gitgitgadget@gmail.com","threadId":"66200","inReplyTo":"pull.2208.v3.git.1788679500.gitgitgadget@gmail.com","subject":"[PATCH v3 3/6] shallow: reject missing boundaries without disconnecting","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-06T07:24:57Z","receivedAt":"2026-09-06T07:25:07Z","isPatch":true,"body":"From: Elijah Newren <newren@gmail.com>\n\nAn incomplete shallow push can refer to a boundary commit the receiver\ndoes not have. remove_nonexistent_theirs_shallow() drops that graft, so\npaint_down() does not recognize it as a boundary and dies when parsing\nthe missing commit. The client then sees only that the remote hung up.\n\nTreat an absent commit as the end of that traversal path rather than\naborting receive-pack. This lets paint_down() process the remaining\ncommits, after which the connectivity check rejects each affected ref\nwith \"missing necessary objects\". A present commit that cannot be parsed\nstill indicates corruption and remains fatal.\n\nAssisted-by: Claude Opus 4.8 & GPT-5.6 Sol\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n shallow.c               | 16 +++++++++++---\n t/t5538-push-shallow.sh | 46 +++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 59 insertions(+), 3 deletions(-)\n\ndiff --git a/shallow.c b/shallow.c\nindex 8e244a5669..c6f7437022 100644\n--- a/shallow.c\n+++ b/shallow.c\n@@ -659,9 +659,19 @@ static void paint_down(struct paint_info *info, const struct object_id *oid,\n \t\tif (c->object.flags & BOTTOM)\n \t\t\tcontinue;\n \n-\t\tif (repo_parse_commit(the_repository, c))\n-\t\t\tdie(\"unable to parse commit %s\",\n-\t\t\t    oid_to_hex(&c->object.oid));\n+\t\tif (repo_parse_commit_gently(the_repository, c, 1)) {\n+\t\t\t/*\n+\t\t\t * remove_nonexistent_theirs_shallow() may have\n+\t\t\t * dropped a missing boundary, leaving it unmarked\n+\t\t\t * as BOTTOM. Let the connectivity check reject a\n+\t\t\t * missing commit, but still die on a corrupt one.\n+\t\t\t */\n+\t\t\tif (odb_has_object(the_repository->objects,\n+\t\t\t\t\t   &c->object.oid, 0))\n+\t\t\t\tdie(\"unable to parse commit %s\",\n+\t\t\t\t    oid_to_hex(&c->object.oid));\n+\t\t\tcontinue;\n+\t\t}\n \n \t\tfor (p = c->parents; p; p = p->next) {\n \t\t\tif (p->item->object.flags & SEEN)\ndiff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh\nindex afab456b32..10ca7833d8 100755\n--- a/t/t5538-push-shallow.sh\n+++ b/t/t5538-push-shallow.sh\n@@ -164,4 +164,50 @@ test_expect_success 'push new commit from shallow clone has good deltas' '\n \ttest_region pack-objects path-walk config-push.txt\n '\n \n+test_expect_success 'incomplete shallow push rejects without disconnecting' '\n+\tgit init raw-origin &&\n+\tgit -C raw-origin checkout -b A &&\n+\ttest_commit -C raw-origin --no-tag has-shared sh shared &&\n+\ttest_commit -C raw-origin --no-tag A1 &&\n+\tA1=$(git -C raw-origin rev-parse HEAD) &&\n+\tgit -C raw-origin switch --orphan B &&\n+\ttest_commit -C raw-origin --no-tag B0 &&\n+\ttest_commit -C raw-origin --no-tag B1 &&\n+\tB1=$(git -C raw-origin rev-parse HEAD) &&\n+\n+\tgit init --bare raw-receiver.git &&\n+\tgit -C raw-receiver.git config receive.fsckObjects false &&\n+\tgit -C raw-origin push ../raw-receiver.git \\\n+\t\tB:refs/heads/B B:refs/heads/A &&\n+\n+\tgit -C raw-origin checkout A &&\n+\ttest_commit -C raw-origin --no-tag cX &&\n+\tcX=$(git -C raw-origin rev-parse HEAD) &&\n+\tgit -C raw-origin checkout -b topic B &&\n+\ttest_commit -C raw-origin --no-tag reintroduce sh shared &&\n+\ttopic=$(git -C raw-origin rev-parse HEAD) &&\n+\n+\t# Declare A1 and B1 as shallow, but omit them and their objects from\n+\t# the pack. This mimics an incomplete shallow push without relying on\n+\t# send-pack to create one.\n+\t{\n+\t\tprintf \"shallow %s\\nshallow %s\\n\" \"$A1\" \"$B1\" |\n+\t\tpacketize &&\n+\t\tprintf \"%s %s refs/heads/A\\0report-status object-format=%s\\n\" \\\n+\t\t\t\"$B1\" \"$cX\" \"$(test_oid algo)\" |\n+\t\tpacketize_raw &&\n+\t\tprintf \"%s %s refs/heads/topic\\n\" \"$ZERO_OID\" \"$topic\" |\n+\t\tpacketize &&\n+\t\tprintf 0000 &&\n+\t\tprintf \"%s\\n%s\\n^%s\\n^%s\\n\" \"$cX\" \"$topic\" \"$A1\" \"$B1\" |\n+\t\tgit -C raw-origin pack-objects --stdout --revs\n+\t} >input &&\n+\n+\tgit receive-pack raw-receiver.git <input >out 2>err &&\n+\tdepacketize <out >out.raw &&\n+\ttest_grep \"ng refs/heads/A missing necessary objects\" out.raw &&\n+\ttest_grep \"ng refs/heads/topic missing necessary objects\" out.raw &&\n+\ttest_grep ! \"unable to parse commit\" err\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"552063","messageId":"7a4fb3845034fe50b83169d518b5d2459259a533.1788679500.git.gitgitgadget@gmail.com","threadId":"66200","inReplyTo":"pull.2208.v3.git.1788679500.gitgitgadget@gmail.com","subject":"[PATCH v3 4/6] send-pack: optionally omit shallow boundaries","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-06T07:24:58Z","receivedAt":"2026-09-06T07:25:09Z","isPatch":true,"body":"From: Elijah Newren <newren@gmail.com>\n\nWhen the receiver advertises no commit the shallow client has, pack\ngeneration walks to a shallow boundary and sends its entire tree. A tiny\npush can consequently transfer gigabytes of objects the receiver likely\nalready has.\n\nThe client already assumes the receiver has the boundary's parents,\nwhich are absent from the shallow clone. Extend that option to the\nboundary itself: push.shallowExcludeBoundary=true adds reachable shallow\ngrafts as negative tips, letting receive-pack's connectivity check\nreject the push if the assumption is wrong.\n\nOnly use grafts reached from refs contributing to the pack. An unrelated\ngraft could otherwise exclude an object another ref needs. Stop at\ncommits known to both sides, since they already bound the pack.\n\nAlso accept \"abort\" to make no assumption, and \"false\" to retain the\nhistorical behavior required when seeding a receive.shallowUpdate\nreceiver.  Keep false as the default for now, so introducing the\nmechanism does not change existing pushes.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n Documentation/config/push.adoc |  23 ++++++\n send-pack.c                    | 122 ++++++++++++++++++++++++++++++\n t/t5538-push-shallow.sh        | 131 +++++++++++++++++++++++++++++++++\n 3 files changed, 276 insertions(+)\n\ndiff --git a/Documentation/config/push.adoc b/Documentation/config/push.adoc\nindex 28132eedfe..0ad55965e8 100644\n--- a/Documentation/config/push.adoc\n+++ b/Documentation/config/push.adoc\n@@ -134,6 +134,29 @@ This will result in only b (a and c are cleared).\n \trely solely on the server's ref advertisement to find commits\n \tin common.\n \n+`push.shallowExcludeBoundary`::\n+\tWhen pushing from a shallow repository, Git can omit the shallow\n+\tgrafts' objects from the generated pack rather than resending the\n+\tfull toplevel tree of those grafts.  This assumes the receiver\n+\talready has those objects.  If it does not, the receiver rejects\n+\tthe push rather than accepting incomplete history. This setting\n+\tcontrols that behavior and accepts three values:\n++\n+--\n+`abort`;;\n+\tIf the push reaches such a boundary, refuse it rather than\n+\tchoosing whether to send or omit it.\n+`true`;;\n+\tOmit the boundary objects (fast). If the receiver does not have\n+\tthem, the push is rejected.\n+`false`;;\n+\t(the default) Send the boundary objects, retaining the historical\n+\tbehavior.  This can send the boundary's entire tree, which may be\n+\tvery large.  This is only needed when pushing to a receiver that\n+\taccepts new shallow roots (i.e. one with `receive.shallowUpdate`\n+\tenabled), which is very rare.\n+--\n+\n `push.useBitmaps`::\n \tIf set to `false`, disable use of bitmaps for `git push` even if\n \t`pack.useBitmaps` is `true`, without preventing other git operations\ndiff --git a/send-pack.c b/send-pack.c\nindex f20460fbf4..386ea8b9a2 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -14,6 +14,7 @@\n #include \"transport.h\"\n #include \"version.h\"\n #include \"oid-array.h\"\n+#include \"oidset.h\"\n #include \"gpg-interface.h\"\n #include \"shallow.h\"\n #include \"parse-options.h\"\n@@ -55,6 +56,105 @@ static void append_negative_object(struct repository *r,\n \toid_array_append(haves, oid);\n }\n \n+static int check_to_send_update(const struct ref *ref,\n+\t\t\t\tconst struct send_pack_args *args);\n+\n+enum exclude_boundary_mode {\n+\tEXCLUDE_BOUNDARY_NONE = 0,\n+\tEXCLUDE_BOUNDARY_YES,\n+\tEXCLUDE_BOUNDARY_ABORT\n+};\n+\n+static enum exclude_boundary_mode get_exclude_boundary_mode(struct repository *r)\n+{\n+\tconst char *value;\n+\n+\tif (repo_config_get_string_tmp(r, \"push.shallowexcludeboundary\", &value))\n+\t\treturn EXCLUDE_BOUNDARY_NONE;\n+\n+\tswitch (git_parse_maybe_bool(value)) {\n+\tcase 1:\n+\t\treturn EXCLUDE_BOUNDARY_YES;\n+\tcase 0:\n+\t\treturn EXCLUDE_BOUNDARY_NONE;\n+\tdefault:\n+\t\tif (!strcasecmp(value, \"abort\"))\n+\t\t\treturn EXCLUDE_BOUNDARY_ABORT;\n+\t\tdie(_(\"bad push.shallowExcludeBoundary value: %s\"), value);\n+\t}\n+}\n+\n+/*\n+ * Append shallow grafts bounding contributing refs. Grafts from unrelated\n+ * history could exclude objects this push needs, while commits both sides\n+ * have make any graft below them irrelevant.\n+ */\n+static int append_reachable_shallow_grafts(struct repository *r,\n+\t\t\t\t\t    const struct ref *refs,\n+\t\t\t\t\t    const struct oid_array *advertised,\n+\t\t\t\t\t    const struct oid_array *negotiated,\n+\t\t\t\t\t    const struct send_pack_args *args,\n+\t\t\t\t\t    struct oid_array *haves)\n+{\n+\tstruct commit_list *pending = NULL;\n+\tstruct oidset seen = OIDSET_INIT;\n+\tstruct oidset known = OIDSET_INIT;\n+\tconst struct ref *ref;\n+\tint found = 0;\n+\tsize_t i;\n+\n+\tfor (i = 0; i < advertised->nr; i++)\n+\t\toidset_insert(&known, &advertised->oid[i]);\n+\tfor (i = 0; i < negotiated->nr; i++)\n+\t\toidset_insert(&known, &negotiated->oid[i]);\n+\n+\t/* Populate \"known\" fully before starting the walk. */\n+\tfor (ref = refs; ref; ref = ref->next) {\n+\t\tstruct commit *commit;\n+\n+\t\tif (!is_null_oid(&ref->old_oid))\n+\t\t\toidset_insert(&known, &ref->old_oid);\n+\n+\t\tif (is_null_oid(&ref->new_oid))\n+\t\t\tcontinue;\n+\t\tif (check_to_send_update(ref, args))\n+\t\t\tcontinue;\n+\t\tcommit = lookup_commit_reference_gently(r, &ref->new_oid, 1);\n+\t\tif (commit)\n+\t\t\tcommit_list_insert(commit, &pending);\n+\t}\n+\n+\twhile (pending) {\n+\t\tstruct commit *commit = pop_commit(&pending);\n+\t\tconst struct object_id *oid = &commit->object.oid;\n+\t\tstruct commit_graft *graft;\n+\t\tstruct commit_list *parent;\n+\n+\t\tif (oidset_insert(&seen, oid))\n+\t\t\tcontinue;\n+\n+\t\tif (oidset_contains(&known, oid) &&\n+\t\t    odb_has_object(r->objects, oid, 0))\n+\t\t\tcontinue;\n+\n+\t\tgraft = lookup_commit_graft(r, oid);\n+\t\tif (graft && graft->nr_parent == -1) {\n+\t\t\tappend_negative_object(r, haves, oid);\n+\t\t\tfound++;\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tif (repo_parse_commit(r, commit))\n+\t\t\tcontinue;\n+\t\tfor (parent = commit->parents; parent; parent = parent->next)\n+\t\t\tcommit_list_insert(parent->item, &pending);\n+\t}\n+\n+\toidset_clear(&seen);\n+\toidset_clear(&known);\n+\treturn found;\n+}\n+\n /*\n  * Make a pack stream and spit it out into file descriptor fd\n  */\n@@ -88,6 +188,13 @@ static int pack_objects(struct repository *r,\n \tfor (size_t i = 0; i < negotiated->nr; i++)\n \t\tappend_negative_object(r, &opts.haves, &negotiated->oid[i]);\n \n+\t/* Exclude reachable shallow boundaries from the pack. */\n+\tif (is_repository_shallow(r) &&\n+\t    get_exclude_boundary_mode(r) == EXCLUDE_BOUNDARY_YES)\n+\t\tappend_reachable_shallow_grafts(r, refs, advertised,\n+\t\t\t\t\t\tnegotiated, args,\n+\t\t\t\t\t\t&opts.haves);\n+\n \twhile (refs) {\n \t\tif (!is_null_oid(&refs->old_oid))\n \t\t\tappend_negative_object(r, &opts.haves, &refs->old_oid);\n@@ -644,6 +751,21 @@ int send_pack(struct repository *r,\n \t\t\tref->status = REF_STATUS_EXPECTING_REPORT;\n \t}\n \n+\t/* Honor ABORT before sending any ref-update commands. */\n+\tif (!args->dry_run && need_pack_data && is_repository_shallow(r) &&\n+\t    get_exclude_boundary_mode(r) == EXCLUDE_BOUNDARY_ABORT) {\n+\t\tstruct oid_array probe = OID_ARRAY_INIT;\n+\t\tint reachable = append_reachable_shallow_grafts(r, remote_refs,\n+\t\t\t\t\t\t\t\textra_have,\n+\t\t\t\t\t\t\t\t&commons, args,\n+\t\t\t\t\t\t\t\t&probe);\n+\t\toid_array_clear(&probe);\n+\t\tif (reachable)\n+\t\t\tdie(_(\"refusing to push a shallow boundary commit\\n\"\n+\t\t\t      \"Set push.shallowExcludeBoundary to true to omit it (fast),\\n\"\n+\t\t\t      \"or false to send it (needed for receive.shallowUpdate).\"));\n+\t}\n+\n \tif (!args->dry_run)\n \t\tadvertise_shallow_grafts_buf(r, &req_buf);\n \ndiff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh\nindex 10ca7833d8..67db51e60e 100755\n--- a/t/t5538-push-shallow.sh\n+++ b/t/t5538-push-shallow.sh\n@@ -210,4 +210,135 @@ test_expect_success 'incomplete shallow push rejects without disconnecting' '\n \ttest_grep ! \"unable to parse commit\" err\n '\n \n+test_expect_success 'shallow boundary exclusion avoids sending the full tree' '\n+\tgit init adv-origin &&\n+\t# The shallow grafts are intentionally untagged so that no\n+\t# advertised ref points at them.\n+\ttest_commit --no-tag -C adv-origin a &&\n+\ttest_commit --no-tag -C adv-origin b &&\n+\n+\tgit clone --depth=1 \"file://$(pwd)/adv-origin\" adv-client &&\n+\n+\t# The remote branch advances past the history we have, so its\n+\t# advertised tip is something we cannot use as a negative tip;\n+\t# only the shallow graft lets us exclude the full tree.\n+\ttest_commit --no-tag -C adv-origin c &&\n+\n+\tgit -C adv-client checkout -b topic &&\n+\ttest_commit --no-tag -C adv-client new &&\n+\tGIT_PROGRESS_DELAY=0 git -C adv-client \\\n+\t\t-c push.shallowExcludeBoundary=true \\\n+\t\tpush --progress origin topic 2>err &&\n+\n+\t# Only the new commit, its tree, and the new blob are sent; sending\n+\t# the full tree is avoided by excluding the shallow graft.\n+\ttest_grep \"Enumerating objects: 4, done.\" err\n+'\n+\n+test_expect_success 'push.shallowExcludeBoundary=false sends full tree' '\n+\tgit init adv-origin2 &&\n+\ttest_commit --no-tag -C adv-origin2 a &&\n+\ttest_commit --no-tag -C adv-origin2 b &&\n+\n+\tgit clone --depth=1 \"file://$(pwd)/adv-origin2\" adv-client2 &&\n+\ttest_commit --no-tag -C adv-origin2 c &&\n+\n+\tgit -C adv-client2 checkout -b topic &&\n+\ttest_commit --no-tag -C adv-client2 new &&\n+\tGIT_PROGRESS_DELAY=0 git -C adv-client2 \\\n+\t\t-c push.shallowExcludeBoundary=false \\\n+\t\tpush --progress origin topic 2>err &&\n+\n+\t# With the optimization disabled and no advertised ref pointing at\n+\t# the shallow graft, the full snapshot down to the shallow graft is\n+\t# resent, including its full tree.\n+\ttest_grep \"Enumerating objects: 7, done.\" err\n+'\n+\n+test_expect_success 'push.shallowExcludeBoundary=abort refuses when a graft is reached' '\n+\tgit init adv-origin3 &&\n+\ttest_commit --no-tag -C adv-origin3 a &&\n+\ttest_commit --no-tag -C adv-origin3 b &&\n+\n+\tgit clone --depth=1 \"file://$(pwd)/adv-origin3\" adv-client3 &&\n+\n+\t# The remote branch advances past the history we have, so its\n+\t# advertised tip cannot bound the walk; only the shallow graft could,\n+\t# which is exactly what \"abort\" refuses to rely on.\n+\ttest_commit --no-tag -C adv-origin3 c &&\n+\n+\tgit -C adv-client3 checkout -b topic &&\n+\ttest_commit --no-tag -C adv-client3 new &&\n+\n+\ttest_must_fail git -C adv-client3 \\\n+\t\t-c push.shallowExcludeBoundary=abort push origin topic 2>err &&\n+\ttest_grep \"push.shallowExcludeBoundary\" err &&\n+\n+\t# The receiver must be left untouched: no ref was created.\n+\ttest_must_fail git -C adv-origin3 rev-parse --verify refs/heads/topic\n+'\n+\n+# A and B are unrelated shallow histories. The receiver has B1 under both\n+# names, but lacks the \"shared\" blob from A1. The client adds cX atop A1 and\n+# reintroduces \"shared\" on a topic atop B1. Pushing A and topic together\n+# rejects A as a non-fast-forward, but A still participates in pack selection.\n+# Its A1 boundary must not exclude the blob needed by topic.\n+test_expect_success 'shallow push does not over-exclude for an accepted ref via a rejected one' '\n+\tgit init tworoot-origin &&\n+\tgit -C tworoot-origin checkout -b A &&\n+\ttest_commit -C tworoot-origin --no-tag has-shared sh shared &&\n+\ttest_commit -C tworoot-origin --no-tag A1 &&\n+\tgit -C tworoot-origin switch --orphan B &&\n+\ttest_commit -C tworoot-origin --no-tag B0 &&\n+\ttest_commit -C tworoot-origin --no-tag B1 &&\n+\n+\tgit init --bare tworoot-receiver.git &&\n+\tgit -C tworoot-origin push \"file://$(pwd)/tworoot-receiver.git\" \\\n+\t\tB:refs/heads/B B:refs/heads/A &&\n+\n+\tgit clone --depth=1 --no-single-branch \\\n+\t\t\"file://$(pwd)/tworoot-origin\" tworoot-client &&\n+\n+\tgit -C tworoot-client checkout A &&\n+\ttest_commit -C tworoot-client --no-tag cX &&\n+\n+\tgit -C tworoot-client checkout -b topic B &&\n+\ttest_commit -C tworoot-client --no-tag reintroduce sh shared &&\n+\n+\ttest_must_fail git -C tworoot-client \\\n+\t\t-c push.shallowExcludeBoundary=true push \\\n+\t\t\"file://$(pwd)/tworoot-receiver.git\" A topic &&\n+\tgit --git-dir=tworoot-receiver.git rev-parse --verify topic\n+'\n+\n+# A receive.shallowUpdate receiver needs the boundary snapshot to adopt a new\n+# shallow root, so omission must reject rather than create a broken ref.\n+test_expect_success 'push to a shallowUpdate receiver rejects a rootless snapshot' '\n+\tgit init seed-origin &&\n+\ttest_commit -C seed-origin s1 &&\n+\ttest_commit -C seed-origin s2 &&\n+\ttest_commit -C seed-origin s3 &&\n+\n+\t# depth-2: a shallow graft at s2, pushing s3 on top of it\n+\tgit clone --depth=2 \"file://$(pwd)/seed-origin\" seed-client &&\n+\n+\tgit init --bare seed-receiver.git &&\n+\tgit --git-dir=seed-receiver.git config receive.shallowUpdate true &&\n+\n+\t# Optimization on: the s2 boundary snapshot is withheld, so the\n+\t# receiver cannot graft the new root and rejects the push, leaving the\n+\t# ref uncreated.\n+\ttest_must_fail git -C seed-client \\\n+\t\t-c push.shallowExcludeBoundary=true push \\\n+\t\t\"file://$(pwd)/seed-receiver.git\" HEAD:refs/heads/seeded 2>err &&\n+\ttest_grep \"remote rejected\" err &&\n+\ttest_must_fail git --git-dir=seed-receiver.git rev-parse --verify seeded &&\n+\n+\t# Opt-out: the full snapshot is sent, so the same push now succeeds and\n+\t# the new shallow root is grafted.\n+\tgit -C seed-client -c push.shallowExcludeBoundary=false push \\\n+\t\t\"file://$(pwd)/seed-receiver.git\" HEAD:refs/heads/seeded &&\n+\tgit --git-dir=seed-receiver.git rev-parse --verify seeded\n+'\n+\n test_done\n-- \ngitgitgadget\n\n"},{"id":"552064","messageId":"afa44c6d2262dda7d04ba243fdd47563d997561d.1788679500.git.gitgitgadget@gmail.com","threadId":"66200","inReplyTo":"pull.2208.v3.git.1788679500.gitgitgadget@gmail.com","subject":"[PATCH v3 5/6] send-pack: default to excluding shallow boundaries","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-06T07:24:59Z","receivedAt":"2026-09-06T07:25:10Z","isPatch":true,"body":"From: Elijah Newren <newren@gmail.com>\n\nSending a shallow boundary is almost always wasted work. We got the\nshallow boundary from somewhere, and most likely that is the server we\nare pushing to.  If the receiver has the boundary, omitting it avoids\ntransferring and recompressing its entire tree.  If the receiver lacks\nboth it and its history, the push is rejected either way, but omission\nreaches that answer without first sending the tree.\n\nMake push.shallowExcludeBoundary default to true. This also covers cases\nwhere push negotiation is disabled, unavailable, or fails to find the\nboundary, so users do not need special configuration to avoid\nunexpectedly huge pushes.\n\nThe practical compatibility cost is the rare use of push to seed a new\nshallow root. That already requires receive.shallowUpdate on the server;\nit now also requires push.shallowExcludeBoundary=false on the client so\nthe receiver gets the boundary snapshot.\n\nTwo other edge cases instead fail faster with the new default:\n\n  (A) A receiver has the boundary's parents but not the boundary itself.\n      This likely means the user is pushing to the wrong receiver, where\n      a quick rejection is preferable to a slow accidental success.\n\n  (B) In a multi-ref push, one ref's shallow boundary can exclude objects\n      needed by another ref. This may reject more refs than necessary,\n      but retrying the refs separately avoids the problem; the next\n      patch advises users to do so.\n\nNeither case justifies making every ordinary shallow push send the\nboundary's potentially enormous tree.\n\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n Documentation/config/push.adoc | 10 +++++-----\n send-pack.c                    |  2 +-\n t/t5538-push-shallow.sh        | 10 ++++++----\n 3 files changed, 12 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/config/push.adoc b/Documentation/config/push.adoc\nindex 0ad55965e8..a08ec04c21 100644\n--- a/Documentation/config/push.adoc\n+++ b/Documentation/config/push.adoc\n@@ -147,12 +147,12 @@ This will result in only b (a and c are cleared).\n \tIf the push reaches such a boundary, refuse it rather than\n \tchoosing whether to send or omit it.\n `true`;;\n-\tOmit the boundary objects (fast). If the receiver does not have\n-\tthem, the push is rejected.\n+\t(the default) Omit the boundary objects (fast). If the receiver\n+\tdoes not have them, the push is rejected.\n `false`;;\n-\t(the default) Send the boundary objects, retaining the historical\n-\tbehavior.  This can send the boundary's entire tree, which may be\n-\tvery large.  This is only needed when pushing to a receiver that\n+\tSend the boundary objects, retaining the historical behavior.\n+\tThis can send the boundary's entire tree, which may be very\n+\tlarge.  This is only needed when pushing to a receiver that\n \taccepts new shallow roots (i.e. one with `receive.shallowUpdate`\n \tenabled), which is very rare.\n --\ndiff --git a/send-pack.c b/send-pack.c\nindex 386ea8b9a2..8a7cedf65a 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -70,7 +70,7 @@ static enum exclude_boundary_mode get_exclude_boundary_mode(struct repository *r\n \tconst char *value;\n \n \tif (repo_config_get_string_tmp(r, \"push.shallowexcludeboundary\", &value))\n-\t\treturn EXCLUDE_BOUNDARY_NONE;\n+\t\treturn EXCLUDE_BOUNDARY_YES;\n \n \tswitch (git_parse_maybe_bool(value)) {\n \tcase 1:\ndiff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh\nindex 67db51e60e..e52f3e50e2 100755\n--- a/t/t5538-push-shallow.sh\n+++ b/t/t5538-push-shallow.sh\n@@ -64,7 +64,8 @@ EOF\n test_expect_success 'push from shallow clone, with grafted roots' '\n \t(\n \tcd shallow2 &&\n-\ttest_must_fail git push ../.git +main:refs/remotes/shallow2/main 2>err &&\n+\ttest_must_fail git -c push.shallowExcludeBoundary=false \\\n+\t\tpush ../.git +main:refs/remotes/shallow2/main 2>err &&\n \ttest_grep \"shallow2/main.*shallow update not allowed\" err\n \t) &&\n \ttest_must_fail git rev-parse shallow2/main &&\n@@ -75,7 +76,8 @@ test_expect_success 'add new shallow root with receive.updateshallow on' '\n \ttest_config receive.shallowupdate true &&\n \t(\n \tcd shallow2 &&\n-\tgit push ../.git +main:refs/remotes/shallow2/main\n+\tgit -c push.shallowExcludeBoundary=false \\\n+\t\tpush ../.git +main:refs/remotes/shallow2/main\n \t) &&\n \tgit log --format=%s shallow2/main >actual &&\n \tgit fsck &&\n@@ -90,7 +92,8 @@ test_expect_success 'push from shallow to shallow' '\n \t(\n \tcd shallow &&\n \tgit --git-dir=../shallow2/.git config receive.shallowupdate true &&\n-\tgit push ../shallow2/.git +main:refs/remotes/shallow/main &&\n+\tgit -c push.shallowExcludeBoundary=false \\\n+\t\tpush ../shallow2/.git +main:refs/remotes/shallow/main &&\n \tgit --git-dir=../shallow2/.git config receive.shallowupdate false\n \t) &&\n \t(\n@@ -227,7 +230,6 @@ test_expect_success 'shallow boundary exclusion avoids sending the full tree' '\n \tgit -C adv-client checkout -b topic &&\n \ttest_commit --no-tag -C adv-client new &&\n \tGIT_PROGRESS_DELAY=0 git -C adv-client \\\n-\t\t-c push.shallowExcludeBoundary=true \\\n \t\tpush --progress origin topic 2>err &&\n \n \t# Only the new commit, its tree, and the new blob are sent; sending\n-- \ngitgitgadget\n\n"},{"id":"552065","messageId":"ae821ce0784286486fe76117b90bce78610ea37f.1788679500.git.gitgitgadget@gmail.com","threadId":"66200","inReplyTo":"pull.2208.v3.git.1788679500.gitgitgadget@gmail.com","subject":"[PATCH v3 6/6] send-pack: advise splitting incomplete shallow pushes","fromName":"Elijah Newren via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2026-09-06T07:25:00Z","receivedAt":"2026-09-06T07:25:12Z","isPatch":true,"body":"From: Elijah Newren <newren@gmail.com>\n\nWhen several refs share a pack, an omitted shallow boundary reached from\none ref can exclude an object needed by another. Pushing each ref\nseparately recomputes the pack and avoids that interaction.\n\nWhen such a multi-ref push fails after excluding a boundary, suggest\nseparate pushes. Gate the message on advice.pushShallowBoundary.\n\nAssisted-by: Claude Opus 4.8\nSigned-off-by: Elijah Newren <newren@gmail.com>\n---\n Documentation/config/advice.adoc |  5 +++++\n advice.c                         |  1 +\n advice.h                         |  1 +\n send-pack.c                      | 26 ++++++++++++++++++++++----\n t/t5538-push-shallow.sh          | 31 +++++++++++++++++++++++++++++++\n 5 files changed, 60 insertions(+), 4 deletions(-)\n\ndiff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc\nindex 81f80a9274..6bb6955246 100644\n--- a/Documentation/config/advice.adoc\n+++ b/Documentation/config/advice.adoc\n@@ -99,6 +99,11 @@ all advice messages.\n \t\ta configured remote but looks like a `<remote>/<branch>` ref,\n \t\tsuggesting that the remote and branch be given as separate\n \t\targuments.\n+\tpushShallowBoundary::\n+\t\tShown when a push from a shallow clone is rejected because\n+\t\tthe remote could not unpack the pack, hinting that a shallow\n+\t\tboundary may have omitted objects and suggesting the refs be\n+\t\tpushed one at a time.\n \tpushUnqualifiedRefname::\n \t\tShown when linkgit:git-push[1] gives up trying to\n \t\tguess based on the source and destination refs what\ndiff --git a/advice.c b/advice.c\nindex 63bf8b0c5f..3701672048 100644\n--- a/advice.c\n+++ b/advice.c\n@@ -70,6 +70,7 @@ static struct {\n \t[ADVICE_PUSH_NON_FF_MATCHING]\t\t\t= { \"pushNonFFMatching\" },\n \t[ADVICE_PUSH_REF_NEEDS_UPDATE]\t\t\t= { \"pushRefNeedsUpdate\" },\n \t[ADVICE_PUSH_REPO_LOOKS_LIKE_REF]\t\t= { \"pushRepoLooksLikeRef\" },\n+\t[ADVICE_PUSH_SHALLOW_BOUNDARY]\t\t\t= { \"pushShallowBoundary\" },\n \t[ADVICE_PUSH_UNQUALIFIED_REF_NAME]\t\t= { \"pushUnqualifiedRefName\" },\n \t[ADVICE_PUSH_UPDATE_REJECTED]\t\t\t= { \"pushUpdateRejected\" },\n \t[ADVICE_PUSH_UPDATE_REJECTED_ALIAS]\t\t= { \"pushNonFastForward\" }, /* backwards compatibility */\ndiff --git a/advice.h b/advice.h\nindex 66f6cd6a77..b2e281baa5 100644\n--- a/advice.h\n+++ b/advice.h\n@@ -37,6 +37,7 @@ enum advice_type {\n \tADVICE_PUSH_NON_FF_MATCHING,\n \tADVICE_PUSH_REF_NEEDS_UPDATE,\n \tADVICE_PUSH_REPO_LOOKS_LIKE_REF,\n+\tADVICE_PUSH_SHALLOW_BOUNDARY,\n \tADVICE_PUSH_UNQUALIFIED_REF_NAME,\n \tADVICE_PUSH_UPDATE_REJECTED,\n \tADVICE_PUSH_UPDATE_REJECTED_ALIAS,\ndiff --git a/send-pack.c b/send-pack.c\nindex 8a7cedf65a..4fa17810a7 100644\n--- a/send-pack.c\n+++ b/send-pack.c\n@@ -1,4 +1,5 @@\n #include \"git-compat-util.h\"\n+#include \"advice.h\"\n #include \"config.h\"\n #include \"commit.h\"\n #include \"date.h\"\n@@ -161,7 +162,8 @@ static int append_reachable_shallow_grafts(struct repository *r,\n static int pack_objects(struct repository *r,\n \t\t\tint fd, struct ref *refs, struct oid_array *advertised,\n \t\t\tstruct oid_array *negotiated,\n-\t\t\tstruct send_pack_args *args)\n+\t\t\tstruct send_pack_args *args,\n+\t\t\tint *excluded_boundary)\n {\n \tstruct odb_generate_pack_options opts = ODB_GENERATE_PACK_OPTIONS_INIT;\n \tstruct odb_pack_generator *generator;\n@@ -191,7 +193,8 @@ static int pack_objects(struct repository *r,\n \t/* Exclude reachable shallow boundaries from the pack. */\n \tif (is_repository_shallow(r) &&\n \t    get_exclude_boundary_mode(r) == EXCLUDE_BOUNDARY_YES)\n-\t\tappend_reachable_shallow_grafts(r, refs, advertised,\n+\t\t*excluded_boundary = append_reachable_shallow_grafts(\n+\t\t\t\t\t\tr, refs, advertised,\n \t\t\t\t\t\tnegotiated, args,\n \t\t\t\t\t\t&opts.haves);\n \n@@ -607,6 +610,8 @@ int send_pack(struct repository *r,\n \tint push_options_supported = 0;\n \tint object_format_supported = 0;\n \tunsigned cmds_sent = 0;\n+\tint excluded_boundary = 0;\n+\tint pack_contributing_refs = 0;\n \tint ret;\n \tstruct async demux;\n \tchar *push_cert_nonce = NULL;\n@@ -742,8 +747,10 @@ int send_pack(struct repository *r,\n \t\tdefault:\n \t\t\tcontinue;\n \t\t}\n-\t\tif (!ref->deletion)\n+\t\tif (!ref->deletion) {\n \t\t\tneed_pack_data = 1;\n+\t\t\tpack_contributing_refs++;\n+\t\t}\n \n \t\tif (args->dry_run || !status_report)\n \t\t\tref->status = REF_STATUS_OK;\n@@ -832,7 +839,8 @@ int send_pack(struct repository *r,\n \t\t\t   PACKET_READ_DIE_ON_ERR_PACKET);\n \n \tif (need_pack_data && cmds_sent) {\n-\t\tif (pack_objects(r, out, remote_refs, extra_have, &commons, args) < 0) {\n+\t\tif (pack_objects(r, out, remote_refs, extra_have, &commons, args,\n+\t\t\t\t &excluded_boundary) < 0) {\n \t\t\tif (args->stateless_rpc)\n \t\t\t\tclose(out);\n \t\t\tif (git_connection_is_socket(conn))\n@@ -878,6 +886,16 @@ int send_pack(struct repository *r,\n \t\t}\n \t}\n \n+\t/*\n+\t * Per-ref pushes prevent one ref's boundary from excluding objects\n+\t * needed by another.\n+\t */\n+\tif (ret < 0 && excluded_boundary && pack_contributing_refs > 1)\n+\t\tadvise_if_enabled(ADVICE_PUSH_SHALLOW_BOUNDARY,\n+\t\t\t_(\"A shallow boundary may have excluded objects needed by another ref.\\n\"\n+\t\t\t  \"Try pushing the refs one at a time, e.g.:\\n\"\n+\t\t\t  \"  git push <remote> <ref>\"));\n+\n \tif (ret < 0)\n \t\tgoto out;\n \ndiff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh\nindex e52f3e50e2..f2a84eb227 100755\n--- a/t/t5538-push-shallow.sh\n+++ b/t/t5538-push-shallow.sh\n@@ -343,4 +343,35 @@ test_expect_success 'push to a shallowUpdate receiver rejects a rootless snapsho\n \tgit --git-dir=seed-receiver.git rev-parse --verify seeded\n '\n \n+# Splitting a multi-ref push recomputes the pack and avoids exclusions from\n+# one ref stripping objects needed by another.\n+test_expect_success 'incomplete multi-ref shallow push advises pushing refs separately' '\n+\tgit init hint-origin &&\n+\tgit -C hint-origin checkout -b A &&\n+\ttest_commit -C hint-origin --no-tag has-shared sh shared &&\n+\ttest_commit -C hint-origin --no-tag A1 &&\n+\tgit -C hint-origin switch --orphan B &&\n+\ttest_commit -C hint-origin --no-tag B0 &&\n+\ttest_commit -C hint-origin --no-tag B1 &&\n+\n+\t# Strict checking rejects the incomplete pack before connectivity.\n+\tgit init --bare hint-receiver.git &&\n+\tgit --git-dir=hint-receiver.git config receive.fsckObjects true &&\n+\tgit -C hint-origin push \"file://$(pwd)/hint-receiver.git\" \\\n+\t\tB:refs/heads/B B:refs/heads/A &&\n+\n+\tgit clone --depth=1 --no-single-branch \\\n+\t\t\"file://$(pwd)/hint-origin\" hint-client &&\n+\n+\tgit -C hint-client checkout A &&\n+\ttest_commit -C hint-client --no-tag cX &&\n+\tgit -C hint-client checkout -b topic B &&\n+\ttest_commit -C hint-client --no-tag reintroduce sh shared &&\n+\n+\ttest_must_fail git -C hint-client \\\n+\t\t-c push.shallowExcludeBoundary=true \\\n+\t\tpush --force \"file://$(pwd)/hint-receiver.git\" A topic 2>err &&\n+\ttest_grep \"shallow boundary may have excluded objects\" err\n+'\n+\n test_done\n-- \ngitgitgadget\n"}]}