Thread (16 messages) 16 messages, 4 authors, 2024-09-30

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

From: Bence Ferdinandy <hidden>
Date: 2024-09-11 12:19:37

Possibly related (same subject, not in this thread)

Hey Junio,

thanks for the very speedy feedback!

On Wed Sep 11, 2024 at 00:19, Junio C Hamano [off-list ref] 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 ...
quoted
+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.
quoted
+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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help