Subject: RE: [PATCH 1/1] Add git remote group sub-command

From: <[email-masked]>
Date: Sat, 3 May 2025 13:19:52 -0400

On May 3, 2025 12:10 PM, Lazar Sumar wrote:
>Remote groups are an existing feature of git presently managed exclusively via the
>config. This patch gives them their own subcommand for listing the existing groups
>with the intent of adding future subcommands under it for
>adding/updating/removing groups.

Can you expand on the primary use case, please? Specifically the implications of what
The group subcommand not only does but the side effects of using it.

Thanks.

>As a side-effect this command can now be used in bash tab completion for the fetch
>command which did not offer them as a completion option previously. Note that
>the push command does not accept remote groups as an argument.
>
>Signed-off-by: Lazar Sumar <[email-masked]>
>---
> Documentation/git-remote.adoc          |  6 ++++
> builtin/remote.c                       | 47 ++++++++++++++++++++++++++
> contrib/completion/git-completion.bash | 21 ++++++++++--
> t/t5506-remote-groups.sh               | 31 +++++++++++++++++
> 4 files changed, 103 insertions(+), 2 deletions(-)
>
>diff --git a/Documentation/git-remote.adoc b/Documentation/git-remote.adoc
>index 932a5c3ea4..87ddda8d58 100644
>--- a/Documentation/git-remote.adoc
>+++ b/Documentation/git-remote.adoc
>@@ -22,6 +22,7 @@ SYNOPSIS
> 'git remote' [-v | --verbose] 'show' [-n] <name>...
> 'git remote prune' [-n | --dry-run] <name>...
> 'git remote' [-v | --verbose] 'update' [-p | --prune] [(<group> | <remote>)...]
>+'git remote group'
>
> DESCRIPTION
> -----------
>@@ -197,6 +198,11 @@ be updated.  (See linkgit:git-config[1]).
> +
> With `--prune` option, run pruning against all the remotes that are updated.
>
>+'group'::
>+
>+List all configured remote groups. The remote groups configuration is
>+achieved using the `remotes.<group>` configuration variables.  (See
>+linkgit:git-config[1]).
>
> DISCUSSION
> ----------
>diff --git a/builtin/remote.c b/builtin/remote.c index b4baa34e66..2217447230
>100644
>--- a/builtin/remote.c
>+++ b/builtin/remote.c
>@@ -28,6 +28,7 @@ static const char * const builtin_remote_usage[] = {
> 	N_("git remote [-v | --verbose] show [-n] <name>"),
> 	N_("git remote prune [-n | --dry-run] <name>"),
> 	N_("git remote [-v | --verbose] update [-p | --prune] [(<group> |
><remote>)...]"),
>+	N_("git remote group"),
> 	N_("git remote set-branches [--add] <name> <branch>..."),
> 	N_("git remote get-url [--push] [--all] <name>"),
> 	N_("git remote set-url [--push] <name> <newurl> [<oldurl>]"), @@ -77,6
>+78,11 @@ static const char * const builtin_remote_update_usage[] = {
> 	NULL
> };
>
>+static const char * const builtin_remote_group_usage[] = {
>+	N_("git remote group"),
>+	NULL
>+};
>+
> static const char * const builtin_remote_geturl_usage[] = {
> 	N_("git remote get-url [--push] [--all] <name>"),
> 	NULL
>@@ -1633,6 +1639,46 @@ static int update(int argc, const char **argv, const char
>*prefix,
> 	return run_command(&cmd);
> }
>
>+static int get_remote_group(const char *key, const char *value UNUSED,
>+			    const struct config_context *ctx UNUSED,
>+			    void *priv)
>+{
>+	struct string_list *remote_group_list = priv;
>+
>+	if (skip_prefix(key, "remotes.", &key)) {
>+		size_t wordlen = strlen(key);
>+		if (wordlen >= 1)
>+			string_list_append_nodup(remote_group_list,
>+						xstrndup(key, wordlen));
>+	}
>+	string_list_remove_duplicates(remote_group_list, 0);
>+
>+	return 0;
>+}
>+
>+static int group(int argc, const char **argv, const char *prefix,
>+	struct repository *repo UNUSED)
>+{
>+	struct string_list remote_group_list = STRING_LIST_INIT_DUP;
>+	struct option options[] = {
>+		OPT_END()
>+	};
>+
>+	argc = parse_options(argc, argv, prefix, options,
>+		builtin_remote_group_usage, 0);
>+	if (argc != 0)
>+		usage_with_options(builtin_remote_group_usage, options);
>+
>+	git_config(get_remote_group, &remote_group_list);
>+	for (int i = 0; i < remote_group_list.nr; i++) {
>+		const char *name = remote_group_list.items[i].string;
>+		printf_ln(_("%s"), name);
>+	}
>+	string_list_clear(&remote_group_list, 0);
>+
>+	return 0;
>+}
>+
> static int remove_all_fetch_refspecs(const char *key)  {
> 	return git_config_set_multivar_gently(key, NULL, NULL, @@ -1844,6
>+1890,7 @@ int cmd_remote(int argc,
> 		OPT_SUBCOMMAND("show", &fn, show),
> 		OPT_SUBCOMMAND("prune", &fn, prune),
> 		OPT_SUBCOMMAND("update", &fn, update),
>+		OPT_SUBCOMMAND("group", &fn, group),
> 		OPT_END()
> 	};
>
>diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-
>completion.bash
>index e3d88b0672..658bea9d96 100644
>--- a/contrib/completion/git-completion.bash
>+++ b/contrib/completion/git-completion.bash
>@@ -1050,6 +1050,17 @@ __git_remotes ()
> 	__git remote
> }
>
>+__git_remote_groups ()
>+{
>+	__git remote group
>+}
>+
>+__git_remotes_or_groups ()
>+{
>+	__git_remotes
>+	__git_remote_groups
>+}
>+
> # Returns true if $1 matches the name of a configured remote, false otherwise.
> __git_is_configured_remote ()
> {
>@@ -1181,7 +1192,10 @@ __git_complete_remote_or_refspec ()
> 		((c++))
> 	done
> 	if [ -z "$remote" ]; then
>-		__gitcomp_nl "$(__git_remotes)"
>+		case "$cmd" in
>+		push) __gitcomp_nl "$(__git_remotes)" ;;
>+		*) __gitcomp_nl "$(__git_remotes_or_groups)" ;;
>+		esac
> 		return
> 	fi
> 	if [ $no_complete_refspec = 1 ]; then
>@@ -3073,7 +3087,7 @@ _git_remote ()
> {
> 	local subcommands="
> 		add rename remove set-head set-branches
>-		get-url set-url show prune update
>+		get-url set-url show prune update group
> 		"
> 	local subcommand="$(__git_find_on_cmdline "$subcommands")"
> 	if [ -z "$subcommand" ]; then
>@@ -3109,6 +3123,9 @@ _git_remote ()
> 	update,*)
> 		__gitcomp "$(__git_remotes) $(__git_get_config_variables
>"remotes")"
> 		;;
>+	group,--*)
>+		__gitcomp_builtin remote_group
>+		;;
> 	set-url,--*)
> 		__gitcomp_builtin remote_set-url
> 		;;
>diff --git a/t/t5506-remote-groups.sh b/t/t5506-remote-groups.sh index
>16e9a1bc2f..7859e6805b 100755
>--- a/t/t5506-remote-groups.sh
>+++ b/t/t5506-remote-groups.sh
>@@ -31,6 +31,12 @@ repo_fetched() {
> 	return 1
> }
>
>+check_groups() {
>+	echo $@ | sort >expected_groups;
>+	git remote group | sort >listed_groups;
>+	git diff --no-index --quiet expected_groups listed_groups }
>+
> test_expect_success 'setup' '
> 	mkdir one && (cd one && git init) &&
> 	mkdir two && (cd two && git init) &&
>@@ -64,6 +70,12 @@ test_expect_success 'updating group updates all members
>(remote update)' '
> 	repo_fetched two
> '
>
>+test_expect_success 'prints the configured group "all" once (remote group)' '
>+	echo "all" >expect &&
>+	test_expect_code 0 git remote group 1>actual &&
>+	test_cmp expect actual
>+'
>+
> test_expect_success 'updating group updates all members (fetch)' '
> 	mark fetch-group-all &&
> 	update_repos &&
>@@ -81,6 +93,15 @@ test_expect_success 'updating group does not update non-
>members (remote update)'
> 	! repo_fetched two
> '
>
>+test_expect_success 'prints configured groups "all" and "some" (remote group)' '
>+	cat >expect <<-EOF &&
>+	all
>+	some
>+	EOF
>+	test_expect_code 0 git remote group 1>actual &&
>+	test_cmp expect actual
>+'
>+
> test_expect_success 'updating group does not update non-members (fetch)' '
> 	mark fetch-group-some &&
> 	update_repos &&
>@@ -107,4 +128,14 @@ test_expect_success 'updating group in parallel with a
>duplicate remote does not
> 	repo_fetched one
> '
>
>+test_expect_success 'groups are printed in order of configuration (remote group)' '
>+	cat >expect <<-EOF &&
>+	all
>+	some
>+	duplicate
>+	EOF
>+	test_expect_code 0 git remote group 1>actual &&
>+	test_cmp expect actual
>+'
>+
> test_done
>--
>2.49.0.460.g0390bdefd0.dirty