[PATCH] fetch: add configuration for making --all default

[PATCH] fetch: add configuration for making --all default

[Øystein Walle]

Fetching from all remotes by default is useful if you're working on a
repo with few and/or fast remotes. It also lets you fetch from origin
even if the current branch's upstream is elsewhere without specifying it
explicitly.

Signed-off-by: Øystein Walle <oystwa@gmail.com>
---
This is scratching a itch I have. Most of the time I just want to fetch
everything, wherever it may be, and fetch's behavour of using the current
upstream's remote sometimes doesn't read my mind as well as I'd like it to.
It's not particularly useful. But more "destructive" behaviours (--prune) can
be made default, so think this could be as well.

 Documentation/config.txt        |  5 +++++
 Documentation/fetch-options.txt |  4 +++-
 Documentation/git-fetch.txt     |  3 ++-
 builtin/fetch.c                 | 21 ++++++++++++++++-----
 t/t5514-fetch-multiple.sh       | 23 +++++++++++++++++++++++
 5 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3e37b93..c40654f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1170,6 +1170,11 @@ fetch.prune::
 	If true, fetch will automatically behave as if the `--prune`
 	option was given on the command line.  See also `remote.<name>.prune`.
 
+fetch.all::
+	If true, fetch will automatically behave as if the `--all`
+	option was given on the command line uness a remote was given. The
+	default is false.
+
 format.attach::
 	Enable multipart/mixed attachments as the default for
 	'format-patch'.  The value can also be a double quoted string
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 45583d8..aa95a30 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -1,5 +1,7 @@
 --all::
-	Fetch all remotes.
+	Fetch all remotes. This can be configured to be the default behaviour
+	when no remotes are given explicitly. See the `fetch.all` configuration
+	variable in linkgit:git-config[1].
 
 -a::
 --append::
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index e62d9a0..584f3fb 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -36,7 +36,8 @@ there is a remotes.<group> entry in the configuration file.
 (See linkgit:git-config[1]).
 
 When no remote is specified, by default the `origin` remote will be used,
-unless there's an upstream branch configured for the current branch.
+unless there's an upstream branch configured for the current branch, or the
+`fetch.all` configuration variable is set to true.
 
 The names of refs that are fetched, together with the object names
 they point at, are written to `.git/FETCH_HEAD`.  This information
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8d5b2db..715ea82 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -30,10 +30,12 @@ enum {
 };
 
 static int fetch_prune_config = -1; /* unspecified */
+static int fetch_all_config = -1; /* unspecified */
 static int prune = -1; /* unspecified */
+static int all = -1; /* unspecified */
 #define PRUNE_BY_DEFAULT 0 /* do we prune by default? */
 
-static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;
+static int append, dry_run, force, keep, multiple, update_head_ok, verbosity;
 static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static int tags = TAGS_DEFAULT, unshallow, update_shallow;
 static const char *depth;
@@ -67,6 +69,10 @@ static int git_fetch_config(const char *k, const char *v, void *cb)
 		fetch_prune_config = git_config_bool(k, v);
 		return 0;
 	}
+	if (!strcmp(k, "fetch.all")) {
+		fetch_all_config = git_config_bool(k, v);
+		return 0;
+	}
 	return git_default_config(k, v, cb);
 }
 
@@ -1168,7 +1174,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		git_config(submodule_config, NULL);
 	}
 
-	if (all) {
+	if (all == 1) {
 		if (argc == 1)
 			die(_("fetch --all does not take a repository argument"));
 		else if (argc > 1)
@@ -1176,9 +1182,14 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		(void) for_each_remote(get_one_remote_for_fetch, &list);
 		result = fetch_multiple(&list);
 	} else if (argc == 0) {
-		/* No arguments -- use default remote */
-		remote = remote_get(NULL);
-		result = fetch_one(remote, argc, argv);
+		if (fetch_all_config && all != 0) {
+			(void) for_each_remote(get_one_remote_for_fetch, &list);
+			result = fetch_multiple(&list);
+		} else {
+			/* No arguments and no --all -- use default remote */
+			remote = remote_get(NULL);
+			result = fetch_one(remote, argc, argv);
+		}
 	} else if (multiple) {
 		/* All arguments are assumed to be remotes or groups */
 		for (i = 0; i < argc; i++)
diff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh
index 4b4b667..4e773ee 100755
--- a/t/t5514-fetch-multiple.sh
+++ b/t/t5514-fetch-multiple.sh
@@ -55,6 +55,18 @@ test_expect_success 'git fetch --all' '
 	 test_cmp expect output)
 '
 
+test_expect_success 'git fetch (fetch.all = true)' '
+	(git clone one test9 &&
+	 cd test9 &&
+	 git config fetch.all true &&
+	 git remote add one ../one &&
+	 git remote add two ../two &&
+	 git remote add three ../three &&
+	 git fetch &&
+	 git branch -r > output &&
+	 test_cmp ../test/expect output)
+'
+
 test_expect_success 'git fetch --all should continue if a remote has errors' '
 	(git clone one test2 &&
 	 cd test2 &&
@@ -91,6 +103,17 @@ test_expect_success 'git fetch --multiple (but only one remote)' '
 	 test_cmp ../expect output)
 '
 
+test_expect_success 'git fetch one (fetch.all = true)' '
+	(cd test3 &&
+	 git config fetch.all true &&
+	 git fetch three &&
+	 git branch -r > output &&
+	 test_cmp ../expect output &&
+	 git fetch --no-all &&
+	 git branch -r > output &&
+	 test_cmp ../expect output)
+'
+
 cat > expect << EOF
   one/master
   one/side
-- 
2.2.0

Re: [PATCH] fetch: add configuration for making --all default

[Remi Galan Alfonso]

Hi,

Øystein Walle <oystwa@gmail.com> writes:
> +fetch.all::
> +        If true, fetch will automatically behave as if the `--all`
> +        option was given on the command line uness a remote was given. The
> +        default is false.

s/uness/unless

> +test_expect_success 'git fetch (fetch.all = true)' '
> +        (git clone one test9 &&
> +         cd test9 &&
> +         git config fetch.all true &&
> +         git remote add one ../one &&
> +         git remote add two ../two &&
> +         git remote add three ../three &&
> +         git fetch &&
> +         git branch -r > output &&

No space after redirection ('>').
It should be:
         git branch -r >output &&

> +test_expect_success 'git fetch one (fetch.all = true)' '
> +        (cd test3 &&
> +         git config fetch.all true &&
> +         git fetch three &&
> +         git branch -r > output &&

Same here

> +         test_cmp ../expect output &&
> +         git fetch --no-all &&
> +         git branch -r > output &&

And here.

> +         test_cmp ../expect output)
> +'

Thanks,
Rémi

[PATCH v2] fetch: add configuration for making --all default

[Øystein Walle]

Fetching from all remotes by default is useful if you're working on a
repo with few and/or fast remotes. It also lets you fetch from origin
even if the current branch's upstream is elsewhere without specifying it
explicitly.

Signed-off-by: Øystein Walle <oystwa@gmail.com>
---
Thanks for the quick feedback, Remi. There's a fixed version.

 Documentation/config.txt        |  5 +++++
 Documentation/fetch-options.txt |  4 +++-
 Documentation/git-fetch.txt     |  3 ++-
 builtin/fetch.c                 | 21 ++++++++++++++++-----
 t/t5514-fetch-multiple.sh       | 23 +++++++++++++++++++++++
 5 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 3e37b93..997a8d9 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1170,6 +1170,11 @@ fetch.prune::
 	If true, fetch will automatically behave as if the `--prune`
 	option was given on the command line.  See also `remote.<name>.prune`.
 
+fetch.all::
+	If true, fetch will automatically behave as if the `--all`
+	option was given on the command line unless a remote was given. The
+	default is false.
+
 format.attach::
 	Enable multipart/mixed attachments as the default for
 	'format-patch'.  The value can also be a double quoted string
diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt
index 45583d8..aa95a30 100644
--- a/Documentation/fetch-options.txt
+++ b/Documentation/fetch-options.txt
@@ -1,5 +1,7 @@
 --all::
-	Fetch all remotes.
+	Fetch all remotes. This can be configured to be the default behaviour
+	when no remotes are given explicitly. See the `fetch.all` configuration
+	variable in linkgit:git-config[1].
 
 -a::
 --append::
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index e62d9a0..584f3fb 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -36,7 +36,8 @@ there is a remotes.<group> entry in the configuration file.
 (See linkgit:git-config[1]).
 
 When no remote is specified, by default the `origin` remote will be used,
-unless there's an upstream branch configured for the current branch.
+unless there's an upstream branch configured for the current branch, or the
+`fetch.all` configuration variable is set to true.
 
 The names of refs that are fetched, together with the object names
 they point at, are written to `.git/FETCH_HEAD`.  This information
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 8d5b2db..715ea82 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -30,10 +30,12 @@ enum {
 };
 
 static int fetch_prune_config = -1; /* unspecified */
+static int fetch_all_config = -1; /* unspecified */
 static int prune = -1; /* unspecified */
+static int all = -1; /* unspecified */
 #define PRUNE_BY_DEFAULT 0 /* do we prune by default? */
 
-static int all, append, dry_run, force, keep, multiple, update_head_ok, verbosity;
+static int append, dry_run, force, keep, multiple, update_head_ok, verbosity;
 static int progress = -1, recurse_submodules = RECURSE_SUBMODULES_DEFAULT;
 static int tags = TAGS_DEFAULT, unshallow, update_shallow;
 static const char *depth;
@@ -67,6 +69,10 @@ static int git_fetch_config(const char *k, const char *v, void *cb)
 		fetch_prune_config = git_config_bool(k, v);
 		return 0;
 	}
+	if (!strcmp(k, "fetch.all")) {
+		fetch_all_config = git_config_bool(k, v);
+		return 0;
+	}
 	return git_default_config(k, v, cb);
 }
 
@@ -1168,7 +1174,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		git_config(submodule_config, NULL);
 	}
 
-	if (all) {
+	if (all == 1) {
 		if (argc == 1)
 			die(_("fetch --all does not take a repository argument"));
 		else if (argc > 1)
@@ -1176,9 +1182,14 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 		(void) for_each_remote(get_one_remote_for_fetch, &list);
 		result = fetch_multiple(&list);
 	} else if (argc == 0) {
-		/* No arguments -- use default remote */
-		remote = remote_get(NULL);
-		result = fetch_one(remote, argc, argv);
+		if (fetch_all_config && all != 0) {
+			(void) for_each_remote(get_one_remote_for_fetch, &list);
+			result = fetch_multiple(&list);
+		} else {
+			/* No arguments and no --all -- use default remote */
+			remote = remote_get(NULL);
+			result = fetch_one(remote, argc, argv);
+		}
 	} else if (multiple) {
 		/* All arguments are assumed to be remotes or groups */
 		for (i = 0; i < argc; i++)
diff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh
index 4b4b667..e0fb744 100755
--- a/t/t5514-fetch-multiple.sh
+++ b/t/t5514-fetch-multiple.sh
@@ -55,6 +55,18 @@ test_expect_success 'git fetch --all' '
 	 test_cmp expect output)
 '
 
+test_expect_success 'git fetch (fetch.all = true)' '
+	(git clone one test9 &&
+	 cd test9 &&
+	 git config fetch.all true &&
+	 git remote add one ../one &&
+	 git remote add two ../two &&
+	 git remote add three ../three &&
+	 git fetch &&
+	 git branch -r >output &&
+	 test_cmp ../test/expect output)
+'
+
 test_expect_success 'git fetch --all should continue if a remote has errors' '
 	(git clone one test2 &&
 	 cd test2 &&
@@ -91,6 +103,17 @@ test_expect_success 'git fetch --multiple (but only one remote)' '
 	 test_cmp ../expect output)
 '
 
+test_expect_success 'git fetch one (fetch.all = true)' '
+	(cd test3 &&
+	 git config fetch.all true &&
+	 git fetch three &&
+	 git branch -r >output &&
+	 test_cmp ../expect output &&
+	 git fetch --no-all &&
+	 git branch -r >output &&
+	 test_cmp ../expect output)
+'
+
 cat > expect << EOF
   one/master
   one/side
-- 
2.2.0

Re: [PATCH] fetch: add configuration for making --all default

[Junio C Hamano]

Øystein Walle <oystwa@gmail.com> writes:

> Fetching from all remotes by default is useful if you're working on a
> repo with few and/or fast remotes.

That part is sensible.

> It also lets you fetch from origin
> even if the current branch's upstream is elsewhere without specifying it
> explicitly.

I do not think this description is necessary or even beneficial.
The users can already do that with 'fetch --all', so there is
no value added by having a new configuration.

The only thing a new configuration gives the users is a convenience
of not having to say `--all` from the command line.

> diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
> index e62d9a0..584f3fb 100644
> --- a/Documentation/git-fetch.txt
> +++ b/Documentation/git-fetch.txt
> @@ -36,7 +36,8 @@ there is a remotes.<group> entry in the configuration file.
>  (See linkgit:git-config[1]).
>  
>  When no remote is specified, by default the `origin` remote will be used,
> -unless there's an upstream branch configured for the current branch.
> +unless there's an upstream branch configured for the current branch, or the
> +`fetch.all` configuration variable is set to true.

This sounds a bit strange.  Other places talk about fetch.all being
the same as giving `--all`, but neither the original description nor
the resulting text with this patch says anything about what would
happen 'when no remote is given but the --all option is given', which
makes it sound as if fetch.all and `--all` behave differently here.

> diff --git a/builtin/fetch.c b/builtin/fetch.c
> ...
> @@ -1168,7 +1174,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
>  		git_config(submodule_config, NULL);
>  	}
>  
> -	if (all) {
> +	if (all == 1) {
>  		if (argc == 1)
>  			die(_("fetch --all does not take a repository argument"));
>  		else if (argc > 1)
> @@ -1176,9 +1182,14 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
>  		(void) for_each_remote(get_one_remote_for_fetch, &list);
>  		result = fetch_multiple(&list);
>  	} else if (argc == 0) {
> -		/* No arguments -- use default remote */
> -		remote = remote_get(NULL);
> -		result = fetch_one(remote, argc, argv);
> +		if (fetch_all_config && all != 0) {
> +			(void) for_each_remote(get_one_remote_for_fetch, &list);
> +			result = fetch_multiple(&list);

Can't you rearrange if/else cascade around here, so that these two
lines do not have to be duplicated?

> +		} else {
> +			/* No arguments and no --all -- use default remote */
> +			remote = remote_get(NULL);
> +			result = fetch_one(remote, argc, argv);
> +		}
>  	} else if (multiple) {
>  		/* All arguments are assumed to be remotes or groups */
>  		for (i = 0; i < argc; i++)
> diff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh
> index 4b4b667..4e773ee 100755
> --- a/t/t5514-fetch-multiple.sh
> +++ b/t/t5514-fetch-multiple.sh
> @@ -55,6 +55,18 @@ test_expect_success 'git fetch --all' '
>  	 test_cmp expect output)
>  '
>  
> +test_expect_success 'git fetch (fetch.all = true)' '
> +	(git clone one test9 &&
> +	 cd test9 &&
> +	 git config fetch.all true &&
> +	 git remote add one ../one &&
> +	 git remote add two ../two &&
> +	 git remote add three ../three &&
> +	 git fetch &&
> +	 git branch -r > output &&
> +	 test_cmp ../test/expect output)
> +'
> +
>  test_expect_success 'git fetch --all should continue if a remote has errors' '
>  	(git clone one test2 &&
>  	 cd test2 &&
> @@ -91,6 +103,17 @@ test_expect_success 'git fetch --multiple (but only one remote)' '
>  	 test_cmp ../expect output)
>  '
>  
> +test_expect_success 'git fetch one (fetch.all = true)' '
> +	(cd test3 &&

This means that this test piece will break if the previous one did
not succeed, which is not a very good idea.

> +	 git config fetch.all true &&
> +	 git fetch three &&
> +	 git branch -r > output &&
> +	 test_cmp ../expect output &&

What future errors will this catch?

You have only "three" and "origin" and the expected output wants to
see tracking from both.  Does the test break when in some future
somebody broke the resulting code after applying this patch in such
a way that "fetch from only 'three'" from the command line gets
ignored and fetch.all went ahead and fetched from everybody?

> +	 git fetch --no-all &&
> +	 git branch -r > output &&
> +	 test_cmp ../expect output)

Same question.

> +'
> +
>  cat > expect << EOF
>    one/master
>    one/side