diff mbox series

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

Message ID 20240910203129.2251090-2-bence@ferdinandy.com (mailing list archive)
State Superseded
Headers show
Series set remote/HEAD with fetch | expand

Commit Message

Bence Ferdinandy Sept. 10, 2024, 8:24 p.m. UTC 
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.

Add a --set-head option to git fetch to automatically set heads on
remotes.

Signed-off-by: Bence Ferdinandy <bence@ferdinandy.com>
---
 builtin/fetch.c  | 29 ++++++++++++++++++++++++-----
 builtin/remote.c |  5 +++++
 2 files changed, 29 insertions(+), 5 deletions(-)

Comments

Jeff King Sept. 11, 2024, 6:54 a.m. UTC | #1 
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).

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.

-Peff
Bence Ferdinandy Sept. 11, 2024, 12:16 p.m. UTC | #2 
On Wed Sep 11, 2024 at 08:54, Jeff King <peff@peff.net> wrote:
> 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 :) ).

>
> 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
diff mbox series

Patch

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);
+}
+
+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;
 		}
 
 	strvec_clear(&argv);
@@ -2062,7 +2075,7 @@  static inline void fetch_one_setup_partial(struct remote *remote)
 }
 
 static int fetch_one(struct remote *remote, int argc, const char **argv,
-		     int prune_tags_ok, int use_stdin_refspecs,
+		     int prune_tags_ok, int set_head, int use_stdin_refspecs,
 		     const struct fetch_config *config)
 {
 	struct refspec rs = REFSPEC_INIT_FETCH;
@@ -2135,9 +2148,12 @@  static int fetch_one(struct remote *remote, int argc, const char **argv,
 	refspec_clear(&rs);
 	transport_disconnect(gtransport);
 	gtransport = NULL;
+	if (set_head && run_set_head(remote -> name))
+		exit_code = 1;
 	return exit_code;
 }
 
+
 int cmd_fetch(int argc, const char **argv, const char *prefix)
 {
 	struct fetch_config config = {
@@ -2154,6 +2170,7 @@  int cmd_fetch(int argc, const char **argv, const char *prefix)
 	struct string_list list = STRING_LIST_INIT_DUP;
 	struct remote *remote = NULL;
 	int all = -1, multiple = 0;
+	int set_head = 0;
 	int result = 0;
 	int prune_tags_ok = 1;
 	int enable_auto_gc = 1;
@@ -2171,6 +2188,8 @@  int cmd_fetch(int argc, const char **argv, const char *prefix)
 		OPT__VERBOSITY(&verbosity),
 		OPT_BOOL(0, "all", &all,
 			 N_("fetch from all remotes")),
+		OPT_BOOL(0, "set-head", &set_head,
+			 N_("auto set remote HEAD")),
 		OPT_BOOL(0, "set-upstream", &set_upstream,
 			 N_("set upstream for git pull/fetch")),
 		OPT_BOOL('a', "append", &append,
@@ -2436,7 +2455,7 @@  int cmd_fetch(int argc, const char **argv, const char *prefix)
 			trace2_region_leave("fetch", "setup-partial", the_repository);
 		}
 		trace2_region_enter("fetch", "fetch-one", the_repository);
-		result = fetch_one(remote, argc, argv, prune_tags_ok, stdin_refspecs,
+		result = fetch_one(remote, argc, argv, prune_tags_ok, set_head, stdin_refspecs,
 				   &config);
 		trace2_region_leave("fetch", "fetch-one", the_repository);
 	} else {
@@ -2459,7 +2478,7 @@  int cmd_fetch(int argc, const char **argv, const char *prefix)
 
 		/* TODO should this also die if we have a previous partial-clone? */
 		trace2_region_enter("fetch", "fetch-multiple", the_repository);
-		result = fetch_multiple(&list, max_children, &config);
+		result = fetch_multiple(&list, max_children, set_head, &config);
 		trace2_region_leave("fetch", "fetch-multiple", the_repository);
 	}
 
diff --git a/builtin/remote.c b/builtin/remote.c
index 0acc547d69..35c54dd103 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -1536,9 +1536,12 @@  static int get_remote_default(const char *key, const char *value UNUSED,
 static int update(int argc, const char **argv, const char *prefix)
 {
 	int i, prune = -1;
+	int set_head = 0;
 	struct option options[] = {
 		OPT_BOOL('p', "prune", &prune,
 			 N_("prune remotes after fetching")),
+		OPT_BOOL(0, "set-head", &set_head,
+			 N_("auto set remote HEAD")),
 		OPT_END()
 	};
 	struct child_process cmd = CHILD_PROCESS_INIT;
@@ -1552,6 +1555,8 @@  static int update(int argc, const char **argv, const char *prefix)
 
 	if (prune != -1)
 		strvec_push(&cmd.args, prune ? "--prune" : "--no-prune");
+	if (set_head)
+		strvec_push(&cmd.args, "--set-head");
 	if (verbose)
 		strvec_push(&cmd.args, "-v");
 	strvec_push(&cmd.args, "--multiple");