[PATCH v3 6/6] send-pack: advise splitting incomplete shallow pushes

["Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>]

From: Elijah Newren <newren@gmail.com>

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 <newren@gmail.com>
---
 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 81f80a9274..6bb6955246 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 `<remote>/<branch>` 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 63bf8b0c5f..3701672048 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 66f6cd6a77..b2e281baa5 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 8a7cedf65a..4fa17810a7 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 <remote> <ref>"));
+
 	if (ret < 0)
 		goto out;
 
diff --git a/t/t5538-push-shallow.sh b/t/t5538-push-shallow.sh
index e52f3e50e2..f2a84eb227 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
-- 
gitgitgadget