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

Re: [RFC PATCH 1/2] fetch: set-head with --set-head option

From
Bence Ferdinandy <bence@ferdinandy.com>
Date
Sep 11, 2024, 12:16 UTC
Message-ID
<D43G52W1ZPWU.2HPFFSQEM3WGV@ferdinandy.com>
In-Reply-To
<20240911065406.GC1538586@coredump.intra.peff.net>
On Wed Sep 11, 2024 at 08:54, Jeff King <peff@peff.net> wrote:
Show 32 quoted lines
> On Tue, Sep 10, 2024 at 10:24:58PM +0200, Bence Ferdinandy wrote:
>
> > When cloning a repository refs/remotes/origin/HEAD is set automatically.
> > In contrast, when using init, remote add and fetch to set a remote, one
> > needs to call remote set-head --auto to achieve the same result.
>
> Yes, I think this is a good goal, but...
>
> > diff --git a/builtin/fetch.c b/builtin/fetch.c
> > index b2b5aee5bf..6392314c6a 100644
> > --- a/builtin/fetch.c
> > +++ b/builtin/fetch.c
> > @@ -1961,8 +1961,19 @@ static int fetch_finished(int result, struct strbuf *out,
> >  	return 0;
> >  }
> >  
> > -static int fetch_multiple(struct string_list *list, int max_children,
> > -			  const struct fetch_config *config)
> > +static int run_set_head(const char *name)
> > +{
> > +	struct child_process cmd = CHILD_PROCESS_INIT;
> > +	strvec_push(&cmd.args, "remote");
> > +	strvec_push(&cmd.args, "set-head");
> > +	strvec_push(&cmd.args, "--auto");
> > +	strvec_push(&cmd.args, name);
> > +	cmd.git_cmd = 1;
> > +	return run_command(&cmd);
> > +}
>
> ...this is just calling "git remote" to do the real work. Which means
> that git-remote is going to make its own separate connection to the
> server (so slow, but may also require the user to reauthenticate, etc).

And indeed it does authenticate the user twice. I'll change this in a v3 (see the discussion on v2, I royally messed up CC address on this one :) ).

Show 5 quoted lines
>
> I think the intent of your patch 2 is that we'd only invoke this when we
> saw a change, which mitigates the impact, but it still seems somewhat
> hacky to me. We already have all of the information we need to do the
> update inside fetch itself.

It was more about not printing slightly misleading information, it did still always try to get the new information with --auto. With the changes mentioned in the other thread I'll also rework this a bit.

Best, Bence

Previous: Jeff KingNext: Bence Ferdinandy
Message 4 of 5 in “set remote/HEAD with fetch”
  1. 0/2 set remote/HEAD with fetchBence Ferdinandy, Sep 10, 2024
  2. 1/2 fetch: set-head with --set-head optionBence Ferdinandy, Sep 10, 2024
  3. Jeff KingSep 11, 2024
  4. Bence FerdinandySep 11, 2024
  5. 2/2 set-head: do not update if there is no changeBence Ferdinandy, Sep 10, 2024

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.