On Wed, Mar 26, 2025 at 05:46:00AM -0700, Junio C Hamano wrote:
> * jk/fetch-follow-remote-head-fix (2025-03-18) 3 commits
> - fetch: don't ask for remote HEAD if followRemoteHEAD is "never"
> - fetch: only respect followRemoteHEAD with configured refspecs
> - Merge branch 'jk/fetch-ref-prefix-cleanup' into jk/fetch-follow-remote-head-fix
> (this branch uses jk/fetch-ref-prefix-cleanup.)
>
> "git fetch [<remote>]" with only the configured fetch refspec
> should be the only thing to update refs/remotes/<remote>/HEAD,
> but the code was overly eager to do so in other cases.
>
> Will merge to 'next'?
> source: <20250318053905.GA2051217@xxxxxxxxxxxxxxxxxxxxxxx>
I think so. The design was based on our discussion, and it seemed to get
positive comments from you and Taylor. It might be nice to get an ack
from Bence, since this is his feature I'm modifying.
Taylor did note one place where the resulting code is a little hard to
read. That could be addressed by adding this on top (or it could be
squashed into patch 1):
-- >8 --
Subject: [PATCH] fetch: make set_head() call easier to read
We ignore any error returned from set_head(), but 638060dcb9 (fetch
set_head: refactor to use remote directly, 2025-01-26) left its call in
a noop "if" conditional as a sort of note-to-self.
When c834d1a7ce (fetch: only respect followRemoteHEAD with configured
refspecs, 2025-03-18) added a "do_set_head" flag, it was rolled into the
same conditional, putting set_head() on the right-hand side of a
short-circuit AND.
That's not wrong, but it really hides the point of the line, which
is (maybe) calling the function.
Instead, let's have a full if() block for the flag, and then our comment
(with some rewording) will be sufficient to clarify the error handling.
Signed-off-by: Jeff King <peff@xxxxxxxx>
---
builtin/fetch.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 3658509740..dbf741ef5b 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1903,12 +1903,13 @@ static int do_fetch(struct transport *transport,
"you need to specify exactly one branch with the --set-upstream option"));
}
}
- if (do_set_head && set_head(remote_refs, transport->remote))
- ;
+ if (do_set_head) {
/*
- * Way too many cases where this can go wrong
- * so let's just fail silently for now.
+ * Way too many cases where this can go wrong so let's just
+ * ignore errors and fail silently for now.
*/
+ set_head(remote_refs, transport->remote);
+ }
cleanup:
if (retcode) {
--
2.49.0.578.gaa93496c6a
