All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] remote: resolve "simple" @{push} when pushing to another remote
@ 2026-10-27 12:43 Harald Nordgren via GitGitGadget
  2026-10-28 22:24 ` D. Ben Knoble
  0 siblings, 1 reply; 3+ messages in thread
From: Harald Nordgren via GitGitGadget @ 2026-10-27 12:43 UTC (permalink / raw)
  To: git; +Cc: Harald Nordgren, Harald Nordgren

From: Harald Nordgren <haraldnordgren@gmail.com>

With push.default left at "simple", a branch whose upstream is on one
remote and whose push remote is another gets pushed by "git push" to a
branch of the same name on the push remote. But @{push} for that
branch fails with "cannot resolve 'simple' push to a single
destination". As a result "git status" shows the "git push" hint under
the comparison with the upstream, even though "git push" goes
elsewhere, and leaves out the comparison with the push branch when
status.compareBranches asks for it.

"git push" learned to handle "simple" this way in ed2b18292b (push:
change `simple` to accommodate triangular workflows, 2013-06-19). When
@{push} was added in e291c75a95 (remote.c: add branch_get_push,
2015-05-21), its "simple" case only covered pushing back to the remote
the branch is fetched from, where the upstream must have the same name.

Make @{push} follow "git push". When the push remote is not the remote
of the branch, resolve to the branch of the same name on the push
remote, as "current" does.

The test added in 9bf9eed093 (remote: fix leak in branch_get_push_1()
with invalid "simple" config, 2026-01-19) relied on a different push
remote being invalid for "simple". Use an upstream with a different
name on the same remote instead, so that it still exercises the error
path.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
    remote: resolve "simple" @{push} when pushing to another remote
    
    With push.default=simple and remote.pushDefault pointing at a fork, git
    push pushes the branch to the fork, but @{push} fails with "cannot
    resolve 'simple' push to a single destination".
    
    So git status cannot compare the branch with where it was pushed. Make
    @{push} resolve to where git push goes.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2466%2FHaraldNordgren%2Fat-push-simple-triangular-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2466/HaraldNordgren/at-push-simple-triangular-v1
Pull-Request: https://github.com/git/git/pull/2466

 remote.c                  |  4 ++++
 t/for-each-ref-tests.sh   |  4 +++-
 t/t1514-rev-parse-push.sh | 11 +++++++++++
 3 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/remote.c b/remote.c
index 49ee2ea318..fd434a3a10 100644
--- a/remote.c
+++ b/remote.c
@@ -2016,6 +2016,10 @@ static char *branch_get_push_1(struct repository *repo,
 			const char *up;
 			char *cur;
 
+			if (strcmp(remote->name,
+				   remotes_remote_for_branch(remote_state, branch, NULL)))
+				return tracking_for_push_dest(repo, remote, branch->refname, err);
+
 			up = branch_get_upstream(branch, err);
 			if (!up)
 				return NULL;
diff --git a/t/for-each-ref-tests.sh b/t/for-each-ref-tests.sh
index b1a10dd933..4991a82b91 100644
--- a/t/for-each-ref-tests.sh
+++ b/t/for-each-ref-tests.sh
@@ -1745,9 +1745,11 @@ test_expect_success ':remotename and :remoteref' '
 '
 
 test_expect_success '%(push) with an invalid push-simple config' '
+	test_when_finished "git config branch.main.merge refs/heads/main" &&
+	git config branch.main.merge refs/heads/not-main &&
 	echo "refs/heads/main " >expect &&
 	git -c push.default=simple \
-	    -c remote.pushdefault=myfork \
+	    -c remote.pushdefault=origin \
 	    for-each-ref \
 	    --format="%(refname) %(push)" refs/heads/main >actual &&
 	test_cmp expect actual
diff --git a/t/t1514-rev-parse-push.sh b/t/t1514-rev-parse-push.sh
index 5a4f16867a..4187e2a504 100755
--- a/t/t1514-rev-parse-push.sh
+++ b/t/t1514-rev-parse-push.sh
@@ -60,6 +60,17 @@ test_expect_success '@{push} with pushremote defined' '
 	resolve topic@{push} refs/remotes/other/topic
 '
 
+test_expect_success 'triangular @{push} with default=simple and remote.pushdefault' '
+	test_config push.default simple &&
+	test_config remote.pushdefault other &&
+	test_when_finished "git checkout main" &&
+	git checkout topic &&
+	test_commit simple-triangular &&
+	git push &&
+	resolve topic@{push} refs/remotes/other/topic &&
+	test_cmp_rev topic topic@{push}
+'
+
 test_expect_success '@{p} is short for @{push}' '
 	test_config push.default current &&
 	test_config branch.topic.pushremote other &&

base-commit: 7e34d8174dcda3435d1404c9a30bef46afe6c13e
-- 
gitgitgadget

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] remote: resolve "simple" @{push} when pushing to another remote
  2026-10-27 12:43 [PATCH] remote: resolve "simple" @{push} when pushing to another remote Harald Nordgren via GitGitGadget
@ 2026-10-28 22:24 ` D. Ben Knoble
  2026-10-29 10:34   ` Harald Nordgren
  0 siblings, 1 reply; 3+ messages in thread
From: D. Ben Knoble @ 2026-10-28 22:24 UTC (permalink / raw)
  To: Harald Nordgren via GitGitGadget; +Cc: git, Harald Nordgren

On Tue, Oct 27, 2026 at 8:48 AM Harald Nordgren via GitGitGadget
<gitgitgadget@gmail.com> wrote:
>
> From: Harald Nordgren <haraldnordgren@gmail.com>
>
> With push.default left at "simple", a branch whose upstream is on one
> remote and whose push remote is another gets pushed by "git push" to a
> branch of the same name on the push remote. But @{push} for that
> branch fails with "cannot resolve 'simple' push to a single
> destination".

At first I was a bit confused about the need for a different push
remote, but then I realized: simple's documentation says that if the
push and pull remote are the *same* you need the upstream to have the
same name! So in the differing case, it might behave quite differently
than I'm used to.

> "git push" learned to handle "simple" this way in ed2b18292b (push:
> change `simple` to accommodate triangular workflows, 2013-06-19). When
> @{push} was added in e291c75a95 (remote.c: add branch_get_push,
> 2015-05-21), its "simple" case only covered pushing back to the remote
> the branch is fetched from, where the upstream must have the same name.
>
> Make @{push} follow "git push". When the push remote is not the remote
> of the branch, resolve to the branch of the same name on the push
> remote, as "current" does.

Whatever else we do, making @{push} work in more scenarios seems good
to me so we can be consistent.

> The test added in 9bf9eed093 (remote: fix leak in branch_get_push_1()
> with invalid "simple" config, 2026-01-19) relied on a different push
> remote being invalid for "simple". Use an upstream with a different
> name on the same remote instead, so that it still exercises the error
> path.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
> ---
>     remote: resolve "simple" @{push} when pushing to another remote
>
>     With push.default=simple and remote.pushDefault pointing at a fork, git
>     push pushes the branch to the fork, but @{push} fails with "cannot
>     resolve 'simple' push to a single destination".
>
>     So git status cannot compare the branch with where it was pushed. Make
>     @{push} resolve to where git push goes.
>
> Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2466%2FHaraldNordgren%2Fat-push-simple-triangular-v1
> Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2466/HaraldNordgren/at-push-simple-triangular-v1
> Pull-Request: https://github.com/git/git/pull/2466
>
>  remote.c                  |  4 ++++
>  t/for-each-ref-tests.sh   |  4 +++-
>  t/t1514-rev-parse-push.sh | 11 +++++++++++
>  3 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/remote.c b/remote.c
> index 49ee2ea318..fd434a3a10 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -2016,6 +2016,10 @@ static char *branch_get_push_1(struct repository *repo,
>                         const char *up;
>                         char *cur;
>
> +                       if (strcmp(remote->name,
> +                                  remotes_remote_for_branch(remote_state, branch, NULL)))
> +                               return tracking_for_push_dest(repo, remote, branch->refname, err);

This certainly matches the logic described in the log message, and I
think it's probably right based on that description. One thing I can't
decide is whether it should come before or after the current set of
cases, and I'm undecided. On one hand, it's a new case, so I would
think it should come after. OTOH, it's a fairly straightforward case,
and I usually like discharging those early so I can focus on the
complex stuff.

So, I would say this is probably fine as-is?

[Skipping the tests for now]

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] remote: resolve "simple" @{push} when pushing to another remote
  2026-10-28 22:24 ` D. Ben Knoble
@ 2026-10-29 10:34   ` Harald Nordgren
  0 siblings, 0 replies; 3+ messages in thread
From: Harald Nordgren @ 2026-10-29 10:34 UTC (permalink / raw)
  To: D. Ben Knoble; +Cc: Harald Nordgren via GitGitGadget, git

> So, I would say this is probably fine as-is?

Thanks!


Harald

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-29 10:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-27 12:43 [PATCH] remote: resolve "simple" @{push} when pushing to another remote Harald Nordgren via GitGitGadget
2026-10-28 22:24 ` D. Ben Knoble
2026-10-29 10:34   ` Harald Nordgren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.