Re: [PATCH v3 1/1] remote.c: fix handling of %(push:remoteref)

[Damien Robert <damien.olivier.robert@gmail.com>]

From Jeff King, Sat 28 Mar 2020 at 09:15:53 (-0400) :

> The tests makes sense to me, though I found a few nits to pick:
[...]

So I updated the tests as follow. I put them in another test, as you
suggested, this is indeed much clearer.

test_expect_success ':push:remoteref' '
	git init remote-tests &&
	(
		cd remote-tests &&
		test_commit initial &&
		git remote add from fifth.coffee:blub &&
		git config branch.master.remote from &&
		actual="$(git -c push.default=simple for-each-ref \
			--format="%(push:remotename),%(push:remoteref)" \
			refs/heads/master)" &&
		test from, = "$actual" &&
		git config branch.master.merge refs/heads/master &&
		actual="$(git -c push.default=simple for-each-ref \
			--format="%(push:remotename),%(push:remoteref)" \
			refs/heads/master)" &&
		test from,refs/heads/master = "$actual" &&
		git config branch.master.merge refs/heads/other &&
		actual="$(git -c push.default=simple for-each-ref \
			--format="%(push:remotename),%(push:remoteref)" \
			refs/heads/master)" &&
		test from, = "$actual" &&
		actual="$(git -c push.default=upstream for-each-ref \
			--format="%(push:remotename),%(push:remoteref)" \
			refs/heads/master)" &&
		test from,refs/heads/other = "$actual" &&
		actual="$(git -c push.default=current for-each-ref \
			--format="%(push:remotename),%(push:remoteref)" \
			refs/heads/master)" &&
		test from,refs/heads/master = "$actual" &&
		actual="$(git -c push.default=matching for-each-ref \
			--format="%(push:remotename),%(push:remoteref)" \
			refs/heads/master)" &&
		test from,refs/heads/master = "$actual" &&
		actual="$(git -c push.default=nothing for-each-ref \
			--format="%(push:remotename),%(push:remoteref)" \
			refs/heads/master)" &&
		test from, = "$actual"
	)
'

And the test works with my patch.

So I decided for completude to also test with
		git config branch.master.pushRemote to
to test with a triangular workflow, and I found several (already existing)
bugs.

Heres what happen with such a triangular workflow when we do a `git push`:
- with push.default=simple, we have
	case PUSH_DEFAULT_SIMPLE:
		if (triangular)
			setup_push_current(remote, branch);
		else
			setup_push_upstream(remote, branch, triangular, 1);
		break;
  so the current branch is always pushed.
- with push.default=upstream, we have
	case PUSH_DEFAULT_UPSTREAM:
		setup_push_upstream(remote, branch, triangular, 0);
		break;
  which then gives
  	if (triangular)
		die(_("You are pushing to remote '%s', which is not the upstream of\n"
		      "your current branch '%s', without telling me what to push\n"
		      "to update which remote branch."),

By the way this matches what the documentation says.

However here is the result of
git -c push.default=$value for-each-ref --format="%(push:remotename),%(push:remoteref),%(push)" refs/heads/master
for $value=
- simple: to,,
- upstream: to,refs/heads/other,refs/remotes/from/other

Note that without my patch the %(push:remoteref) values would always be empty,
but my patch does not touch %(push).

So in both branch_get_push_1 and branch_get_push_remoteref I should first
detect if we have a triangular workflow, and update the logic of the code
accordingly.