threads / rfc / 21537

RFC patch, 4 partsRe-implement 'git remote update' using 'git fetch'

Subject: [RFC/PATCH 4/4] Re-implement 'git remote update' using 'git fetch'

## tl;dr

5 messages between Nov 8, 2009 and Nov 9, 2009. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Björn Gustavsson· Nov 8, 2009, 15:48 UTC · lore

In order to not duplicate functionality, re-implement 'git remote update' in terms of 'git fetch'.

There is one incompatibility: the skipDefaultUpdate configuration variable will only be honored if remotes.default is not set (i.e. when 'git remote update' will invoke 'git fetch --all', not when it will invoke 'git fetch default').

Signed-off-by: Björn Gustavsson <bgustavsson@gmail.com>
---
 builtin-remote.c |   86 ++++++++++++++++-------------------------------------
 1 files changed, 26 insertions(+), 60 deletions(-)
Show changes to builtin-remote.c +26 −60
diff --git a/builtin-remote.c b/builtin-remote.c
index 0777dd7..fd7e0b2 100644
--- a/builtin-remote.c
+++ b/builtin-remote.c
@@ -1189,88 +1189,54 @@ static int prune_remote(const char *remote, int dry_run)
 	return result;
 }
 
-static int get_one_remote_for_update(struct remote *remote, void *priv)
+static int get_remote_default(const char *key, const char *value, void *priv)
 {
-	struct string_list *list = priv;
-	if (!remote->skip_default_update)
-		string_list_append(remote->name, list);
-	return 0;
-}
-
-static struct remote_group {
-	const char *name;
-	struct string_list *list;
-} remote_group;
-
-static int get_remote_group(const char *key, const char *value, void *num_hits)
-{
-	if (!prefixcmp(key, "remotes.") &&
-			!strcmp(key + 8, remote_group.name)) {
-		/* split list by white space */
-		int space = strcspn(value, " \t\n");
-		while (*value) {
-			if (space > 1) {
-				string_list_append(xstrndup(value, space),
-						remote_group.list);
-				++*((int *)num_hits);
-			}
-			value += space + (value[space] != '\0');
-			space = strcspn(value, " \t\n");
-		}
+	if (strcmp(key, "remotes.default") == 0) {
+		int *found = priv;
+		*found = 1;
 	}
-
 	return 0;
 }
 
 static int update(int argc, const char **argv)
 {
-	int i, result = 0, prune = 0;
-	struct string_list list = { NULL, 0, 0, 0 };
-	static const char *default_argv[] = { NULL, "default", NULL };
+	int i, prune = 0;
 	struct option options[] = {
 		OPT_GROUP("update specific options"),
 		OPT_BOOLEAN('p', "prune", &prune,
 			    "prune remotes after fetching"),
 		OPT_END()
 	};
+	const char **fetch_argv;
+	int fetch_argc = 0;
+	int default_defined = 0;
+
+	fetch_argv = xmalloc(sizeof(char *) * (argc+4));
 
 	argc = parse_options(argc, argv, NULL, options, builtin_remote_usage,
 			     PARSE_OPT_KEEP_ARGV0);
-	if (argc < 2) {
-		argc = 2;
-		argv = default_argv;
-	}
 
-	remote_group.list = &list;
-	for (i = 1; i < argc; i++) {
-		int groups_found = 0;
-		remote_group.name = argv[i];
-		result = git_config(get_remote_group, &groups_found);
-		if (!groups_found && (i != 1 || strcmp(argv[1], "default"))) {
-			struct remote *remote;
-			if (!remote_is_configured(argv[i]))
-				die("No such remote or remote group: %s",
-				    argv[i]);
-			remote = remote_get(argv[i]);
-			string_list_append(remote->name, remote_group.list);
-		}
-	}
+	fetch_argv[fetch_argc++] = "fetch";
 
-	if (!result && !list.nr  && argc == 2 && !strcmp(argv[1], "default"))
-		result = for_each_remote(get_one_remote_for_update, &list);
+	if (verbose)
+		fetch_argv[fetch_argc++] = "-v";
+	if (argc < 2) {
+		fetch_argv[fetch_argc++] = "default";
+	} else {
+		fetch_argv[fetch_argc++] = "--multiple";
+		for (i = 1; i < argc; i++)
+			fetch_argv[fetch_argc++] = argv[i];
+	}
 
-	for (i = 0; i < list.nr; i++) {
-		int err = fetch_remote(list.items[i].string);
-		result |= err;
-		if (!err && prune)
-			result |= prune_remote(list.items[i].string, 0);
+	if (strcmp(fetch_argv[fetch_argc-1], "default") == 0) {
+		git_config(get_remote_default, &default_defined);
+		if (!default_defined)
+			fetch_argv[fetch_argc-1] = "--all";
 	}
 
-	/* all names were strdup()ed or strndup()ed */
-	list.strdup_strings = 1;
-	string_list_clear(&list, 0);
+	fetch_argv[fetch_argc] = NULL;
 
-	return result;
+	return run_command_v_opt(fetch_argv, RUN_GIT_CMD);
 }
 
 static int get_one_entry(struct remote *remote, void *priv)
-- 
1.6.5.1.69.g36942
Paolo Bonzini· Nov 8, 2009, 21:10 UTC · re: Björn Gustavsson · lore

Re: [RFC/PATCH 4/4] Re-implement 'git remote update' using 'git fetch'

On 11/08/2009 04:48 PM, Björn Gustavsson wrote:
Show 7 quoted lines
> In order to not duplicate functionality, re-implement 'git remote
> update' in terms of 'git fetch'.
>
> There is one incompatibility: the skipDefaultUpdate configuration
> variable will only be honored if remotes.default is not set (i.e.
> when 'git remote update' will invoke 'git fetch --all', not when
> it will invoke 'git fetch default').

Rather than introducing this incompatibility, I'd rather see 'git remote update' deprecated (so that the code will one day go away for good) and, for now, leave duplicated functionality for the incompatible case.

It would be great if the deprecation message for "git remote update" spewed out commands to convert the old configuration to what is needed for "git fetch --all" to work in the same way.

Paolo
Björn Gustavsson· Nov 8, 2009, 22:23 UTC · re: Paolo Bonzini · lore

Re: [RFC/PATCH 4/4] Re-implement 'git remote update' using 'git fetch'

On Sun, Nov 8, 2009 at 10:10 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
Show 9 quoted lines
> On 11/08/2009 04:48 PM, Björn Gustavsson wrote:
>> There is one incompatibility: the skipDefaultUpdate configuration
>> variable will only be honored if remotes.default is not set (i.e.
>> when 'git remote update' will invoke 'git fetch --all', not when
>> it will invoke 'git fetch default').
>
> Rather than introducing this incompatibility, I'd rather see 'git remote
> update' deprecated (so that the code will one day go away for good) and, for
> now, leave duplicated functionality for the incompatible case.

I didn't check my facts properly before writing that commit message. Looking again at the old code in builtin_remote.c (after a long break), and actually testing the behavior, I see that there is no incompatibility.

Both the old and the new code use skipDefaultUpdate only if remotes.default is not set. If remotes.default is set, that list of remotes will be used. Only if remotes.default is not set, will 'git remote update' go through the list of all remotes and filter away those that have skipDefaultUpdate set to true.

Sorry for the confusion. I will correct the commit message in my re-roll of the patch.

-- 
Björn Gustavsson, Erlang/OTP, Ericsson AB
Jay Soffian· Nov 9, 2009, 00:43 UTC · re: Björn Gustavsson · lore

Re: [RFC/PATCH 4/4] Re-implement 'git remote update' using 'git fetch'

2009/11/8 Björn Gustavsson <bgustavsson@gmail.com>:
> In order to not duplicate functionality, re-implement 'git remote
> update' in terms of 'git fetch'.

Junio, I guess I'll wait for this to hit pu and then rebase my fetch --prune changes on top of it?

j.
Björn Gustavsson· Nov 9, 2009, 06:37 UTC · re: Jay Soffian · lore

Re: [RFC/PATCH 4/4] Re-implement 'git remote update' using 'git fetch'

2009/11/9 Jay Soffian <jaysoffian@gmail.com>:
Show 6 quoted lines
> 2009/11/8 Björn Gustavsson <bgustavsson@gmail.com>:
>> In order to not duplicate functionality, re-implement 'git remote
>> update' in terms of 'git fetch'.
>
> Junio, I guess I'll wait for this to hit pu and then rebase my fetch
> --prune changes on top of it?
It has hit pu now.

If you'll rebase and finish your patch series, I can base my final patch in my series on your changes, because that patch will need a fetch that supports --prune in order to support 'git remote update --prune'.

-- 
Björn Gustavsson, Erlang/OTP, Ericsson AB

← back to recent threads