remote: resolve "simple" @{push} when pushing to another remote - #2466

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]

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

Thanks!


Harald

"D. Ben Knoble" <ben.knoble@gmail.com> writes:

>> 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.

I actually wondered if the existing case for "simple" becomes a
subset of this new code?  After all, tracking_for_push_dest() asks
where the thing gets pushed to, no?

In any case, all the case arms in this function is getting a bit
unmanageably long and this block is, with an extra artificial block,
unnecessarily deeply nested by one extra level.  I wonder if we can
move the code for "case PUSH_DEFAULT_SIMPLE" case to a separate
function and make it a bit more readable?

I agree that the end result the commit wants to see is sensible and
consistent with the earlier "what should happen with 'simple' when
triangular?" topic.

Thanks.

> >> +                       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.
>
> I actually wondered if the existing case for "simple" becomes a
> subset of this new code?  After all, tracking_for_push_dest() asks
> where the thing gets pushed to, no?

Yes, both resolve to the same branch. The same remote case only adds a
check that the upstream has the same name, because "git push" refuses
otherwise.

> In any case, all the case arms in this function is getting a bit
> unmanageably long and this block is, with an extra artificial block,
> unnecessarily deeply nested by one extra level.  I wonder if we can
> move the code for "case PUSH_DEFAULT_SIMPLE" case to a separate
> function and make it a bit more readable?

Good point.


Harald

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> 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.
>
> Changes in v2:
>
>  * Moved the "simple" @{push} code out of branch_get_push_1() into its own
>    function in a new preparatory commit.

Looking good.  Will queue.

Thanks.