| Message ID | 20240104222259.15659-2-dev@tb6.eu (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/2] fetch: add new config option fetch.all | expand |
On Thu, Jan 4, 2024 at 5:23 PM Tamino Bauknecht <dev@tb6.eu> wrote: > This option can be used to restore the default behavior of "git fetch" > if the "fetch.all" config option is enabled. > The flag cannot be used in combination with "--all" or explicit > remote(s). > > Signed-off-by: Tamino Bauknecht <dev@tb6.eu> > --- > diff --git a/builtin/fetch.c b/builtin/fetch.c > @@ -2344,15 +2347,23 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) > + if (all && default_only) { > + die(_("fetch --all does not work with fetch --default-only")); To simplify the life of people who translate Git messages into other languages, these days we have standard wording for this type of message, and we extract the literal option from the message itself. So, this should be: die(_("options '%s' and '%s' cannot be used together"), "--all", "--default-only"); > diff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh > @@ -304,4 +304,45 @@ test_expect_success 'git config fetch.all false (fetch only default remote)' ' > +for fetch_all in true false > +do > + test_expect_success "git fetch --default-only (fetch only default remote with fetch.all = $fetch_all)" ' > + test_dir="test_default_only_$fetch_all" && > + setup_test_clone "$test_dir" && > + ( > + cd "$test_dir" && > + git config fetch.all $fetch_all && > + git fetch --default-only && > + cat >expect <<-\EOF && > + origin/HEAD -> origin/main > + origin/main > + origin/side > + EOF > + git branch -r >actual && > + test_cmp expect actual > + ) > + ' > +done Do you also want to test the case when "fetch.all" isn't set? > +test_expect_success 'git fetch --all does not work with --default-only' ' > + ( > + cd test && > + test_must_fail git fetch --all --default-only > + ) > +' Minor point: This sort of test can be written more succinctly without the subshell: test_expect_success 'git fetch --all does not work with --default-only' ' test_must_fail git -C test fetch --all --default-only '
Tamino Bauknecht <dev@tb6.eu> writes: > This option can be used to restore the default behavior of "git fetch" > if the "fetch.all" config option is enabled. > The flag cannot be used in combination with "--all" or explicit > remote(s). There is "--all" option that can be used to alter the behaviour of the command. This is OPT_BOOL(), and if you have [alias] f = fetch --all you can say "git f --no-all" to countermand it from the command line, as it is equivalent to "git fetch --all --no-all" and the last one wins. If you add "fetch.all" that makes the command behave as if it was given "--all" from the command line even when the user didn't, passing "--no-all" would be a lot more natural way to countermand it, no? "git fetch" (no parameters) are omitting two kinds of stuff, i.e., where we fetch from (repository) and what we are fetching from there (refspec). You created a need to override the configured fetch source (aka fetch.all), and in order to countermand it, one way is to tell the command to "fetch from the default repository", but for that, an option that does not say "repository" anywhere is closing door for future evolution of the command. What if the next person (which could be you) invented a way to say what refspec to be used without specifying it from the command line, and needs to say "no, no, no, do not use the configured value, but just use the default"? In other words, "--default" will be met by a reaction "default of which one???". Compared to that, I would think "--[no-]all" would be a lot simpler to understand. The best part is that you do not have to add a new option. Just make sure the one specified from the command line, either --all or --no-all, will cause the command to ignore the configured fetch.all and you do not need to add anything new to the UI.
diff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt index 0638cf276e..6c3a9bc3f6 100644 --- a/Documentation/config/fetch.txt +++ b/Documentation/config/fetch.txt @@ -52,8 +52,9 @@ fetch.pruneTags:: fetch.all:: If true, fetch will attempt to update all available remotes. - This behavior can be overridden by explicitly specifying one or - more remote(s) to fetch from. Defaults to false. + This behavior can be overridden by passing `--default-only` or + by explicitly specifying one or more remote(s) to fetch from. + Defaults to false. fetch.output:: Control how ref update status is printed. Valid values are diff --git a/Documentation/fetch-options.txt b/Documentation/fetch-options.txt index a1d6633a4f..61da5915f1 100644 --- a/Documentation/fetch-options.txt +++ b/Documentation/fetch-options.txt @@ -1,6 +1,10 @@ --all:: Fetch all remotes. +--default-only:: + Fetch only default remote. This flag can be used to overrule the + `fetch.all` configuration option and restore the default behavior. + -a:: --append:: Append ref names and object names of fetched refs to the diff --git a/builtin/fetch.c b/builtin/fetch.c index f1ad3e608e..de1f659b96 100644 --- a/builtin/fetch.c +++ b/builtin/fetch.c @@ -2140,6 +2140,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 = 0, multiple = 0; + int default_only = 0; int result = 0; int prune_tags_ok = 1; int enable_auto_gc = 1; @@ -2157,6 +2158,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, "default-only", &default_only, + N_("only fetch default remote")), OPT_BOOL(0, "set-upstream", &set_upstream, N_("set upstream for git pull/fetch")), OPT_BOOL('a', "append", &append, @@ -2344,15 +2347,23 @@ int cmd_fetch(int argc, const char **argv, const char *prefix) fetch_bundle_uri(the_repository, bundle_uri, NULL)) warning(_("failed to fetch bundles from '%s'"), bundle_uri); - if (all) { + if (all && default_only) { + die(_("fetch --all does not work with fetch --default-only")); + } else if (all || default_only) { + const char *fetch_argument = all ? "--all" : "--default-only"; if (argc == 1) - die(_("fetch --all does not take a repository argument")); + die(_("fetch %s does not take a repository argument"), + fetch_argument); else if (argc > 1) - die(_("fetch --all does not make sense with refspecs")); + die(_("fetch %s does not make sense with refspecs"), + fetch_argument); } - if (all || (config.all > 0 && !argc)) { - /* Only use fetch.all config option if no remotes were explicitly given */ + if (all || (config.all > 0 && !argc && !default_only)) { + /* + * Only use fetch.all config option if no remotes were + * explicitly given and if --default-only was not passed + */ (void) for_each_remote(get_one_remote_for_fetch, &list); /* do not do fetch_multiple() of one */ diff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh index 781c781808..1b23eef32c 100755 --- a/t/t5514-fetch-multiple.sh +++ b/t/t5514-fetch-multiple.sh @@ -304,4 +304,45 @@ test_expect_success 'git config fetch.all false (fetch only default remote)' ' ) ' +for fetch_all in true false +do + test_expect_success "git fetch --default-only (fetch only default remote with fetch.all = $fetch_all)" ' + test_dir="test_default_only_$fetch_all" && + setup_test_clone "$test_dir" && + ( + cd "$test_dir" && + git config fetch.all $fetch_all && + git fetch --default-only && + cat >expect <<-\EOF && + origin/HEAD -> origin/main + origin/main + origin/side + EOF + git branch -r >actual && + test_cmp expect actual + ) + ' +done + +test_expect_success 'git fetch --all does not work with --default-only' ' + ( + cd test && + test_must_fail git fetch --all --default-only + ) +' + +test_expect_success 'git fetch --default-only does not accept one explicit remote' ' + ( + cd test && + test_must_fail git fetch --default-only one + ) +' + +test_expect_success 'git fetch --default-only does not accept multiple explicit remotes' ' + ( + cd test && + test_must_fail git fetch --default-only one two three + ) +' + test_done
This option can be used to restore the default behavior of "git fetch" if the "fetch.all" config option is enabled. The flag cannot be used in combination with "--all" or explicit remote(s). Signed-off-by: Tamino Bauknecht <dev@tb6.eu> --- A first proposal for the command line option Junio mentioned. It's called "--default-only" for now, but I don't have a strong opinion on that matter and am open to suggestions. Alternatives I considered were "--default-remote" and only "--default". I'm also not sure about the positioning in code and documentation, is there some kind of convention about the order? For now, I simply added it behind "all" since it is related to (although incompatible with) it. Documentation/config/fetch.txt | 5 ++-- Documentation/fetch-options.txt | 4 ++++ builtin/fetch.c | 21 +++++++++++++---- t/t5514-fetch-multiple.sh | 41 +++++++++++++++++++++++++++++++++ 4 files changed, 64 insertions(+), 7 deletions(-)