Merge branch 'jk/fetch-ref-prefix-cleanup' into jk/fetch-follow-remote-head-fix

* jk/fetch-ref-prefix-cleanup:
  fetch: use ref prefix list to skip ls-refs
  fetch: avoid ls-refs only to ask for HEAD symref update
  fetch: stop protecting additions to ref-prefix list
  fetch: ask server to advertise HEAD for config-less fetch
  refspec_ref_prefixes(): clean up refspec_item logic
  t5516: beef up exact-oid ref prefixes test
  t5516: drop NEEDSWORK about v2 reachability behavior
  t5516: prefer "oid" to "sha1" in some test titles
  t5702: fix typo in test name

Author: gitster

builtin/fetch.c

@@ -1718,7 +1718,6 @@ static int do_fetch(struct transport *transport,
 	const struct ref *remote_refs;
 	struct transport_ls_refs_options transport_ls_refs_options =
 		TRANSPORT_LS_REFS_OPTIONS_INIT;
-	int must_list_refs = 1;
 	struct fetch_head fetch_head = { 0 };
 	struct strbuf err = STRBUF_INIT;
 
@@ -1737,21 +1736,7 @@ static int do_fetch(struct transport *transport,
 	}
 
 	if (rs->nr) {
-		int i;
-
 		refspec_ref_prefixes(rs, &transport_ls_refs_options.ref_prefixes);
-
-		/*
-		 * We can avoid listing refs if all of them are exact
-		 * OIDs
-		 */
-		must_list_refs = 0;
-		for (i = 0; i < rs->nr; i++) {
-			if (!rs->items[i].exact_sha1) {
-				must_list_refs = 1;
-				break;
-			}
-		}
 	} else {
 		struct branch *branch = branch_get(NULL);
 
@@ -1766,23 +1751,30 @@ static int do_fetch(struct transport *transport,
 					    branch->merge[i]->src);
 			}
 		}
-	}
 
-	if (tags == TAGS_SET || tags == TAGS_DEFAULT) {
-		must_list_refs = 1;
-		if (transport_ls_refs_options.ref_prefixes.nr)
-			strvec_push(&transport_ls_refs_options.ref_prefixes,
-				    "refs/tags/");
-	}
-
-	if (uses_remote_tracking(transport, rs)) {
-		must_list_refs = 1;
-		if (transport_ls_refs_options.ref_prefixes.nr)
+		/*
+		 * If there are no refs specified to fetch, then we just
+		 * fetch HEAD; mention that to narrow the advertisement.
+		 */
+		if (!transport_ls_refs_options.ref_prefixes.nr)
 			strvec_push(&transport_ls_refs_options.ref_prefixes,
 				    "HEAD");
 	}
 
-	if (must_list_refs) {
+	if (tags == TAGS_SET || tags == TAGS_DEFAULT)
+		strvec_push(&transport_ls_refs_options.ref_prefixes,
+			    "refs/tags/");
+
+	if (transport_ls_refs_options.ref_prefixes.nr &&
+	    uses_remote_tracking(transport, rs))
+		strvec_push(&transport_ls_refs_options.ref_prefixes,
+			    "HEAD");
+
+	/*
+	 * Only initiate ref listing if we have at least one ref we want to
+	 * know about.
+	 */
+	if (transport_ls_refs_options.ref_prefixes.nr) {
 		trace2_region_enter("fetch", "remote_refs", the_repository);
 		remote_refs = transport_get_remote_refs(transport,
 							&transport_ls_refs_options);

refspec.c

@@ -246,14 +246,24 @@ void refspec_ref_prefixes(const struct refspec *rs,
 		const struct refspec_item *item = &rs->items[i];
 		const char *prefix = NULL;
 
-		if (item->exact_sha1 || item->negative)
+		if (item->negative)
 			continue;
-		if (rs->fetch == REFSPEC_FETCH)
-			prefix = item->src;
-		else if (item->dst)
-			prefix = item->dst;
-		else if (item->src && !item->exact_sha1)
+
+		if (rs->fetch == REFSPEC_FETCH) {
+			if (item->exact_sha1)
+				continue;
 			prefix = item->src;
+		} else {
+			/*
+			 * Pushes can have an explicit destination like
+			 * "foo:bar", or can implicitly use the src for both
+			 * ("foo" is the same as "foo:foo").
+			 */
+			if (item->dst)
+				prefix = item->dst;
+			else if (item->src && !item->exact_sha1)
+				prefix = item->src;
+		}
 
 		if (!prefix)
 			continue;

t/t5516-fetch-push.sh

@@ -495,7 +495,7 @@ test_expect_success 'push tag with non-existent, incomplete dest' '
 
 '
 
-test_expect_success 'push sha1 with non-existent, incomplete dest' '
+test_expect_success 'push oid with non-existent, incomplete dest' '
 
 	mk_test testrepo &&
 	test_must_fail git push testrepo $(git rev-parse main):foo
@@ -1251,7 +1251,7 @@ do
 	'
 done
 
-test_expect_success 'fetch exact SHA1' '
+test_expect_success 'fetch exact oid' '
 	mk_test testrepo heads/main hidden/one &&
 	git push testrepo main:refs/hidden/one &&
 	(
@@ -1297,7 +1297,7 @@ test_expect_success 'fetch exact SHA1' '
 	)
 '
 
-test_expect_success 'fetch exact SHA1 in protocol v2' '
+test_expect_success 'fetch exact oid in protocol v2' '
 	mk_test testrepo heads/main hidden/one &&
 	git push testrepo main:refs/hidden/one &&
 	git -C testrepo config transfer.hiderefs refs/hidden &&
@@ -1312,8 +1312,10 @@ test_expect_success 'fetch exact SHA1 in protocol v2' '
 	test_must_fail git -C child cat-file -t $the_commit &&
 
 	# fetching the hidden object succeeds by default
-	# NEEDSWORK: should this match the v0 behavior instead?
-	git -C child fetch -v ../testrepo $the_commit:refs/heads/copy
+	GIT_TRACE_PACKET=$PWD/trace.out \
+	git -C child fetch -v ../testrepo $the_commit:refs/heads/copy &&
+
+	test_grep ! "ref-prefix.*$the_commit" trace.out
 '
 
 for configallowtipsha1inwant in true false

t/t5702-protocol-v2.sh

@@ -665,7 +665,7 @@ test_expect_success 'even with handcrafted request, filter does not work if not
 	test-tool -C server serve-v2 --stateless-rpc <in >/dev/null
 '
 
-test_expect_success 'default refspec is used to filter ref when fetchcing' '
+test_expect_success 'default refspec is used to filter ref when fetching' '
 	test_when_finished "rm -f log" &&
 
 	GIT_TRACE_PACKET="$(pwd)/log" git -C file_child -c protocol.version=2 \
@@ -679,6 +679,48 @@ test_expect_success 'default refspec is used to filter ref when fetchcing' '
 	grep "ref-prefix refs/tags/" log
 '
 
+test_expect_success 'set up parent for prefix tests' '
+	git init prefix-parent &&
+	git -C prefix-parent commit --allow-empty -m foo &&
+	git -C prefix-parent tag my-tag &&
+	git -C prefix-parent branch unrelated-branch
+'
+
+test_expect_success 'empty refspec filters refs when fetching' '
+	git init configless-child &&
+
+	test_when_finished "rm -f log" &&
+	GIT_TRACE_PACKET="$(pwd)/log" \
+		git -C configless-child fetch ../prefix-parent &&
+	test_grep ! unrelated-branch log
+'
+
+test_expect_success 'exact oid fetch with tag following' '
+	git init exact-oid-tags &&
+
+	commit=$(git -C prefix-parent rev-parse --verify HEAD) &&
+
+	test_when_finished "rm -f log" &&
+	GIT_TRACE_PACKET="$(pwd)/log" \
+		git -C exact-oid-tags fetch ../prefix-parent \
+			$commit:refs/heads/exact &&
+	test_grep ! unrelated-branch log &&
+	git -C exact-oid-tags rev-parse --verify my-tag
+'
+
+test_expect_success 'exact oid fetch avoids pointless HEAD request' '
+	git init exact-oid-head &&
+	git -C exact-oid-head remote add origin ../prefix-parent &&
+
+	commit=$(git -C prefix-parent rev-parse --verify HEAD) &&
+
+	test_when_finished "rm -f log" &&
+	GIT_TRACE_PACKET="$(pwd)/log" \
+		git -C exact-oid-head fetch --no-tags origin \
+			$commit:refs/heads/exact &&
+	test_grep ! command=ls-refs log
+'
+
 test_expect_success 'fetch supports various ways of have lines' '
 	rm -rf server client trace &&
 	git init server &&