{"thread":{"id":"44394","subject":"[PATCH] push: do not use potentially ambiguous default refspec","startedAt":"2016-10-28T19:25:42Z","lastAt":"2016-10-31T21:11:16Z","messageCount":3,"participants":["Junio C Hamano","Dennis Kaarsemaker"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"305119","messageId":"xmqqvawcz36d.fsf@gitster.mtv.corp.google.com","threadId":"44394","inReplyTo":null,"subject":"[PATCH] push: do not use potentially ambiguous default refspec","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-28T19:25:30Z","receivedAt":"2016-10-28T19:25:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When the user does the lazy \"git push\" with no parameters with\npush.default set to either \"upstream\", \"simple\" or \"current\",\nwe internally generated a refspec that has the current branch name\non the source side and used it to push.\n\nHowever, the branch name (say \"test\") may be an ambiguous refname in\nthe context of the source repository---there may be a tag with the\nsame name, for example. This would trigger an unnecessary error\nwithout any fault on the end-user's side.\n\nBe explicit and give a full refname as the source side to avoid the\nambiguity. The destination side when pushing with the \"current\"\nsent only the name of the branch and forcing the receiving end to\nguess, which is the same issue. Be explicit there as well.\n\nReported-by: Kannan Goundan \nSigned-off-by: Junio C Hamano \n---\n\n * It is appreciated if somebody with spare cycles can add a test or\n two for this in t/t5523-push-upstream.sh or somewhere nearby.\n\n builtin/push.c | 7 +++++--\n 1 file changed, 5 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/push.c b/builtin/push.c\nindex 4e9e4dbab2..00a1b68483 100644\n--- a/builtin/push.c\n+++ b/builtin/push.c\n@@ -194,15 +194,18 @@ static void setup_push_upstream(struct remote *remote, struct branch *branch,\n \t\t\tdie_push_simple(branch, remote);\n \t}\n \n-\tstrbuf_addf(&refspec, \"%s:%s\", branch->name, branch->merge[0]->src);\n+\tstrbuf_addf(&refspec, \"%s:%s\", branch->refname, branch->merge[0]->src);\n \tadd_refspec(refspec.buf);\n }\n \n static void setup_push_current(struct remote *remote, struct branch *branch)\n {\n+\tstruct strbuf refspec = STRBUF_INIT;\n+\n \tif (!branch)\n \t\tdie(_(message_detached_head_die), remote->name);\n-\tadd_refspec(branch->name);\n+\tstrbuf_addf(&refspec, \"%s:%s\", branch->refname, branch->refname);\n+\tadd_refspec(refspec.buf);\n }\n \n static int is_workflow_triangular(struct remote *remote)\n-- \n2.10.2-722-gd3f6888e25\n\n"},{"id":"305204","messageId":"20161031203834.dfavyjzkob2goa5n@hurricane","threadId":"44394","inReplyTo":"xmqqvawcz36d.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] push: do not use potentially ambiguous default refspec","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2016-10-31T20:38:35Z","receivedAt":"2016-10-31T20:38:45Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"On Fri, Oct 28, 2016 at 12:25:30PM -0700, Junio C Hamano wrote:\n\n> * It is appreciated if somebody with spare cycles can add a test or\n> two for this in t/t5523-push-upstream.sh or somewhere nearby.\n\n5523 is for push --set-upstream-to, 5528 seemed more appropriate. Here's\nsomething squashable that fails before your patch and succeeds after.\n\n>8----\nSubject: [PATCH] push: test pushing ambiguously named branches\n\nSigned-off-by: Dennis Kaarsemaker \n---\n t/t5528-push-default.sh | 10 ++++++++++\n 1 file changed, 10 insertions(+)\n\ndiff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh\nindex 73f4bb6..ac103ce 100755\n--- a/t/t5528-push-default.sh\n+++ b/t/t5528-push-default.sh\n@@ -98,6 +98,16 @@ test_expect_success 'push from/to new branch with upstream, matching and simple'\n \ttest_push_failure upstream\n '\n \n+test_expect_success 'push ambiguously named branch with upstream, matching and simple' '\n+\tgit checkout -b ambiguous &&\n+\ttest_config branch.ambiguous.remote parent1 &&\n+\ttest_config branch.ambiguous.merge refs/heads/ambiguous &&\n+\tgit tag ambiguous &&\n+\ttest_push_success simple ambiguous &&\n+\ttest_push_success matching ambiguous &&\n+\ttest_push_success upstream ambiguous\n+'\n+\n test_expect_success 'push from/to new branch with current creates remote branch' '\n \ttest_config branch.new-branch.remote repo1 &&\n \tgit checkout new-branch &&\n-- \n2.10.1-449-gab0f84c\n"},{"id":"305206","messageId":"xmqqk2corzq5.fsf@gitster.mtv.corp.google.com","threadId":"44394","inReplyTo":"20161031203834.dfavyjzkob2goa5n@hurricane","subject":"Re: [PATCH] push: do not use potentially ambiguous default refspec","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-10-31T21:10:58Z","receivedAt":"2016-10-31T21:11:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dennis Kaarsemaker writes:\n\n> On Fri, Oct 28, 2016 at 12:25:30PM -0700, Junio C Hamano wrote:\n>\n>> * It is appreciated if somebody with spare cycles can add a test or\n>> two for this in t/t5523-push-upstream.sh or somewhere nearby.\n>\n> 5523 is for push --set-upstream-to, 5528 seemed more appropriate. Here's\n> something squashable that fails before your patch and succeeds after.\n\nThanks.\n\n>\n>>8----\n> Subject: [PATCH] push: test pushing ambiguously named branches\n>\n> Signed-off-by: Dennis Kaarsemaker \n> ---\n> t/t5528-push-default.sh | 10 ++++++++++\n> 1 file changed, 10 insertions(+)\n>\n> diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh\n> index 73f4bb6..ac103ce 100755\n> --- a/t/t5528-push-default.sh\n> +++ b/t/t5528-push-default.sh\n> @@ -98,6 +98,16 @@ test_expect_success 'push from/to new branch with upstream, matching and simple'\n> \ttest_push_failure upstream\n> '\n> \n> +test_expect_success 'push ambiguously named branch with upstream, matching and simple' '\n> +\tgit checkout -b ambiguous &&\n> +\ttest_config branch.ambiguous.remote parent1 &&\n> +\ttest_config branch.ambiguous.merge refs/heads/ambiguous &&\n> +\tgit tag ambiguous &&\n> +\ttest_push_success simple ambiguous &&\n> +\ttest_push_success matching ambiguous &&\n> +\ttest_push_success upstream ambiguous\n> +'\n> +\n> test_expect_success 'push from/to new branch with current creates remote branch' '\n> \ttest_config branch.new-branch.remote repo1 &&\n> \tgit checkout new-branch &&\n"}]}