[PATCH 8/9] fetch: avoid ls-refs only to ask for HEAD symref update

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



When we fetch from a configured remote, we may try to update the local
refs/remotes/<origin>/HEAD, and so we ask the server to advertise its
HEAD to us.

But if we aren't otherwise asking about any refs at all, then we know
this HEAD update can never happen! To consider a new value for HEAD,
the set_head() function uses guess_remote_head(). And even if it sees an
explicit symref value for HEAD, it will only report that as a match if
we also saw that remote ref advertised, and it mapped to a local
tracking ref via get_fetch_map().

In other words, a fetch like this:

  git fetch origin $exact_oid:refs/heads/foo

can never update HEAD, because we will never have fetched (nor even see
the advertisement for) the ref that HEAD points to.

Currently the command above will still call ls-refs to ask about the
HEAD, even though it is pointless. This patch teaches it to skip the
ls-refs call entirely in this case, which avoids a round-trip to the
server.

Signed-off-by: Jeff King <peff@xxxxxxxx>
---
This describes the current behavior of set_head(). But I do wonder if it
should be more aggressive. If we found out that the other side is
pointing to refs/heads/foo, should we map that ourselves to find that it
would be stored as refs/remotes/origin/foo, and update HEAD anyway?

That would let us actually update HEAD in this case (and also in many
other cases where we fetch a specific ref that is not pointed to by
the remote HEAD).

OTOH, it would mean we basically always do an ls-refs just to find out
about HEAD. Which is working against the optimization from e70a3030e7
(fetch: do not list refs if fetching only hashes, 2018-09-27), and the
code before this patch might even be considered a regression.

I also wonder if the uses_remote_tracking() check added by 6c915c3f85
(fetch: do not ask for HEAD unnecessarily, 2024-12-06) is not quite
right. It is looking for a configured remote along with at least one
refspec with a destination. But wouldn't it need to have both a source
ref (not an exact oid) and a destination to work? I suspect we could do
the fix there and end up with the same optimized behavior as I have
here.

 builtin/fetch.c        |  5 ++---
 t/t5702-protocol-v2.sh | 13 +++++++++++++
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/builtin/fetch.c b/builtin/fetch.c
index 6ab101fa6d..c26866e674 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1782,11 +1782,10 @@ static int do_fetch(struct transport *transport,
 			    "refs/tags/");
 	}
 
-	if (uses_remote_tracking(transport, rs)) {
-		must_list_refs = 1;
+	if (must_list_refs &&
+	    uses_remote_tracking(transport, rs))
 		strvec_push(&transport_ls_refs_options.ref_prefixes,
 			    "HEAD");
-	}
 
 	if (must_list_refs) {
 		trace2_region_enter("fetch", "remote_refs", the_repository);
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index 626deb05f0..4d0cbe9872 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -708,6 +708,19 @@ test_expect_success 'exact oid fetch with tag following' '
 	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 &&
-- 
2.49.0.rc1.381.gc60f5426ff





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux