From 6056689be039696d03dc67b8365300449b08676d Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 2 Sep 2026 17:45:34 -0700 Subject: [PATCH 1/6] unpack-objects: distinguish missing objects from type mismatches With receive.fsckObjects enabled, an incomplete pushed pack reports "object of unexpected type" when the expected object is simply absent. That suggests corruption rather than identifying the missing object. Use the same diagnostics as index-pack: report "did not receive expected object" when lookup fails, and reserve the type-mismatch message for an object that exists with the wrong type. Signed-off-by: Elijah Newren --- builtin/unpack-objects.c | 9 +++++++-- t/t5504-fetch-receive-strict.sh | 7 +++++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index 351948724ab703..ceefeb5a492791 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -233,8 +233,13 @@ static int check_object(struct object *obj, enum object_type type, if (!(obj->flags & FLAG_OPEN)) { size_t size; int type = odb_read_object_info(the_repository->objects, &obj->oid, &size); - if (type != obj->type || type <= 0) - die("object of unexpected type"); + if (type <= 0) + die(_("did not receive expected object %s"), + oid_to_hex(&obj->oid)); + if (type != obj->type) + die(_("object %s: expected type %s, found %s"), + oid_to_hex(&obj->oid), + type_name(obj->type), type_name(type)); obj->flags |= FLAG_WRITTEN; return 0; } diff --git a/t/t5504-fetch-receive-strict.sh b/t/t5504-fetch-receive-strict.sh index 75b2b87999a08d..0848e2da4a0195 100755 --- a/t/t5504-fetch-receive-strict.sh +++ b/t/t5504-fetch-receive-strict.sh @@ -105,8 +105,11 @@ test_expect_success 'push with receive.fsckobjects' ' To dst ! refs/heads/main:refs/heads/test [remote rejected] (unpacker error) EOF - test_must_fail git push --porcelain dst main:refs/heads/test >act && - test_cmp exp act + test_must_fail git push --porcelain dst main:refs/heads/test >act 2>err && + test_cmp exp act && + missing_oid=$(sed -e s%/%% S) && + test_grep "did not receive expected object $missing_oid" err && + test_grep ! "object of unexpected type" err ' test_expect_success 'push with transfer.fsckobjects' ' From 74a52a632e81e12a0b3fceebb50756c4fa434bb5 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 2 Sep 2026 17:45:34 -0700 Subject: [PATCH 2/6] receive-pack: avoid repeating connectivity errors receive-pack first checks all proposed ref tips together. If that bulk connectivity check fails, it checks each tip separately to identify which ref updates need "missing necessary objects". The bulk check already reports rev-list's diagnostic. The per-ref checks repeat it merely as a side effect of attributing the failure, potentially once for every broken ref. Silence their stderr while retaining their exit status and the per-ref rejection. Signed-off-by: Elijah Newren --- builtin/receive-pack.c | 7 +++++++ t/t5410-receive-pack.sh | 6 ++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index e6e54ba55f7a8c..8079901bb602b6 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1785,6 +1785,13 @@ static void set_connectivity_errors(struct command *commands, /* to be checked in update_shallow_ref() */ continue; + /* + * The bulk check already reported rev-list's diagnostics; + * this per-ref pass only attributes the failure, so keep it + * quiet rather than repeat those errors for every ref. + */ + opt.quiet = 1; + odb_transaction_env(transaction, &env); opt.env = env.v; diff --git a/t/t5410-receive-pack.sh b/t/t5410-receive-pack.sh index 09d6bfd2a10f5a..20d221044fdeaf 100755 --- a/t/t5410-receive-pack.sh +++ b/t/t5410-receive-pack.sh @@ -68,9 +68,11 @@ test_expect_success TEE_DOES_NOT_HANG \ # Replay captured git-send-pack(1) output on new empty repository. git init --bare remote.git && git receive-pack remote.git actual 2>err && + depacketize actual.raw && - test_grep "missing necessary objects" actual && - test_grep "fatal: Failed to traverse parents" err && + test_grep "missing necessary objects" actual.raw && + test_grep "fatal: Failed to traverse parents" actual.raw && + test_must_be_empty err && test_must_fail git -C remote.git cat-file -e $(git -C repo rev-parse HEAD) ' From fc21ecf8327722ed02b656a85e76b4a60371597b Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 2 Sep 2026 20:31:49 -0700 Subject: [PATCH 3/6] shallow: reject missing boundaries without disconnecting An incomplete shallow push can refer to a boundary commit the receiver does not have. remove_nonexistent_theirs_shallow() drops that graft, so paint_down() does not recognize it as a boundary and dies when parsing the missing commit. The client then sees only that the remote hung up. Treat an absent commit as the end of that traversal path rather than aborting receive-pack. This lets paint_down() process the remaining commits, after which the connectivity check rejects each affected ref with "missing necessary objects". A present commit that cannot be parsed still indicates corruption and remains fatal. Assisted-by: Claude Opus 4.8 & GPT-5.6 Sol Signed-off-by: Elijah Newren --- shallow.c | 16 +++++++++++--- t/t5538-push-shallow.sh | 46 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/shallow.c b/shallow.c index 8e244a5669ec1f..c6f743702268d7 100644 --- a/shallow.c +++ b/shallow.c @@ -659,9 +659,19 @@ static void paint_down(struct paint_info *info, const struct object_id *oid, if (c->object.flags & BOTTOM) continue; - if (repo_parse_commit(the_repository, c)) - die("unable to parse commit %s", - oid_to_hex(&c->object.oid)); + if (repo_parse_commit_gently(the_repository, c, 1)) { + /* + * remove_nonexistent_theirs_shallow() may have + * dropped a missing boundary, leaving it unmarked + * as BOTTOM. Let the connectivity check reject a + * missing commit, but still die on a corrupt one. + */ + if (odb_has_object(the_repository->objects, + &c->object.oid, 0)) + die("unable to parse commit %s", + oid_to_hex(&c->object.oid)); + continue; + } for (p = c->parents; p; p = p->next) { if (p->item->object.flags & SEEN) diff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh index afab456b327049..10ca7833d85dec 100755 --- a/t/t5538-push-shallow.sh +++ b/t/t5538-push-shallow.sh @@ -164,4 +164,50 @@ test_expect_success 'push new commit from shallow clone has good deltas' ' test_region pack-objects path-walk config-push.txt ' +test_expect_success 'incomplete shallow push rejects without disconnecting' ' + git init raw-origin && + git -C raw-origin checkout -b A && + test_commit -C raw-origin --no-tag has-shared sh shared && + test_commit -C raw-origin --no-tag A1 && + A1=$(git -C raw-origin rev-parse HEAD) && + git -C raw-origin switch --orphan B && + test_commit -C raw-origin --no-tag B0 && + test_commit -C raw-origin --no-tag B1 && + B1=$(git -C raw-origin rev-parse HEAD) && + + git init --bare raw-receiver.git && + git -C raw-receiver.git config receive.fsckObjects false && + git -C raw-origin push ../raw-receiver.git \ + B:refs/heads/B B:refs/heads/A && + + git -C raw-origin checkout A && + test_commit -C raw-origin --no-tag cX && + cX=$(git -C raw-origin rev-parse HEAD) && + git -C raw-origin checkout -b topic B && + test_commit -C raw-origin --no-tag reintroduce sh shared && + topic=$(git -C raw-origin rev-parse HEAD) && + + # Declare A1 and B1 as shallow, but omit them and their objects from + # the pack. This mimics an incomplete shallow push without relying on + # send-pack to create one. + { + printf "shallow %s\nshallow %s\n" "$A1" "$B1" | + packetize && + printf "%s %s refs/heads/A\0report-status object-format=%s\n" \ + "$B1" "$cX" "$(test_oid algo)" | + packetize_raw && + printf "%s %s refs/heads/topic\n" "$ZERO_OID" "$topic" | + packetize && + printf 0000 && + printf "%s\n%s\n^%s\n^%s\n" "$cX" "$topic" "$A1" "$B1" | + git -C raw-origin pack-objects --stdout --revs + } >input && + + git receive-pack raw-receiver.git out 2>err && + depacketize out.raw && + test_grep "ng refs/heads/A missing necessary objects" out.raw && + test_grep "ng refs/heads/topic missing necessary objects" out.raw && + test_grep ! "unable to parse commit" err +' + test_done From 7a4fb3845034fe50b83169d518b5d2459259a533 Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Sat, 5 Sep 2026 20:24:47 -0700 Subject: [PATCH 4/6] send-pack: optionally omit shallow boundaries When the receiver advertises no commit the shallow client has, pack generation walks to a shallow boundary and sends its entire tree. A tiny push can consequently transfer gigabytes of objects the receiver likely already has. The client already assumes the receiver has the boundary's parents, which are absent from the shallow clone. Extend that option to the boundary itself: push.shallowExcludeBoundary=true adds reachable shallow grafts as negative tips, letting receive-pack's connectivity check reject the push if the assumption is wrong. Only use grafts reached from refs contributing to the pack. An unrelated graft could otherwise exclude an object another ref needs. Stop at commits known to both sides, since they already bound the pack. Also accept "abort" to make no assumption, and "false" to retain the historical behavior required when seeding a receive.shallowUpdate receiver. Keep false as the default for now, so introducing the mechanism does not change existing pushes. Signed-off-by: Elijah Newren --- Documentation/config/push.adoc | 23 ++++++ send-pack.c | 122 ++++++++++++++++++++++++++++++ t/t5538-push-shallow.sh | 131 +++++++++++++++++++++++++++++++++ 3 files changed, 276 insertions(+) diff --git a/Documentation/config/push.adoc b/Documentation/config/push.adoc index 28132eedfee6c0..0ad55965e87e9f 100644 --- a/Documentation/config/push.adoc +++ b/Documentation/config/push.adoc @@ -134,6 +134,29 @@ This will result in only b (a and c are cleared). rely solely on the server's ref advertisement to find commits in common. +`push.shallowExcludeBoundary`:: + When pushing from a shallow repository, Git can omit the shallow + grafts' objects from the generated pack rather than resending the + full toplevel tree of those grafts. This assumes the receiver + already has those objects. If it does not, the receiver rejects + the push rather than accepting incomplete history. This setting + controls that behavior and accepts three values: ++ +-- +`abort`;; + If the push reaches such a boundary, refuse it rather than + choosing whether to send or omit it. +`true`;; + Omit the boundary objects (fast). If the receiver does not have + them, the push is rejected. +`false`;; + (the default) Send the boundary objects, retaining the historical + behavior. This can send the boundary's entire tree, which may be + very large. This is only needed when pushing to a receiver that + accepts new shallow roots (i.e. one with `receive.shallowUpdate` + enabled), which is very rare. +-- + `push.useBitmaps`:: If set to `false`, disable use of bitmaps for `git push` even if `pack.useBitmaps` is `true`, without preventing other git operations diff --git a/send-pack.c b/send-pack.c index f20460fbf487bf..386ea8b9a28f8a 100644 --- a/send-pack.c +++ b/send-pack.c @@ -14,6 +14,7 @@ #include "transport.h" #include "version.h" #include "oid-array.h" +#include "oidset.h" #include "gpg-interface.h" #include "shallow.h" #include "parse-options.h" @@ -55,6 +56,105 @@ static void append_negative_object(struct repository *r, oid_array_append(haves, oid); } +static int check_to_send_update(const struct ref *ref, + const struct send_pack_args *args); + +enum exclude_boundary_mode { + EXCLUDE_BOUNDARY_NONE = 0, + EXCLUDE_BOUNDARY_YES, + EXCLUDE_BOUNDARY_ABORT +}; + +static enum exclude_boundary_mode get_exclude_boundary_mode(struct repository *r) +{ + const char *value; + + if (repo_config_get_string_tmp(r, "push.shallowexcludeboundary", &value)) + return EXCLUDE_BOUNDARY_NONE; + + switch (git_parse_maybe_bool(value)) { + case 1: + return EXCLUDE_BOUNDARY_YES; + case 0: + return EXCLUDE_BOUNDARY_NONE; + default: + if (!strcasecmp(value, "abort")) + return EXCLUDE_BOUNDARY_ABORT; + die(_("bad push.shallowExcludeBoundary value: %s"), value); + } +} + +/* + * Append shallow grafts bounding contributing refs. Grafts from unrelated + * history could exclude objects this push needs, while commits both sides + * have make any graft below them irrelevant. + */ +static int append_reachable_shallow_grafts(struct repository *r, + const struct ref *refs, + const struct oid_array *advertised, + const struct oid_array *negotiated, + const struct send_pack_args *args, + struct oid_array *haves) +{ + struct commit_list *pending = NULL; + struct oidset seen = OIDSET_INIT; + struct oidset known = OIDSET_INIT; + const struct ref *ref; + int found = 0; + size_t i; + + for (i = 0; i < advertised->nr; i++) + oidset_insert(&known, &advertised->oid[i]); + for (i = 0; i < negotiated->nr; i++) + oidset_insert(&known, &negotiated->oid[i]); + + /* Populate "known" fully before starting the walk. */ + for (ref = refs; ref; ref = ref->next) { + struct commit *commit; + + if (!is_null_oid(&ref->old_oid)) + oidset_insert(&known, &ref->old_oid); + + if (is_null_oid(&ref->new_oid)) + continue; + if (check_to_send_update(ref, args)) + continue; + commit = lookup_commit_reference_gently(r, &ref->new_oid, 1); + if (commit) + commit_list_insert(commit, &pending); + } + + while (pending) { + struct commit *commit = pop_commit(&pending); + const struct object_id *oid = &commit->object.oid; + struct commit_graft *graft; + struct commit_list *parent; + + if (oidset_insert(&seen, oid)) + continue; + + if (oidset_contains(&known, oid) && + odb_has_object(r->objects, oid, 0)) + continue; + + graft = lookup_commit_graft(r, oid); + if (graft && graft->nr_parent == -1) { + append_negative_object(r, haves, oid); + found++; + continue; + } + + if (repo_parse_commit(r, commit)) + continue; + for (parent = commit->parents; parent; parent = parent->next) + commit_list_insert(parent->item, &pending); + } + + oidset_clear(&seen); + oidset_clear(&known); + return found; +} + /* * Make a pack stream and spit it out into file descriptor fd */ @@ -88,6 +188,13 @@ static int pack_objects(struct repository *r, for (size_t i = 0; i < negotiated->nr; i++) append_negative_object(r, &opts.haves, &negotiated->oid[i]); + /* Exclude reachable shallow boundaries from the pack. */ + if (is_repository_shallow(r) && + get_exclude_boundary_mode(r) == EXCLUDE_BOUNDARY_YES) + append_reachable_shallow_grafts(r, refs, advertised, + negotiated, args, + &opts.haves); + while (refs) { if (!is_null_oid(&refs->old_oid)) append_negative_object(r, &opts.haves, &refs->old_oid); @@ -644,6 +751,21 @@ int send_pack(struct repository *r, ref->status = REF_STATUS_EXPECTING_REPORT; } + /* Honor ABORT before sending any ref-update commands. */ + if (!args->dry_run && need_pack_data && is_repository_shallow(r) && + get_exclude_boundary_mode(r) == EXCLUDE_BOUNDARY_ABORT) { + struct oid_array probe = OID_ARRAY_INIT; + int reachable = append_reachable_shallow_grafts(r, remote_refs, + extra_have, + &commons, args, + &probe); + oid_array_clear(&probe); + if (reachable) + die(_("refusing to push a shallow boundary commit\n" + "Set push.shallowExcludeBoundary to true to omit it (fast),\n" + "or false to send it (needed for receive.shallowUpdate).")); + } + if (!args->dry_run) advertise_shallow_grafts_buf(r, &req_buf); diff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh index 10ca7833d85dec..67db51e60eca05 100755 --- a/t/t5538-push-shallow.sh +++ b/t/t5538-push-shallow.sh @@ -210,4 +210,135 @@ test_expect_success 'incomplete shallow push rejects without disconnecting' ' test_grep ! "unable to parse commit" err ' +test_expect_success 'shallow boundary exclusion avoids sending the full tree' ' + git init adv-origin && + # The shallow grafts are intentionally untagged so that no + # advertised ref points at them. + test_commit --no-tag -C adv-origin a && + test_commit --no-tag -C adv-origin b && + + git clone --depth=1 "file://$(pwd)/adv-origin" adv-client && + + # The remote branch advances past the history we have, so its + # advertised tip is something we cannot use as a negative tip; + # only the shallow graft lets us exclude the full tree. + test_commit --no-tag -C adv-origin c && + + git -C adv-client checkout -b topic && + test_commit --no-tag -C adv-client new && + GIT_PROGRESS_DELAY=0 git -C adv-client \ + -c push.shallowExcludeBoundary=true \ + push --progress origin topic 2>err && + + # Only the new commit, its tree, and the new blob are sent; sending + # the full tree is avoided by excluding the shallow graft. + test_grep "Enumerating objects: 4, done." err +' + +test_expect_success 'push.shallowExcludeBoundary=false sends full tree' ' + git init adv-origin2 && + test_commit --no-tag -C adv-origin2 a && + test_commit --no-tag -C adv-origin2 b && + + git clone --depth=1 "file://$(pwd)/adv-origin2" adv-client2 && + test_commit --no-tag -C adv-origin2 c && + + git -C adv-client2 checkout -b topic && + test_commit --no-tag -C adv-client2 new && + GIT_PROGRESS_DELAY=0 git -C adv-client2 \ + -c push.shallowExcludeBoundary=false \ + push --progress origin topic 2>err && + + # With the optimization disabled and no advertised ref pointing at + # the shallow graft, the full snapshot down to the shallow graft is + # resent, including its full tree. + test_grep "Enumerating objects: 7, done." err +' + +test_expect_success 'push.shallowExcludeBoundary=abort refuses when a graft is reached' ' + git init adv-origin3 && + test_commit --no-tag -C adv-origin3 a && + test_commit --no-tag -C adv-origin3 b && + + git clone --depth=1 "file://$(pwd)/adv-origin3" adv-client3 && + + # The remote branch advances past the history we have, so its + # advertised tip cannot bound the walk; only the shallow graft could, + # which is exactly what "abort" refuses to rely on. + test_commit --no-tag -C adv-origin3 c && + + git -C adv-client3 checkout -b topic && + test_commit --no-tag -C adv-client3 new && + + test_must_fail git -C adv-client3 \ + -c push.shallowExcludeBoundary=abort push origin topic 2>err && + test_grep "push.shallowExcludeBoundary" err && + + # The receiver must be left untouched: no ref was created. + test_must_fail git -C adv-origin3 rev-parse --verify refs/heads/topic +' + +# A and B are unrelated shallow histories. The receiver has B1 under both +# names, but lacks the "shared" blob from A1. The client adds cX atop A1 and +# reintroduces "shared" on a topic atop B1. Pushing A and topic together +# rejects A as a non-fast-forward, but A still participates in pack selection. +# Its A1 boundary must not exclude the blob needed by topic. +test_expect_success 'shallow push does not over-exclude for an accepted ref via a rejected one' ' + git init tworoot-origin && + git -C tworoot-origin checkout -b A && + test_commit -C tworoot-origin --no-tag has-shared sh shared && + test_commit -C tworoot-origin --no-tag A1 && + git -C tworoot-origin switch --orphan B && + test_commit -C tworoot-origin --no-tag B0 && + test_commit -C tworoot-origin --no-tag B1 && + + git init --bare tworoot-receiver.git && + git -C tworoot-origin push "file://$(pwd)/tworoot-receiver.git" \ + B:refs/heads/B B:refs/heads/A && + + git clone --depth=1 --no-single-branch \ + "file://$(pwd)/tworoot-origin" tworoot-client && + + git -C tworoot-client checkout A && + test_commit -C tworoot-client --no-tag cX && + + git -C tworoot-client checkout -b topic B && + test_commit -C tworoot-client --no-tag reintroduce sh shared && + + test_must_fail git -C tworoot-client \ + -c push.shallowExcludeBoundary=true push \ + "file://$(pwd)/tworoot-receiver.git" A topic && + git --git-dir=tworoot-receiver.git rev-parse --verify topic +' + +# A receive.shallowUpdate receiver needs the boundary snapshot to adopt a new +# shallow root, so omission must reject rather than create a broken ref. +test_expect_success 'push to a shallowUpdate receiver rejects a rootless snapshot' ' + git init seed-origin && + test_commit -C seed-origin s1 && + test_commit -C seed-origin s2 && + test_commit -C seed-origin s3 && + + # depth-2: a shallow graft at s2, pushing s3 on top of it + git clone --depth=2 "file://$(pwd)/seed-origin" seed-client && + + git init --bare seed-receiver.git && + git --git-dir=seed-receiver.git config receive.shallowUpdate true && + + # Optimization on: the s2 boundary snapshot is withheld, so the + # receiver cannot graft the new root and rejects the push, leaving the + # ref uncreated. + test_must_fail git -C seed-client \ + -c push.shallowExcludeBoundary=true push \ + "file://$(pwd)/seed-receiver.git" HEAD:refs/heads/seeded 2>err && + test_grep "remote rejected" err && + test_must_fail git --git-dir=seed-receiver.git rev-parse --verify seeded && + + # Opt-out: the full snapshot is sent, so the same push now succeeds and + # the new shallow root is grafted. + git -C seed-client -c push.shallowExcludeBoundary=false push \ + "file://$(pwd)/seed-receiver.git" HEAD:refs/heads/seeded && + git --git-dir=seed-receiver.git rev-parse --verify seeded +' + test_done From afa44c6d2262dda7d04ba243fdd47563d997561d Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Sat, 5 Sep 2026 20:25:09 -0700 Subject: [PATCH 5/6] send-pack: default to excluding shallow boundaries Sending a shallow boundary is almost always wasted work. We got the shallow boundary from somewhere, and most likely that is the server we are pushing to. If the receiver has the boundary, omitting it avoids transferring and recompressing its entire tree. If the receiver lacks both it and its history, the push is rejected either way, but omission reaches that answer without first sending the tree. Make push.shallowExcludeBoundary default to true. This also covers cases where push negotiation is disabled, unavailable, or fails to find the boundary, so users do not need special configuration to avoid unexpectedly huge pushes. The practical compatibility cost is the rare use of push to seed a new shallow root. That already requires receive.shallowUpdate on the server; it now also requires push.shallowExcludeBoundary=false on the client so the receiver gets the boundary snapshot. Two other edge cases instead fail faster with the new default: (A) A receiver has the boundary's parents but not the boundary itself. This likely means the user is pushing to the wrong receiver, where a quick rejection is preferable to a slow accidental success. (B) In a multi-ref push, one ref's shallow boundary can exclude objects needed by another ref. This may reject more refs than necessary, but retrying the refs separately avoids the problem; the next patch advises users to do so. Neither case justifies making every ordinary shallow push send the boundary's potentially enormous tree. Signed-off-by: Elijah Newren --- Documentation/config/push.adoc | 10 +++++----- send-pack.c | 2 +- t/t5538-push-shallow.sh | 10 ++++++---- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/Documentation/config/push.adoc b/Documentation/config/push.adoc index 0ad55965e87e9f..a08ec04c21cc62 100644 --- a/Documentation/config/push.adoc +++ b/Documentation/config/push.adoc @@ -147,12 +147,12 @@ This will result in only b (a and c are cleared). If the push reaches such a boundary, refuse it rather than choosing whether to send or omit it. `true`;; - Omit the boundary objects (fast). If the receiver does not have - them, the push is rejected. + (the default) Omit the boundary objects (fast). If the receiver + does not have them, the push is rejected. `false`;; - (the default) Send the boundary objects, retaining the historical - behavior. This can send the boundary's entire tree, which may be - very large. This is only needed when pushing to a receiver that + Send the boundary objects, retaining the historical behavior. + This can send the boundary's entire tree, which may be very + large. This is only needed when pushing to a receiver that accepts new shallow roots (i.e. one with `receive.shallowUpdate` enabled), which is very rare. -- diff --git a/send-pack.c b/send-pack.c index 386ea8b9a28f8a..8a7cedf65a38ef 100644 --- a/send-pack.c +++ b/send-pack.c @@ -70,7 +70,7 @@ static enum exclude_boundary_mode get_exclude_boundary_mode(struct repository *r const char *value; if (repo_config_get_string_tmp(r, "push.shallowexcludeboundary", &value)) - return EXCLUDE_BOUNDARY_NONE; + return EXCLUDE_BOUNDARY_YES; switch (git_parse_maybe_bool(value)) { case 1: diff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh index 67db51e60eca05..e52f3e50e21017 100755 --- a/t/t5538-push-shallow.sh +++ b/t/t5538-push-shallow.sh @@ -64,7 +64,8 @@ EOF test_expect_success 'push from shallow clone, with grafted roots' ' ( cd shallow2 && - test_must_fail git push ../.git +main:refs/remotes/shallow2/main 2>err && + test_must_fail git -c push.shallowExcludeBoundary=false \ + push ../.git +main:refs/remotes/shallow2/main 2>err && test_grep "shallow2/main.*shallow update not allowed" err ) && test_must_fail git rev-parse shallow2/main && @@ -75,7 +76,8 @@ test_expect_success 'add new shallow root with receive.updateshallow on' ' test_config receive.shallowupdate true && ( cd shallow2 && - git push ../.git +main:refs/remotes/shallow2/main + git -c push.shallowExcludeBoundary=false \ + push ../.git +main:refs/remotes/shallow2/main ) && git log --format=%s shallow2/main >actual && git fsck && @@ -90,7 +92,8 @@ test_expect_success 'push from shallow to shallow' ' ( cd shallow && git --git-dir=../shallow2/.git config receive.shallowupdate true && - git push ../shallow2/.git +main:refs/remotes/shallow/main && + git -c push.shallowExcludeBoundary=false \ + push ../shallow2/.git +main:refs/remotes/shallow/main && git --git-dir=../shallow2/.git config receive.shallowupdate false ) && ( @@ -227,7 +230,6 @@ test_expect_success 'shallow boundary exclusion avoids sending the full tree' ' git -C adv-client checkout -b topic && test_commit --no-tag -C adv-client new && GIT_PROGRESS_DELAY=0 git -C adv-client \ - -c push.shallowExcludeBoundary=true \ push --progress origin topic 2>err && # Only the new commit, its tree, and the new blob are sent; sending From ae821ce0784286486fe76117b90bce78610ea37f Mon Sep 17 00:00:00 2001 From: Elijah Newren Date: Wed, 2 Sep 2026 19:58:37 -0700 Subject: [PATCH 6/6] send-pack: advise splitting incomplete shallow pushes When several refs share a pack, an omitted shallow boundary reached from one ref can exclude an object needed by another. Pushing each ref separately recomputes the pack and avoids that interaction. When such a multi-ref push fails after excluding a boundary, suggest separate pushes. Gate the message on advice.pushShallowBoundary. Assisted-by: Claude Opus 4.8 Signed-off-by: Elijah Newren --- Documentation/config/advice.adoc | 5 +++++ advice.c | 1 + advice.h | 1 + send-pack.c | 26 ++++++++++++++++++++++---- t/t5538-push-shallow.sh | 31 +++++++++++++++++++++++++++++++ 5 files changed, 60 insertions(+), 4 deletions(-) diff --git a/Documentation/config/advice.adoc b/Documentation/config/advice.adoc index 81f80a92745123..6bb69552468738 100644 --- a/Documentation/config/advice.adoc +++ b/Documentation/config/advice.adoc @@ -99,6 +99,11 @@ all advice messages. a configured remote but looks like a `/` ref, suggesting that the remote and branch be given as separate arguments. + pushShallowBoundary:: + Shown when a push from a shallow clone is rejected because + the remote could not unpack the pack, hinting that a shallow + boundary may have omitted objects and suggesting the refs be + pushed one at a time. pushUnqualifiedRefname:: Shown when linkgit:git-push[1] gives up trying to guess based on the source and destination refs what diff --git a/advice.c b/advice.c index 63bf8b0c5f0481..3701672048df8d 100644 --- a/advice.c +++ b/advice.c @@ -70,6 +70,7 @@ static struct { [ADVICE_PUSH_NON_FF_MATCHING] = { "pushNonFFMatching" }, [ADVICE_PUSH_REF_NEEDS_UPDATE] = { "pushRefNeedsUpdate" }, [ADVICE_PUSH_REPO_LOOKS_LIKE_REF] = { "pushRepoLooksLikeRef" }, + [ADVICE_PUSH_SHALLOW_BOUNDARY] = { "pushShallowBoundary" }, [ADVICE_PUSH_UNQUALIFIED_REF_NAME] = { "pushUnqualifiedRefName" }, [ADVICE_PUSH_UPDATE_REJECTED] = { "pushUpdateRejected" }, [ADVICE_PUSH_UPDATE_REJECTED_ALIAS] = { "pushNonFastForward" }, /* backwards compatibility */ diff --git a/advice.h b/advice.h index 66f6cd6a772d8c..b2e281baa5b84c 100644 --- a/advice.h +++ b/advice.h @@ -37,6 +37,7 @@ enum advice_type { ADVICE_PUSH_NON_FF_MATCHING, ADVICE_PUSH_REF_NEEDS_UPDATE, ADVICE_PUSH_REPO_LOOKS_LIKE_REF, + ADVICE_PUSH_SHALLOW_BOUNDARY, ADVICE_PUSH_UNQUALIFIED_REF_NAME, ADVICE_PUSH_UPDATE_REJECTED, ADVICE_PUSH_UPDATE_REJECTED_ALIAS, diff --git a/send-pack.c b/send-pack.c index 8a7cedf65a38ef..4fa17810a7952c 100644 --- a/send-pack.c +++ b/send-pack.c @@ -1,4 +1,5 @@ #include "git-compat-util.h" +#include "advice.h" #include "config.h" #include "commit.h" #include "date.h" @@ -161,7 +162,8 @@ static int append_reachable_shallow_grafts(struct repository *r, static int pack_objects(struct repository *r, int fd, struct ref *refs, struct oid_array *advertised, struct oid_array *negotiated, - struct send_pack_args *args) + struct send_pack_args *args, + int *excluded_boundary) { struct odb_generate_pack_options opts = ODB_GENERATE_PACK_OPTIONS_INIT; struct odb_pack_generator *generator; @@ -191,7 +193,8 @@ static int pack_objects(struct repository *r, /* Exclude reachable shallow boundaries from the pack. */ if (is_repository_shallow(r) && get_exclude_boundary_mode(r) == EXCLUDE_BOUNDARY_YES) - append_reachable_shallow_grafts(r, refs, advertised, + *excluded_boundary = append_reachable_shallow_grafts( + r, refs, advertised, negotiated, args, &opts.haves); @@ -607,6 +610,8 @@ int send_pack(struct repository *r, int push_options_supported = 0; int object_format_supported = 0; unsigned cmds_sent = 0; + int excluded_boundary = 0; + int pack_contributing_refs = 0; int ret; struct async demux; char *push_cert_nonce = NULL; @@ -742,8 +747,10 @@ int send_pack(struct repository *r, default: continue; } - if (!ref->deletion) + if (!ref->deletion) { need_pack_data = 1; + pack_contributing_refs++; + } if (args->dry_run || !status_report) ref->status = REF_STATUS_OK; @@ -832,7 +839,8 @@ int send_pack(struct repository *r, PACKET_READ_DIE_ON_ERR_PACKET); if (need_pack_data && cmds_sent) { - if (pack_objects(r, out, remote_refs, extra_have, &commons, args) < 0) { + if (pack_objects(r, out, remote_refs, extra_have, &commons, args, + &excluded_boundary) < 0) { if (args->stateless_rpc) close(out); if (git_connection_is_socket(conn)) @@ -878,6 +886,16 @@ int send_pack(struct repository *r, } } + /* + * Per-ref pushes prevent one ref's boundary from excluding objects + * needed by another. + */ + if (ret < 0 && excluded_boundary && pack_contributing_refs > 1) + advise_if_enabled(ADVICE_PUSH_SHALLOW_BOUNDARY, + _("A shallow boundary may have excluded objects needed by another ref.\n" + "Try pushing the refs one at a time, e.g.:\n" + " git push ")); + if (ret < 0) goto out; diff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh index e52f3e50e21017..f2a84eb2271353 100755 --- a/t/t5538-push-shallow.sh +++ b/t/t5538-push-shallow.sh @@ -343,4 +343,35 @@ test_expect_success 'push to a shallowUpdate receiver rejects a rootless snapsho git --git-dir=seed-receiver.git rev-parse --verify seeded ' +# Splitting a multi-ref push recomputes the pack and avoids exclusions from +# one ref stripping objects needed by another. +test_expect_success 'incomplete multi-ref shallow push advises pushing refs separately' ' + git init hint-origin && + git -C hint-origin checkout -b A && + test_commit -C hint-origin --no-tag has-shared sh shared && + test_commit -C hint-origin --no-tag A1 && + git -C hint-origin switch --orphan B && + test_commit -C hint-origin --no-tag B0 && + test_commit -C hint-origin --no-tag B1 && + + # Strict checking rejects the incomplete pack before connectivity. + git init --bare hint-receiver.git && + git --git-dir=hint-receiver.git config receive.fsckObjects true && + git -C hint-origin push "file://$(pwd)/hint-receiver.git" \ + B:refs/heads/B B:refs/heads/A && + + git clone --depth=1 --no-single-branch \ + "file://$(pwd)/hint-origin" hint-client && + + git -C hint-client checkout A && + test_commit -C hint-client --no-tag cX && + git -C hint-client checkout -b topic B && + test_commit -C hint-client --no-tag reintroduce sh shared && + + test_must_fail git -C hint-client \ + -c push.shallowExcludeBoundary=true \ + push --force "file://$(pwd)/hint-receiver.git" A topic 2>err && + test_grep "shallow boundary may have excluded objects" err +' + test_done