From: Harald Nordgren <haraldnordgren@gmail.com>
Date: Tue, 19 May 2026 07:52:34 GMT
Subject: Re: [PATCH v10] checkout: extend --track with a "fetch" mode to refresh start-point
Message-ID: <CAHwyqnUx=59MK5zKL0uuFXKrZ6PEc1j_2WT-_xtsGewVH3gBKQ@mail.gmail.com>
In-Reply-To: <xmqq8q9f9b5w.fsf@gitster.g>

> > +     strbuf_release(&dst);
> > +     *remote_out = remote_name;
> > +     return 0;
> > +}
> > +
> > +static void fetch_remote_for_start_point(const char *arg)
> > +{
> > +     char *remote_name = NULL;
> > +     char *src_ref = NULL;
> > +     char *existing_ref = NULL;
> > +     struct child_process cmd = CHILD_PROCESS_INIT;
> > +
> > +     if (resolve_fetch_target(arg, &remote_name, &src_ref, &existing_ref))
> > +             return;
> > +
> > +     strvec_pushl(&cmd.args, "fetch", remote_name, NULL);
> > +     if (src_ref)
> > +             strvec_push(&cmd.args, src_ref);
>
> What should happen with this configuration
>
>     [remote "origin"]
>         fetch = refs/heads/*:refs/upstream/*
>
> and the user says either of these two:
>
>     $ git checkout --track=fetch upstream
>     $ git checkout --track=fetch upstream/master
>
> We fail to find in "where does the remote name ends and branch name
> start?" loop that this request is about remote "origin" at all, no?
> We may see in the former case that there is
> refs/remotes/upstream/HEAD >that points at "master" in the same
> hierarchy, but the code thinks "upstream" is the remote name, which
> would mean you would "git fetch upstream", when the remote you need
> to fetch from is "origin".
>
> > +     cmd.git_cmd = 1;
> > +     if (run_command(&cmd)) {
> > +             if (existing_ref)
> > +                     warning(_("failed to fetch start-point '%s'; "
> > +                               "using existing '%s'"),
> > +                             arg, existing_ref);
> > +             else
> > +                     die(_("failed to fetch start-point '%s'"), arg);
>
> If we failed to set *existing_ref_out, shouldn't we fail without
> even attempting to call run_command() here, as we will have to die()
> anyway even if "git fetch" succeeds.  For that matter, it may be
> simpler and more correct for resolve_fetch_target() to fail (return
> -1) when it happens, by making the lat "if (rest) {...}" to have a
> corresponding "else { return -1 }" after it.

Yeah, good point. I will try to address this and send a new patch.


Harald