[PATCH] transport: report refs only if transport does

[Jonathan Tan <jonathantanmy@google.com>]

Commit 989b8c4452 ("fetch-pack: put shallow info in output parameter",
2018-06-28) allows transports to report the refs that they have fetched
in a new out-parameter "fetched_refs". If they do so,
transport_fetch_refs() makes this information available to its caller.

Because transport_fetch_refs() filters the refs sent to the transport,
it cannot just report the transport's result directly, but first needs
to readd the excluded refs, pretending that they are fetched. However,
this results in a wrong result if the transport did not report the refs
that they have fetched in "fetched_refs" - the excluded refs would be
added and reported, presenting an incomplete picture to the caller.

Instead, readd the excluded refs only if the transport reported fetched
refs.

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