fetch-pack.c: do not declare local commits as "have" in partial repos

In a partial repository, creating a local commit and then fetching
causes the following state to occur:

commit  tree  blob
 C3 ---- T3 -- B3 (fetched from remote, in promisor pack)
 |
 C2 ---- T2 -- B2 (created locally, in non-promisor pack)
 |
 C1 ---- T1 -- B1 (fetched from remote, in promisor pack)

During garbage collection, parents of promisor objects are marked as
UNINTERESTING and are subsequently garbage collected. In this case, C2
would be deleted and attempts to access that commit would result in "bad
object" errors (originally reported here[1]).

This is not a bug in gc since it should be the case that parents of
promisor objects are also promisor objects (fsck assumes this as
well). When promisor objects are fetched, the state of the repository
should ensure that the above holds true. Therefore, do not declare local
commits as "have" in partial repositores so they can be fetched into a
promisor pack.

[1] https://lore.kernel.org/git/[email protected]/

Helped-by: Jonathan Tan <[email protected]>
Signed-off-by: Calvin Wan <[email protected]>
Signed-off-by: Junio C Hamano <[email protected]>

Author: calvin-wan-google

fetch-pack.c

@@ -1295,12 +1295,23 @@ static void add_common(struct strbuf *req_buf, struct oidset *common)
 
 static int add_haves(struct fetch_negotiator *negotiator,
 		     struct strbuf *req_buf,
-		     int *haves_to_send)
+		     int *haves_to_send,
+		     int from_promisor)
 {
 	int haves_added = 0;
 	const struct object_id *oid;
 
 	while ((oid = negotiator->next(negotiator))) {
+		/*
+		 * In partial repos, do not declare local objects as "have"
+		 * so that they can be fetched into a promisor pack. Certain
+		 * operations mark parent commits of promisor objects as
+		 * UNINTERESTING and are subsequently garbage collected so
+		 * this ensures local commits are still available in promisor
+		 * packs after a fetch + gc.
+		 */
+		if (from_promisor && !is_in_promisor_pack(oid, 0))
+			continue;
 		packet_buf_write(req_buf, "have %s\n", oid_to_hex(oid));
 		if (++haves_added >= *haves_to_send)
 			break;
@@ -1403,7 +1414,7 @@ static int send_fetch_request(struct fetch_negotiator *negotiator, int fd_out,
 	/* Add all of the common commits we've found in previous rounds */
 	add_common(&req_buf, common);
 
-	haves_added = add_haves(negotiator, &req_buf, haves_to_send);
+	haves_added = add_haves(negotiator, &req_buf, haves_to_send, args->from_promisor);
 	*in_vain += haves_added;
 	trace2_data_intmax("negotiation_v2", the_repository, "haves_added", haves_added);
 	trace2_data_intmax("negotiation_v2", the_repository, "in_vain", *in_vain);
@@ -2176,7 +2187,7 @@ void negotiate_using_fetch(const struct oid_array *negotiation_tips,
 
 		packet_buf_write(&req_buf, "wait-for-done");
 
-		haves_added = add_haves(&negotiator, &req_buf, &haves_to_send);
+		haves_added = add_haves(&negotiator, &req_buf, &haves_to_send, 0);
 		in_vain += haves_added;
 		if (!haves_added || (seen_ack && in_vain >= MAX_IN_VAIN))
 			last_iteration = 1;

t/t5616-partial-clone.sh

@@ -693,6 +693,35 @@ test_expect_success 'lazy-fetch in submodule succeeds' '
 	git -C client restore --recurse-submodules --source=HEAD^ :/
 '
 
+test_expect_success 'fetching from promisor remote fetches previously local commits' '
+	# Setup
+	git init full &&
+	git -C full config uploadpack.allowfilter 1 &&
+	git -C full config uploadpack.allowanysha1inwant 1 &&
+	touch full/foo &&
+	git -C full add foo &&
+	git -C full commit -m "commit 1" &&
+	git -C full checkout --detach &&
+
+	# Partial clone and push commit to remote
+	git clone "file://$(pwd)/full" --filter=blob:none partial &&
+	echo "hello" > partial/foo &&
+	git -C partial commit -a -m "commit 2" &&
+	git -C partial push &&
+
+	# gc in partial repo
+	git -C partial gc --prune=now &&
+
+	# Create another commit in normal repo
+	git -C full checkout main &&
+	echo " world" >> full/foo &&
+	git -C full commit -a -m "commit 3" &&
+
+	# Pull from remote in partial repo, and run gc again
+	git -C partial pull &&
+	git -C partial gc --prune=now
+'
+
 . "$TEST_DIRECTORY"/lib-httpd.sh
 start_httpd