Re: [RFC PATCH v2 2/2] push: support pushing to a remote group

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 18, 2026, 22:25 UTC
Message-ID: <xmqqpl50ojvy.fsf@gitster.g>
In-Reply-To: <20260318204028.1010487-3-usmanakinyemi202@gmail.com>

Usman Akinyemi <usmanakinyemi202@gmail.com> writes:

Show 29 quoted lines

> diff --git a/t/t5566-push-group.sh b/t/t5566-push-group.sh
> new file mode 100755
> index 0000000000..9e0d378f2a
> --- /dev/null
> +++ b/t/t5566-push-group.sh
> @@ -0,0 +1,95 @@
> +#!/bin/sh
> +
> +test_description='push to remote group'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup' '
> +	for i in 1 2 3
> +	do
> +		git init --bare dest-$i.git &&
> +		git -C dest-$i.git symbolic-ref HEAD refs/heads/not-a-branch ||
> +		return 1
> +	done &&
> +	test_tick &&
> +	git commit --allow-empty -m "initial" &&
> +	git config set remote.remote-1.url "file://$(pwd)/dest-1.git" &&
> +	git config set remote.remote-1.fetch "+refs/heads/*:refs/remotes/remote-1/*" &&
> +	git config set remote.remote-2.url "file://$(pwd)/dest-2.git" &&
> +	git config set remote.remote-2.fetch "+refs/heads/*:refs/remotes/remote-2/*" &&
> +	git config set remote.remote-3.url "file://$(pwd)/dest-3.git" &&
> +	git config set remote.remote-3.fetch "+refs/heads/*:refs/remotes/remote-3/*" &&
> +	git config set remotes.all-remotes "remote-1 remote-2 remote-3"
> +'

So we have three remotes, dest-{1,2,3}.git/ that are all bare, and a remote group "all-remotes" that name them. Is there a reason why you want to use an unborn HEAD?

> +test_expect_success 'push to remote group pushes to all members' '
> +	git push all-remotes HEAD:refs/heads/main &&

Our "push" exits with 0 status. How would we make sure we pushed correctly?

> +	j= &&
> +	for i in 1 2 3
> +	do
> +		git -C dest-$i.git for-each-ref >actual-$i &&

We grab dest-$i's refs to actual-$i

> +		if test -n "$j"
> +		then
> +			test_cmp actual-$j actual-$i

and make sure if refs in dest-N differ from dest-(N-1)'s refs.

> +		else
> +			cat actual-$i

of course, the first one has nothing to compare against, so we get a debugging "cat" for it.

> +		fi &&
> +		j=$i ||
> +		return 1

But does this loop test what we really want to make sure? You could write your "group push" to push one commit less than what was asked to push out to all remotes, and they will match with each other to pass the above test, but it would be different from our original.

Don't we know the exact state of refs in these dest-$i.git repositories? If we do, then

    printf "%s commit\trefs/heads/main\n" >expect &&
    for i in 1 2 3
    do
	git -C dest-$i.git for-each-ref >actual &&
	test_cmp expect actual || return 1
    done

perhaps?

Show 12 quoted lines

> +test_expect_success 'push second commit to group updates all members' '
> +	test_tick &&
> +	git commit --allow-empty -m "second" &&
> +	git push all-remotes HEAD:refs/heads/main &&
> +	for i in 1 2 3
> +	do
> +		git -C dest-$i.git rev-parse refs/heads/main >hash-$i ||
> +		return 1
> +	done &&
> +	test_cmp hash-1 hash-2 &&
> +	test_cmp hash-2 hash-3
> +'

Again, the primary thing we are interested in is that dest-*.git has a copy of what we pushed. They may be identical to each other among themselves but they still could be different from what we pushed, and that is something we want to catch, no?

    git rev-parse refs/heads/main >expect &&
    for i in 1 2 3
    do
	git -C dest-$i.git rev-parse refs/heads/main >actual &&
	test_cmp expect actual || return 1
    done

Show 9 quoted lines

> +
> +test_expect_success 'push to single remote in group does not affect others' '
> +	test_tick &&
> +	git commit --allow-empty -m "third" &&
> +	git push remote-1 HEAD:refs/heads/main &&
> +	git -C dest-1.git rev-parse refs/heads/main >hash-after-1 &&
> +	git -C dest-2.git rev-parse refs/heads/main >hash-after-2 &&
> +	! test_cmp hash-after-1 hash-after-2
> +'

Obviously correct.

> +test_expect_success 'push to nonexistent group fails with error' '
> +	test_must_fail git push no-such-group HEAD:refs/heads/main
> +'

Obviously correct---we probably should already have a test to see that a push to nonexistent remote repository fails (missing one you cannot even tell if it is a single remote or a group), in which case this is not even needed.

> +test_expect_success 'push explicit refspec to group' '
> +	test_tick &&
> +	git commit --allow-empty -m "fourth" &&
> +	git push all-remotes HEAD:refs/heads/other &&

Didn't we do this already? We did so with 'main' into dest-*.git that did not know anything about 'main' (after its HEAD repointed to a missing branch).

Show 13 quoted lines

> +	for i in 1 2 3
> +	do
> +		git -C dest-$i.git rev-parse refs/heads/other >other-hash-$i ||
> +		return 1
> +	done &&
> +	test_cmp other-hash-1 other-hash-2 &&
> +	test_cmp other-hash-2 other-hash-3
> +'
> +
> +test_expect_success 'mirror remote in group with refspec fails' '
> +	git config set remote.remote-1.mirror true &&
> +	test_must_fail git push all-remotes HEAD:refs/heads/main 2>err &&
> +	grep "mirror" err &&

test_grep??

Show 11 quoted lines

> +	git config unset remote.remote-1.mirror
> +'
> +test_expect_success 'push.default=current works with group push' '
> +	git config set push.default current &&
> +	test_tick &&
> +	git commit --allow-empty -m "fifth" &&
> +	git push all-remotes &&
> +	git config unset push.default
> +'
> +
> +test_done