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

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

[Björn Gustavsson]

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(-)

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

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

[Paolo Bonzini]

On 11/08/2009 04:48 PM, Björn Gustavsson wrote:
> 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

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

[Björn Gustavsson]

On Sun, Nov 8, 2009 at 10:10 PM, Paolo Bonzini <bonzini@gnu.org> wrote:
> 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

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

[Jay Soffian]

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.

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

[Björn Gustavsson]

2009/11/9 Jay Soffian <jaysoffian@gmail.com>:
> 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