send-pack: avoid sending the whole tree when pushing from a shallow clone

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

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`;;
+	(the default) Omit the boundary objects (fast). If the receiver
+	does not have them, the push is rejected.
+`false`;;
+	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

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 */

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,

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;
 

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;
 	}

send-pack.c

@@ -1,4 +1,5 @@
 #include "git-compat-util.h"
+#include "advice.h"
 #include "config.h"
 #include "commit.h"
 #include "date.h"
@@ -14,6 +15,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,13 +57,113 @@ 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_YES;
+
+	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
  */
 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;
@@ -88,6 +190,14 @@ 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)
+		*excluded_boundary = 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);
@@ -500,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;
@@ -635,15 +747,32 @@ 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;
 		else
 			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);
 
@@ -710,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))
@@ -756,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;
 

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)

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 <out >actual 2>err &&
+	depacketize <actual >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)
 '
 

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' '