git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] fetch.c: defer fetch.followRemoteHEAD validation

From
Colin Hinton <colinlewishinton@gmail.com>
Date
Sep 25, 2026, 18:48 UTC
Message-ID
<CAHeTm9M9c7D71QUA-Dy9o_YD2PGbNYRa9LnNWKrYSaHEoUtpSQ@mail.gmail.com>
In-Reply-To
<DLNDRU6GIB30.1F5G8Z3JIR67W@lfurio.us>
On Thu, Sep 24, 2026 at 12:50 AM Matt Hunter <m@lfurio.us> wrote:
Show 27 quoted lines
>
> On Wed Sep 23, 2026 at 1:03 AM EDT, Colin Hinton wrote:
> >> > @@ -1962,6 +1953,14 @@ static int do_fetch(struct transport *transport,
> >> >               if (transport->remote->fetch.nr) {
> >> >                       refspec_ref_prefixes(&transport->remote->fetch,
> >> >                                            &transport_ls_refs_options.ref_prefixes);
> >> > +
> >> > +                     if (transport->remote->follow_remote_head)
> >> > +                             follow_remote_head = transport->remote->follow_remote_head;
> >>
> >> The code assumes that remote.*.followRemoteHEAD has been pre-parsed.
> >> Doesn't the code to do so in remote.c::handle_config() share exactly
> >> the same problem as you are fixing here?
> >>
> > I agree that the same problem that is being addressed here is present
> > in remote.c as well. The only difference being, that there is no
> > return call in the followremotehead block in remote.c,
>
> I'm not exactly sure why the config parsing in remote.c doesn't end with
> a fallback 'return git_default_config(...)', though the followremotehead
> case piggybacking the common 'return 0' at the end should be no problem.
>
> > and it at most only throws a warning if no valid value is present.
>
> which _was_ the case for fetch.followRemoteHEAD as well.  So, we should
> keep the two in sync right?
>

To respond to both of your emails, I agree that the two should be kept in sync, as Junio pointed out, a valueless followremotehead will currently result in a die, yet I agree with your point from your [1] that it would be more sensible to warn, and treat a valueless or bogus followremotehead as FOLLOW_REMOTE_NEVER. For this patch, I will keep the behavior similar, but for a follow-on patch and with some approval I agree with this change.

Show 29 quoted lines
> > I think this
> > should be addressed, but I am uncertain if this is within the scope of
> > this issue and should be resolved now, or if this requires its own
> > investigation and should be resolved in a future patch. Regardless I
> > am eager to work on it, but would like some guidance as to what is
> > most appropriate for a change in remote.c.
>
> I spent some time drafting up what changes to remote.c could look like,
> based on your work so far.  This follow-up patch also has extra changes
> to builtin/fetch.c to accommodate the same allowed functionality as
> before.  There are two awkward bits to this patch as-is, though:
>
> builtin/remote.c::set_head()
>
> 012bc566bad7 (remote set-head: set followRemoteHEAD to "warn" if "always")
> added this behavior to overrule a remote's "always" setting if the user
> ever modified their HEAD manually.  So, this file needs to know about the
> followRemoteHEAD values, but parsing into the enums is currently confined
> to fetch.c.  This just adds another bit of string parsing.
>
> builtin/fetch.c::get_follow_remote_head()
>
> is updated to serve double-duty for both the fetch and remote configs,
> and needs a better warning message if a bad value is detected.  Perhaps
> add another parameter to the function?
>
> With this patch below, it's arguable whether the enum definition for the
> followRemoteHEAD values now better fits in fetch.c instead of remote.h.
>

Thank you very much for this. I think this is a great starting point for a follow up patch.

-Colin Hinton
Previous: Matt HunterNext: Matt Hunter
Message 5 of 22 in “fetch.c: defer fetch.followRemoteHEAD validation”
  1. fetch.c: defer fetch.followRemoteHEAD validationColin Hinton, Sep 22, 2026
  2. Junio C HamanoSep 22, 2026
  3. Colin HintonSep 23, 2026
  4. Matt HunterSep 24, 2026
  5. Colin HintonSep 25, 2026
  6. Matt HunterSep 24, 2026
  7. fetch.c: defer fetch.followRemoteHEAD validationColin Hinton, Sep 25, 2026
  8. Junio C HamanoSep 25, 2026
  9. Colin HintonSep 25, 2026
  10. Matt HunterSep 30, 2026
  11. Junio C HamanoSep 30, 2026
  12. Colin HintonOct 3, 2026
  13. fetch.c: defer fetch.followRemoteHEAD validationColin Hinton, Sep 25, 2026
  14. fetch.c: defer fetch.followRemoteHEAD validationColin Hinton, Oct 3, 2026
  15. Junio C HamanoOct 4, 2026
  16. Colin HintonOct 4, 2026
  17. Junio C HamanoOct 4, 2026
  18. fetch.c: defer fetch.followRemoteHEAD validationColin Hinton, Oct 4, 2026
  19. Matt HunterOct 5, 2026
  20. Junio C HamanoOct 5, 2026
  21. fetch.c: defer fetch.followRemoteHEAD validationColin Hinton, Oct 6, 2026
  22. Junio C HamanoOct 7, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.