All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] push: do not use potentially ambiguous default refspec
@ 2016-10-28 19:25 Junio C Hamano
  2016-10-31 20:38 ` Dennis Kaarsemaker
  0 siblings, 1 reply; 3+ messages in thread
From: Junio C Hamano @ 2016-10-28 19:25 UTC (permalink / raw)
  To: git; +Cc: Kannan Goundan

When the user does the lazy "git push" with no parameters with
push.default set to either "upstream", "simple" or "current",
we internally generated a refspec that has the current branch name
on the source side and used it to push.

However, the branch name (say "test") may be an ambiguous refname in
the context of the source repository---there may be a tag with the
same name, for example.  This would trigger an unnecessary error
without any fault on the end-user's side.

Be explicit and give a full refname as the source side to avoid the
ambiguity.  The destination side when pushing with the "current"
sent only the name of the branch and forcing the receiving end to
guess, which is the same issue.  Be explicit there as well.

Reported-by: Kannan Goundan <kannan@cakoose.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * It is appreciated if somebody with spare cycles can add a test or
   two for this in t/t5523-push-upstream.sh or somewhere nearby.

 builtin/push.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/builtin/push.c b/builtin/push.c
index 4e9e4dbab2..00a1b68483 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -194,15 +194,18 @@ static void setup_push_upstream(struct remote *remote, struct branch *branch,
 			die_push_simple(branch, remote);
 	}
 
-	strbuf_addf(&refspec, "%s:%s", branch->name, branch->merge[0]->src);
+	strbuf_addf(&refspec, "%s:%s", branch->refname, branch->merge[0]->src);
 	add_refspec(refspec.buf);
 }
 
 static void setup_push_current(struct remote *remote, struct branch *branch)
 {
+	struct strbuf refspec = STRBUF_INIT;
+
 	if (!branch)
 		die(_(message_detached_head_die), remote->name);
-	add_refspec(branch->name);
+	strbuf_addf(&refspec, "%s:%s", branch->refname, branch->refname);
+	add_refspec(refspec.buf);
 }
 
 static int is_workflow_triangular(struct remote *remote)
-- 
2.10.2-722-gd3f6888e25


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

* Re: [PATCH] push: do not use potentially ambiguous default refspec
  2016-10-28 19:25 [PATCH] push: do not use potentially ambiguous default refspec Junio C Hamano
@ 2016-10-31 20:38 ` Dennis Kaarsemaker
  2016-10-31 21:10   ` Junio C Hamano
  0 siblings, 1 reply; 3+ messages in thread
From: Dennis Kaarsemaker @ 2016-10-31 20:38 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Kannan Goundan

On Fri, Oct 28, 2016 at 12:25:30PM -0700, Junio C Hamano wrote:

>  * It is appreciated if somebody with spare cycles can add a test or
>    two for this in t/t5523-push-upstream.sh or somewhere nearby.

5523 is for push --set-upstream-to, 5528 seemed more appropriate. Here's
something squashable that fails before your patch and succeeds after.

>8----
Subject: [PATCH] push: test pushing ambiguously named branches

Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>
---
 t/t5528-push-default.sh | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
index 73f4bb6..ac103ce 100755
--- a/t/t5528-push-default.sh
+++ b/t/t5528-push-default.sh
@@ -98,6 +98,16 @@ test_expect_success 'push from/to new branch with upstream, matching and simple'
 	test_push_failure upstream
 '
 
+test_expect_success 'push ambiguously named branch with upstream, matching and simple' '
+	git checkout -b ambiguous &&
+	test_config branch.ambiguous.remote parent1 &&
+	test_config branch.ambiguous.merge refs/heads/ambiguous &&
+	git tag ambiguous &&
+	test_push_success simple ambiguous &&
+	test_push_success matching ambiguous &&
+	test_push_success upstream ambiguous
+'
+
 test_expect_success 'push from/to new branch with current creates remote branch' '
 	test_config branch.new-branch.remote repo1 &&
 	git checkout new-branch &&
-- 
2.10.1-449-gab0f84c

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

* Re: [PATCH] push: do not use potentially ambiguous default refspec
  2016-10-31 20:38 ` Dennis Kaarsemaker
@ 2016-10-31 21:10   ` Junio C Hamano
  0 siblings, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2016-10-31 21:10 UTC (permalink / raw)
  To: Dennis Kaarsemaker; +Cc: git, Kannan Goundan

Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:

> On Fri, Oct 28, 2016 at 12:25:30PM -0700, Junio C Hamano wrote:
>
>>  * It is appreciated if somebody with spare cycles can add a test or
>>    two for this in t/t5523-push-upstream.sh or somewhere nearby.
>
> 5523 is for push --set-upstream-to, 5528 seemed more appropriate. Here's
> something squashable that fails before your patch and succeeds after.

Thanks.

>
>>8----
> Subject: [PATCH] push: test pushing ambiguously named branches
>
> Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>
> ---
>  t/t5528-push-default.sh | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/t/t5528-push-default.sh b/t/t5528-push-default.sh
> index 73f4bb6..ac103ce 100755
> --- a/t/t5528-push-default.sh
> +++ b/t/t5528-push-default.sh
> @@ -98,6 +98,16 @@ test_expect_success 'push from/to new branch with upstream, matching and simple'
>  	test_push_failure upstream
>  '
>  
> +test_expect_success 'push ambiguously named branch with upstream, matching and simple' '
> +	git checkout -b ambiguous &&
> +	test_config branch.ambiguous.remote parent1 &&
> +	test_config branch.ambiguous.merge refs/heads/ambiguous &&
> +	git tag ambiguous &&
> +	test_push_success simple ambiguous &&
> +	test_push_success matching ambiguous &&
> +	test_push_success upstream ambiguous
> +'
> +
>  test_expect_success 'push from/to new branch with current creates remote branch' '
>  	test_config branch.new-branch.remote repo1 &&
>  	git checkout new-branch &&

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

end of thread, other threads:[~2016-10-31 21:11 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-28 19:25 [PATCH] push: do not use potentially ambiguous default refspec Junio C Hamano
2016-10-31 20:38 ` Dennis Kaarsemaker
2016-10-31 21:10   ` Junio C Hamano

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.