remote: resolve "simple" @{push} when pushing to another remote
PATCH 1/1 remote: resolve "simple" @{push} when pushing to another remote
From: Harald Nordgren <[email protected]>
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.
---
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(-)
remote.c +4 -0
@@ -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 ;
t/for-each-ref-tests.sh +3 -1
@@ -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
t/t1514-rev-parse-push.sh +11 -0
@@ -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
Signature · 1 line
gitgitgadget
Quoted 8 lines · Harald Nordgren via GitGitGadget
From: Harald Nordgren <[email protected]>
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.
Quoted 9 lines
"git push" learned to handle "simple" this way in
ed2b18292b(push:
changesimpleto accommodate triangular workflows, 2013-06-19). When @{push} was added ine291c75a95(remote.c: addbranch_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.
Quoted 31 lines
The test added in
9bf9eed093(remote: fix leak inbranch_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 <[email protected]>---
remote: resolve "simple" @{push} when pushing to another remoteWith 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/2466remote.c | 4 ++++ t/for-each-ref-tests.sh | 4 +++- t/t1514-rev-parse-push.sh | 11 +++++++++++ 3 files changed, 18 insertions(+), 1 deletion(-)remote.c +0
-0
remote.c static char *branch_get_push_1(struct repository *repo, · line 2021
Quoted 6 lines
remote.c +3
-0@@ -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]
Quoted 1 line
So, I would say this is probably fine as-is?
Thanks!
Harald
Quoted 8 lines · D. Ben Knoble
Quoted 8 lines
remote.c +0
-0@@ -2016,6 +2016,10 @@ static char *branch_get_push_1(struct repository *repo,const char *up;char *cur;
Quoted 11 lines
remote.c static char *branch_get_push_1(struct repository *repo, · line 2021
Quoted 3 lines
remote.c +3
-0+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.
Quoted 12 lines
Quoted 11 lines
remote.c static char *branch_get_push_1(struct repository *repo, · line 2021
Quoted 3 lines
remote.c +3
-0+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.
Quoted 3 lines
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.
Quoted 5 lines
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
v2 cover remote: resolve "simple" @{push} when pushing to another remote
In reply to Harald Nordgren via GitGitGadgetWith 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.
Harald Nordgren (2):
remote: move "simple" @{push} resolution into a helper
remote: resolve "simple" @{push} when pushing to another remote
remote.c | 43 +++++++++++++++++++++++---------------- t/for-each-ref-tests.sh | 4 +++- t/t1514-rev-parse-push.sh | 13 +++++++++++- 3 files changed, 41 insertions(+), 19 deletions(-)
base-commit: b9e5cff1803ab24582201ef059516eb16014b9d9
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2466%2FHaraldNordgren%2Fat-push-simple-triangular-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2466/HaraldNordgren/at-push-simple-triangular-v2
Pull-Request: https://github.com/git/git/pull/2466
Range-diff vs v1:
-: ---------- > 1: 151fef69b9 remote: move "simple" @{push} resolution into a helper
1: d2b1a00d23 ! 2: c58afcc4b1 remote: resolve "simple" @{push} when pushing to another remote@@ Commit message
## remote.c ##
-@@ remote.c: static char *branch_get_push_1(struct repository *repo,
- const char *up;
- char *cur;
+@@ remote.c: static char *branch_get_push_simple(struct repository *repo,
+ struct branch *branch,
+ struct strbuf *err)
+ {
+- const char *up;
++ const char *up = NULL;
+ 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;
+- up = branch_get_upstream(branch, err);
+- if (!up)
+- return NULL;
++ if (!strcmp(remote->name,
++ remotes_remote_for_branch(repo->remote_state, branch, NULL))) {
++ up = branch_get_upstream(branch, err);
++ if (!up)
++ return NULL;
++ }
+ cur = tracking_for_push_dest(repo, remote, branch->refname, err);
+ if (!cur)
+ return NULL;
+- if (strcmp(cur, up)) {
++ if (up && strcmp(cur, up)) {
+ free(cur);
+ return error_buf(err,
+ _("cannot resolve 'simple' push to a single destination"));
## t/for-each-ref-tests.sh ##
@@ t/for-each-ref-tests.sh: test_expect_success ':remotename and :remoteref' '
@@ t/for-each-ref-tests.sh: test_expect_success ':remotename and :remoteref' '
test_cmp expect actual
## t/t1514-rev-parse-push.sh ##
+@@ t/t1514-rev-parse-push.sh: test_expect_success '@{push} with default=simple' '
+ resolve main@{pUSh} refs/remotes/origin/main
+ '
+
+-test_expect_success 'triangular @{push} fails with default=simple' ' ++test_expect_success '@{push} with default=simple fails when upstream has a different name' '
+ test_config push.default simple &&
+ test_must_fail git rev-parse topic@{push}
+ '
@@ t/t1514-rev-parse-push.sh: test_expect_success '@{push} with pushremote defined' '
resolve topic@{push} refs/remotes/other/topic
'
Signature · 1 line
gitgitgadget
PATCH v2 1/2 remote: move "simple" @{push} resolution into a helper
From: Harald Nordgren <[email protected]>
The "simple" arm is the longest in the switch of branch_get_push_1() and needs an extra block to declare its variables. Give it its own function so that it reads at one level of indentation and the next commit can teach it about pushing to another remote without nesting it further.
No functional change.
--- remote.c | 40 +++++++++++++++++++++++----------------- 1 file changed, 23 insertions(+), 17 deletions(-)
remote.c +23 -17
@@ -1967,6 +1967,28 @@ static char *tracking_for_push_dest(struct repository *repo,return ret; } +static char *branch_get_push_simple (struct repository *repo, +struct remote *remote, +struct branch *branch, +struct strbuf *err) +{ +const char *up; +char *cur; + + up =branch_get_upstream (branch, err); +if (!up) +return NULL ; + cur =tracking_for_push_dest (repo, remote, branch->refname, err); +if (!cur) +return NULL ; +if (strcmp (cur, up)) { +free (cur); +return error_buf (err, +_ ("cannot resolve 'simple' push to a single destination" )); + } +return cur; +} +static char *branch_get_push_1 (struct repository *repo,struct branch *branch,struct strbuf *err) {
@@ -2012,23 +2034,7 @@ static char *branch_get_push_1(struct repository *repo,case PUSH_DEFAULT_UNSPECIFIED :case PUSH_DEFAULT_SIMPLE : - { -const char *up; -char *cur; - - up =branch_get_upstream (branch, err); -if (!up) -return NULL ; - cur =tracking_for_push_dest (repo, remote, branch->refname, err); -if (!cur) -return NULL ; -if (strcmp (cur, up)) { -free (cur); -return error_buf (err, -_ ("cannot resolve 'simple' push to a single destination" )); - } -return cur; - } +return branch_get_push_simple (repo, remote, branch, err); }BUG ("unhandled push situation" );
Signature · 1 line
gitgitgadget
PATCH v2 2/2 remote: resolve "simple" @{push} when pushing to another remote
In reply to Harald Nordgren via GitGitGadgetFrom: Harald Nordgren <[email protected]>
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.
--- remote.c | 13 ++++++++----- t/for-each-ref-tests.sh | 4 +++- t/t1514-rev-parse-push.sh | 13 ++++++++++++- 3 files changed, 23 insertions(+), 7 deletions(-)
remote.c +8 -5
@@ -1972,16 +1972,19 @@ static char *branch_get_push_simple(struct repository *repo,struct branch *branch, struct strbuf *err) { - const char *up; + const char *up = NULL;char *cur; - up =branch_get_upstream (branch, err); -if (!up) -return NULL ; +if (!strcmp (remote->name, +remotes_remote_for_branch (repo->remote_state, branch,NULL ))) { + up =branch_get_upstream (branch, err); +if (!up) +return NULL ; + } cur =tracking_for_push_dest (repo, remote, branch->refname, err);if (!cur)return NULL ; -if (strcmp (cur, up)) { +if (up &&strcmp (cur, up)) {free (cur);return error_buf (err,_ ("cannot resolve 'simple' push to a single destination" ));
t/for-each-ref-tests.sh +3 -1
@@ -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
t/t1514-rev-parse-push.sh +12 -1
@@ -39,7 +39,7 @@ test_expect_success '@{push} with default=simple' ' resolve main@{pUSh} refs/remotes/origin/main ' -test_expect_success 'triangular @{push} fails with default=simple' ' +test_expect_success '@{push} with default=simple fails when upstream has a different name' ' test_config push.default simple && test_must_fail git rev-parse topic@{push} '
@@ -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 &&
Signature · 1 line
gitgitgadget
Quoted 11 lines · Harald Nordgren via GitGitGadget
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.