From: Jeff King <peff@peff•net>
To: Junio C Hamano <gitster@pobox•com>
Cc: Taylor Blau <me@ttaylorr•com>,
Bence Ferdinandy <bence@ferdinandy•com>,
git@vger•kernel.org
Subject: jk/fetch-follow-remote-head-fix, was Re: What's cooking in git.git (Mar 2025, #07; Wed, 26)
Date: Fri, 4 Apr 2025 04:58:12 -0400 [thread overview]
Message-ID: <20250404085812.GA772404@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqiknwhsdz.fsf@gitster.g>
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@coredump•intra.peff.net>
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@peff•net>
---
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
next prev parent reply other threads:[~2025-04-04 8:58 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-26 12:46 What's cooking in git.git (Mar 2025, #07; Wed, 26) Junio C Hamano
2025-03-26 17:19 ` Taylor Blau
2025-03-26 18:09 ` Jeff King
2025-03-29 0:34 ` D. Ben Knoble
2025-04-08 22:39 ` D. Ben Knoble
2025-04-08 22:49 ` Junio C Hamano
2025-04-04 8:58 ` Jeff King [this message]
2025-04-05 21:11 ` jk/fetch-follow-remote-head-fix, was " Bence Ferdinandy
2025-04-04 9:02 ` jk/zlib-inflate-fixes, " Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250404085812.GA772404@coredump.intra.peff.net \
--to=peff@peff$(echo .)net \
--cc=bence@ferdinandy$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=me@ttaylorr$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox