diff mbox series

[v2,1/2] fetch: add new config option fetch.all

Message ID 20240104222259.15659-1-dev@tb6.eu (mailing list archive)
State Superseded
Headers show
Series [v2,1/2] fetch: add new config option fetch.all | expand

Commit Message

Tamino Bauknecht Jan. 4, 2024, 10:22 p.m. UTC
This commit introduces the new boolean configuration option fetch.all
which allows to fetch all available remotes by default. The config
option can be overridden by explicitly specifying a remote.
The behavior for --all is unchanged and calling git-fetch with --all and
a remote will still result in an error.

The option was also added to the config documentation and new tests
cover the expected behavior.

Signed-off-by: Tamino Bauknecht <dev@tb6.eu>
---
I fixed the formatting in the test cases and replaced the "cp" with an
explicit "expect".

 Documentation/config/fetch.txt |  5 ++
 builtin/fetch.c                | 11 ++++
 t/t5514-fetch-multiple.sh      | 95 ++++++++++++++++++++++++++++++++++
 3 files changed, 111 insertions(+)

Comments

Eric Sunshine Jan. 5, 2024, 1:02 a.m. UTC | #1
On Thu, Jan 4, 2024 at 5:23 PM Tamino Bauknecht <dev@tb6.eu> wrote:
> This commit introduces the new boolean configuration option fetch.all
> which allows to fetch all available remotes by default. The config
> option can be overridden by explicitly specifying a remote.
> The behavior for --all is unchanged and calling git-fetch with --all and
> a remote will still result in an error.
>
> The option was also added to the config documentation and new tests
> cover the expected behavior.
>
> Signed-off-by: Tamino Bauknecht <dev@tb6.eu>
> ---
> I fixed the formatting in the test cases and replaced the "cp" with an
> explicit "expect".

Thanks, looks better. More below...

> diff --git a/t/t5514-fetch-multiple.sh b/t/t5514-fetch-multiple.sh
> @@ -24,6 +24,15 @@ setup_repository () {
> +setup_test_clone () {
> +       test_dir="$1"
> +       git clone one "$test_dir"
> +       for r in one two three
> +       do
> +               git -C "$test_dir" remote add "$r" "../$r" || return 1
> +       done
> +}

I wasn't paying attention to this function in the previous round, but
it ought to be made more robust. If the `clone` operation fails, we
want to know about it right away rather than finding out about it "by
accident" when the subsequent `remote add` operation fails. In other
words, you should maintain an unbroken &&-chain, not just in test
bodies, but also in functions which are called from within test
bodies. So, this should really be:

    setup_test_clone () {
        test_dir="$1" &&
        git clone one "$test_dir" &&
        for r in one two three
        do
            git -C "$test_dir" remote add "$r" "../$r" || return 1
        done
    }

> @@ -209,4 +218,90 @@ test_expect_success 'git fetch --multiple --jobs=0 picks a default' '
> +for fetch_all in true false
> +do
> +       test_expect_success "git fetch --all (works with fetch.all = $fetch_all)" '
> +               test_dir="test_fetch_all_$fetch_all" &&
> +               setup_test_clone "$test_dir" &&
> +               (
> +                       cd "$test_dir" &&
> +                       git config fetch.all $fetch_all &&
> +                       git fetch --all &&
> +                       cat >expect <<-\EOF &&
> +                         ...
> +                       EOF
> +                       git branch -r >actual &&
> +                       test_cmp expect actual
> +               )
> +       '
> +done
> +
> +test_expect_success 'git fetch (fetch all remotes with fetch.all = true)' '
> +       setup_test_clone test9 &&
> +       (
> +               cd test9 &&
> +               git config fetch.all true &&
> +               git fetch --all &&
> +               git branch -r >actual &&
> +               cat >expect <<-\EOF &&
> +                 ...
> +               EOF
> +               test_cmp expect actual
> +       )
> +'

I'm probably overlooking something, but isn't this testing the exact
same thing as was tested in the "true" case of the loop just above?

Or maybe there is a bug in this test and you meant `git fetch` rather
than `git fetch --all`?
diff mbox series

Patch

diff --git a/Documentation/config/fetch.txt b/Documentation/config/fetch.txt
index aea5b97477..0638cf276e 100644
--- a/Documentation/config/fetch.txt
+++ b/Documentation/config/fetch.txt
@@ -50,6 +50,11 @@  fetch.pruneTags::
 	refs. See also `remote.<name>.pruneTags` and the PRUNING
 	section of linkgit:git-fetch[1].
 
+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.
+
 fetch.output::
 	Control how ref update status is printed. Valid values are
 	`full` and `compact`. Default value is `full`. See the
diff --git a/builtin/fetch.c b/builtin/fetch.c
index a284b970ef..f1ad3e608e 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -102,6 +102,7 @@  static struct string_list negotiation_tip = STRING_LIST_INIT_NODUP;
 
 struct fetch_config {
 	enum display_format display_format;
+	int all;
 	int prune;
 	int prune_tags;
 	int show_forced_updates;
@@ -115,6 +116,11 @@  static int git_fetch_config(const char *k, const char *v,
 {
 	struct fetch_config *fetch_config = cb;
 
+	if (!strcmp(k, "fetch.all")) {
+		fetch_config->all = git_config_bool(k, v);
+		return 0;
+	}
+
 	if (!strcmp(k, "fetch.prune")) {
 		fetch_config->prune = git_config_bool(k, v);
 		return 0;
@@ -2121,6 +2127,7 @@  int cmd_fetch(int argc, const char **argv, const char *prefix)
 {
 	struct fetch_config config = {
 		.display_format = DISPLAY_FORMAT_FULL,
+		.all = -1,
 		.prune = -1,
 		.prune_tags = -1,
 		.show_forced_updates = 1,
@@ -2342,6 +2349,10 @@  int cmd_fetch(int argc, const char **argv, const char *prefix)
 			die(_("fetch --all does not take a repository argument"));
 		else if (argc > 1)
 			die(_("fetch --all does not make sense with refspecs"));
+	}
+
+	if (all || (config.all > 0 && !argc)) {
+		/* Only use fetch.all config option if no remotes were explicitly given */
 		(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 a95841dc36..781c781808 100755
--- a/t/t5514-fetch-multiple.sh
+++ b/t/t5514-fetch-multiple.sh
@@ -24,6 +24,15 @@  setup_repository () {
 	)
 }
 
+setup_test_clone () {
+	test_dir="$1"
+	git clone one "$test_dir"
+	for r in one two three
+	do
+		git -C "$test_dir" remote add "$r" "../$r" || return 1
+	done
+}
+
 test_expect_success setup '
 	setup_repository one &&
 	setup_repository two &&
@@ -209,4 +218,90 @@  test_expect_success 'git fetch --multiple --jobs=0 picks a default' '
 	 git fetch --multiple --jobs=0)
 '
 
+for fetch_all in true false
+do
+	test_expect_success "git fetch --all (works with fetch.all = $fetch_all)" '
+		test_dir="test_fetch_all_$fetch_all" &&
+		setup_test_clone "$test_dir" &&
+		(
+			cd "$test_dir" &&
+			git config fetch.all $fetch_all &&
+			git fetch --all &&
+			cat >expect <<-\EOF &&
+			  one/main
+			  one/side
+			  origin/HEAD -> origin/main
+			  origin/main
+			  origin/side
+			  three/another
+			  three/main
+			  three/side
+			  two/another
+			  two/main
+			  two/side
+			EOF
+			git branch -r >actual &&
+			test_cmp expect actual
+		)
+	'
+done
+
+test_expect_success 'git fetch (fetch all remotes with fetch.all = true)' '
+	setup_test_clone test9 &&
+	(
+		cd test9 &&
+		git config fetch.all true &&
+		git fetch --all &&
+		git branch -r >actual &&
+		cat >expect <<-\EOF &&
+		  one/main
+		  one/side
+		  origin/HEAD -> origin/main
+		  origin/main
+		  origin/side
+		  three/another
+		  three/main
+		  three/side
+		  two/another
+		  two/main
+		  two/side
+		EOF
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'git fetch one (explicit remote overrides fetch.all)' '
+	setup_test_clone test10 &&
+	(
+		cd test10 &&
+		git config fetch.all true &&
+		git fetch one &&
+		cat >expect <<-\EOF &&
+		  one/main
+		  one/side
+		  origin/HEAD -> origin/main
+		  origin/main
+		  origin/side
+		EOF
+		git branch -r >actual &&
+		test_cmp expect actual
+	)
+'
+
+test_expect_success 'git config fetch.all false (fetch only default remote)' '
+	setup_test_clone test11 &&
+	(
+		cd test11 &&
+		git config fetch.all false &&
+		git fetch &&
+		cat >expect <<-\EOF &&
+		  origin/HEAD -> origin/main
+		  origin/main
+		  origin/side
+		EOF
+		git branch -r >actual &&
+		test_cmp expect actual
+	)
+'
+
 test_done