Øystein Walle <oystwa@xxxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html