Hey Junio,
thanks for the very speedy feedback!
On Wed Sep 11, 2024 at 00:19, Junio C Hamano <gitster@xxxxxxxxx> wrote:
[snip]
>
> Ideally, because every "git fetch" you do will automatically learn
> what their HEAD of the day points at, even without "--set-head", it
> may be nice to let the user know when their HEAD changed, so that
> the user can inspect the situation and decide.
That would actually make sense, we could print a message saying HEAD has
changed and I guess helpfully print the exact set-head command they would need
to manually update should they wish to do so.
>
> If "fetch $repo", when it notices that refs/remotes/$repo/HEAD is
> missing, always unconditionally stores where their HEAD points at
> in refs/remotes/$repo/HEAD, and did nothing else, wouldn't that be
> sufficient?
>
> The users have "remote set-head" to do this when needed. What is
> the true motivation that "fetch" (which presumably happens a lot
> more often) needs to be involved in this process? The *only* upside
> I can see with "fetch --set-head" to blindly follow every switch of
> HEAD on the remote end is that you can keep up with the remote that
> flips its HEAD very often, but is that really a realistic need? If
> we reject such a use case as invalid, I suspect that the end-user
> experience would be simplified quite a lot. Imagine that we teach
> "git fetch $repo" to notice refs/remotes/$repo/HEAD is missing, and
> create it from the remote HEAD information automatically. And we do
> NOTHING ELSE. What would the end-user experience look like?
>
> * Usually, you start with "git clone" and 'origin' will know which
> branch 'origin/HEAD' points at.
>
> * You may run "git remote add -f $repo $URL" to add one. Because
> this runs "git fetch $repo", the previous addition to the "git
> fetch" will make sure refs/remotes/$repo/HEAD would be there.
>
> * You may run "git remote add $repo $URL" to add one, and then
> separately "git fetch $repo" yourself. The end result would be
> the same; refs/remotes/$repo/HEAD will be there.
I'm convinced :) It also does drop a lot of complexity. So there will be no
extra flag for fetch, rather:
- if the remote HEAD does not exist, we create it
- if it does exist, but has changed we print a message saying it was changed from X to Y and print the required command to update to this
- no configuration needed
The only place I can imagine this not being sufficient is in a CI/CD process
that is fetching into a cached repo needs to be updated, but then those people
can just always run remote set-head -a.
>
> Having said all that, the implementation here ...
>
> > +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);
> > +}
>
> ... does look quite straight-forward.
Unfortunately, as Jeff has pointed out in the other thread, this implementation
requires the user to authenticate twice ... Now, we could still rely on
set-head to actually do the writing, since if you explicitly give it what to
set to it will not do a network query, but considering all of the above,
I think it would make more sense not to do this, rather just bring in the
required logic here. This would also allow for the second patch to be a bit
more explicit on what did or did not happen during a remote set-head.
>
> > +static int fetch_multiple(struct string_list *list, int max_children, int set_head,
> > + const struct fetch_config *config)
> > {
> > int i, result = 0;
> > struct strvec argv = STRVEC_INIT;
> > @@ -2014,6 +2025,8 @@ static int fetch_multiple(struct string_list *list, int max_children,
> > error(_("could not fetch %s"), name);
> > result = 1;
> > }
> > + if (set_head && run_set_head(name))
> > + result = 1;
>
> This looked a bit questionable---I expected that we'd run the
> subfetches with "--set-head" option, but this makes us do the
> set-head step ourselves after the subfetches are done. What are
> pros-and-cons considered to reach the decision to do it this way?
Just noobish oversight, I figured out how to do it in the subfetch, but with
the above there will be no need for this anyway I guess.
I'll send a v2 soonish.
Best,
Bence
