[PATCH] transport: report refs only if transport does

From

JTJonathan Tan <jonathantanmy@google.com>

Date

Jul 30, 2018, 22:56 UTC

Message-ID

<20180730225601.107502-1-jonathantanmy@google.com>

In-Reply-To

<20180729121900.GA16770@sigill.intra.peff.net>

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
Thanks for the reproduction recipe, Peff. Here's a fix. It can be
reproduced with something using a remote helper's fetch command (and not
using "connect" or "stateless-connect"), fetching at least one ref that
requires a ref update and at least one that does not (as you can see
from the included test).
---
 t/t5551-http-fetch-smart.sh | 18 ++++++++++++++++++
 transport.c                 | 32 ++++++++++++++++++++++++--------
 2 files changed, 42 insertions(+), 8 deletions(-)

diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
index 913089b144..989d034acc 100755
--- a/t/t5551-http-fetch-smart.sh
+++ b/t/t5551-http-fetch-smart.sh
@@ -369,6 +369,24 @@ test_expect_success 'custom http headers' '
 		submodule update sub
 '
 
+test_expect_success 'using fetch command in remote-curl updates refs' '
+	SERVER="$HTTPD_DOCUMENT_ROOT_PATH/twobranch" &&
+	rm -rf "$SERVER" client &&
+
+	git init "$SERVER" &&
+	test_commit -C "$SERVER" foo &&
+	git -C "$SERVER" update-ref refs/heads/anotherbranch foo &&
+
+	git clone $HTTPD_URL/smart/twobranch client &&
+
+	test_commit -C "$SERVER" bar &&
+	git -C client -c protocol.version=0 fetch &&
+
+	git -C "$SERVER" rev-parse master >expect &&
+	git -C client rev-parse origin/master >actual &&
+	test_cmp expect actual
+'
+
 test_expect_success 'GIT_REDACT_COOKIES redacts cookies' '
 	rm -rf clone &&
 	echo "Set-Cookie: Foo=1" >cookies &&
diff --git a/transport.c b/transport.c
index fdd813f684..2a2415d79c 100644
--- a/transport.c
+++ b/transport.c
@@ -1230,17 +1230,18 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs,
 	struct ref **heads = NULL;
 	struct ref *nop_head = NULL, **nop_tail = &nop_head;
 	struct ref *rm;
+	struct ref *fetched_by_transport = NULL;
 
 	for (rm = refs; rm; rm = rm->next) {
 		nr_refs++;
 		if (rm->peer_ref &&
 		    !is_null_oid(&rm->old_oid) &&
 		    !oidcmp(&rm->peer_ref->old_oid, &rm->old_oid)) {
-			/*
-			 * These need to be reported as fetched, but we don't
-			 * actually need to fetch them.
-			 */
 			if (fetched_refs) {
+				/*
+				 * These may need to be reported as fetched,
+				 * but we don't actually need to fetch them.
+				 */
 				struct ref *nop_ref = copy_ref(rm);
 				*nop_tail = nop_ref;
 				nop_tail = &nop_ref->next;
@@ -1264,10 +1265,25 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs,
 			heads[nr_heads++] = rm;
 	}
 
-	rc = transport->vtable->fetch(transport, nr_heads, heads, fetched_refs);
-	if (fetched_refs && nop_head) {
-		*nop_tail = *fetched_refs;
-		*fetched_refs = nop_head;
+	rc = transport->vtable->fetch(transport, nr_heads, heads,
+				      fetched_refs ? &fetched_by_transport : NULL);
+	if (fetched_refs) {
+		if (fetched_by_transport) {
+			/*
+			 * The transport reported its fetched refs. Pretend
+			 * that we also fetched the ones that we didn't need to
+			 * fetch.
+			 */
+			*nop_tail = fetched_by_transport;
+			*fetched_refs = nop_head;
+		} else if (!fetched_by_transport) {
+			/*
+			 * The transport didn't report its fetched refs, so
+			 * this function will not report them either. We have
+			 * no use for nop_head.
+			 */
+			free_refs(nop_head);
+		}
 	}
 
 	free(heads);

-- 
2.18.0.345.g5c9ce644c3-goog