threads / patch / 54716

v3, 3 partsrefspec: make @ a synonym of HEAD

Subject: [PATCH v3 0/3] refspec: make @ a synonym of HEAD

## tl;dr

7 messages between Nov 25, 2020 and Nov 27, 2020. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Felipe Contreras· Nov 25, 2020, 23:34 UTC · lore

Basically $subject.

Since v2 the tests have been revamped at the behest of Junio.

Felipe Contreras (3):
  tests: push: improve cleanup of HEAD tests
  tests: push: trivial cleanup
  refspec: make @ a synonym of HEAD

 refspec.c             |   5 +-
 t/t5511-refspec.sh    |   2 +
 t/t5516-fetch-push.sh | 105 ++++++++++++++++++++++--------------------
 3 files changed, 61 insertions(+), 51 deletions(-)

-- 
2.29.2

Felipe Contreras· Nov 25, 2020, 23:34 UTC · re: Felipe Contreras · lore

[PATCH v3 1/3] tests: push: improve cleanup of HEAD tests

So that we are not left in an inconsistent state between them.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t5516-fetch-push.sh | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Show changes to t/t5516-fetch-push.sh +3 −1

diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index d11382f769..4077471937 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -449,6 +449,7 @@ test_expect_success 'push with HEAD nonexisting at remote' '
 
 	mk_test testrepo heads/master &&
 	git checkout -b local master &&
+	test_when_finished "git checkout master; git branch -D local" &&
 	git push testrepo HEAD &&
 	check_push_result testrepo $the_commit heads/local
 '
@@ -457,8 +458,8 @@ test_expect_success 'push with +HEAD' '
 
 	mk_test testrepo heads/master &&
 	git checkout master &&
-	git branch -D local &&
 	git checkout -b local &&
+	test_when_finished "git checkout master; git branch -D local" &&
 	git push testrepo master local &&
 	check_push_result testrepo $the_commit heads/master &&
 	check_push_result testrepo $the_commit heads/local &&
@@ -488,6 +489,7 @@ test_expect_success 'push with config remote.*.push = HEAD' '
 	mk_test testrepo heads/local &&
 	git checkout master &&
 	git branch -f local $the_commit &&
+	test_when_finished "git branch -D local" &&
 	(
 		cd testrepo &&
 		git checkout local &&

-- 
2.29.2

Felipe Contreras· Nov 25, 2020, 23:34 UTC · re: Felipe Contreras · lore

[PATCH v3 2/3] tests: push: trivial cleanup

No need to do two checkouts.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 t/t5516-fetch-push.sh | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Show changes to t/t5516-fetch-push.sh +1 −2

diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 4077471937..245eafb170 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -457,8 +457,7 @@ test_expect_success 'push with HEAD nonexisting at remote' '
 test_expect_success 'push with +HEAD' '
 
 	mk_test testrepo heads/master &&
-	git checkout master &&
-	git checkout -b local &&
+	git checkout -b local master &&
 	test_when_finished "git checkout master; git branch -D local" &&
 	git push testrepo master local &&
 	check_push_result testrepo $the_commit heads/master &&

-- 
2.29.2

Felipe Contreras· Nov 25, 2020, 23:34 UTC · re: Felipe Contreras · lore

[PATCH v3 3/3] refspec: make @ a synonym of HEAD

Since commit 9ba89f484e git learned how to push to a remote branch using the source @, for example:

  git push origin @:master

However, if the right-hand side is missing, the push fails:

  git push origin @

It is obvious what is the desired behavior, and allowing the push makes things more consistent.

Additionally, @:master now has the same semantics as HEAD:master.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 refspec.c             |   5 +-
 t/t5511-refspec.sh    |   2 +
 t/t5516-fetch-push.sh | 106 ++++++++++++++++++++++--------------------
 3 files changed, 61 insertions(+), 52 deletions(-)

Show changes to 3 files +61 −52

refspec.c, t/t5511-refspec.sh, t/t5516-fetch-push.sh

diff --git a/refspec.c b/refspec.c
index c49347c2d7..adbfb3283a 100644
--- a/refspec.c
+++ b/refspec.c
@@ -71,7 +71,10 @@ static int parse_refspec(struct refspec_item *item, const char *refspec, int fet
 	}
 
 	item->pattern = is_glob;
-	item->src = xstrndup(lhs, llen);
+	if (llen == 1 && *lhs == '@')
+		item->src = xstrdup("HEAD");
+	else
+		item->src = xstrndup(lhs, llen);
 	flags = REFNAME_ALLOW_ONELEVEL | (is_glob ? REFNAME_REFSPEC_PATTERN : 0);
 
 	if (item->negative) {
diff --git a/t/t5511-refspec.sh b/t/t5511-refspec.sh
index f541f30bc2..f808649de4 100755
--- a/t/t5511-refspec.sh
+++ b/t/t5511-refspec.sh
@@ -58,6 +58,8 @@ test_refspec fetch 'HEAD~4:refs/remotes/frotz/new'		invalid
 
 test_refspec push 'HEAD'
 test_refspec fetch 'HEAD'
+test_refspec push '@'
+test_refspec fetch '@'
 test_refspec push 'refs/heads/ nitfol'				invalid
 test_refspec fetch 'refs/heads/ nitfol'				invalid
 
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 245eafb170..0cc5688135 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -436,71 +436,75 @@ test_expect_success 'push ref expression with non-existent, incomplete dest' '
 
 '
 
-test_expect_success 'push with HEAD' '
+for head in HEAD @; do
 
-	mk_test testrepo heads/master &&
-	git checkout master &&
-	git push testrepo HEAD &&
-	check_push_result testrepo $the_commit heads/master
+	test_expect_success "push with $head" "
 
-'
+		mk_test testrepo heads/master &&
+		git checkout master &&
+		git push testrepo $head &&
+		check_push_result testrepo \$the_commit heads/master
 
-test_expect_success 'push with HEAD nonexisting at remote' '
+	"
 
-	mk_test testrepo heads/master &&
-	git checkout -b local master &&
-	test_when_finished "git checkout master; git branch -D local" &&
-	git push testrepo HEAD &&
-	check_push_result testrepo $the_commit heads/local
-'
+	test_expect_success "push with $head nonexisting at remote" "
 
-test_expect_success 'push with +HEAD' '
+		mk_test testrepo heads/master &&
+		git checkout -b local master &&
+		test_when_finished 'git checkout master; git branch -D local' &&
+		git push testrepo $head &&
+		check_push_result testrepo \$the_commit heads/local
+	"
 
-	mk_test testrepo heads/master &&
-	git checkout -b local master &&
-	test_when_finished "git checkout master; git branch -D local" &&
-	git push testrepo master local &&
-	check_push_result testrepo $the_commit heads/master &&
-	check_push_result testrepo $the_commit heads/local &&
+	test_expect_success "push with +$head" "
 
-	# Without force rewinding should fail
-	git reset --hard HEAD^ &&
-	test_must_fail git push testrepo HEAD &&
-	check_push_result testrepo $the_commit heads/local &&
+		mk_test testrepo heads/master &&
+		git checkout -b local master &&
+		test_when_finished 'git checkout master; git branch -D local' &&
+		git push testrepo master local &&
+		check_push_result testrepo \$the_commit heads/master &&
+		check_push_result testrepo \$the_commit heads/local &&
 
-	# With force rewinding should succeed
-	git push testrepo +HEAD &&
-	check_push_result testrepo $the_first_commit heads/local
+		# Without force rewinding should fail
+		git reset --hard $head^ &&
+		test_must_fail git push testrepo $head &&
+		check_push_result testrepo \$the_commit heads/local &&
 
-'
+		# With force rewinding should succeed
+		git push testrepo +$head &&
+		check_push_result testrepo \$the_first_commit heads/local
 
-test_expect_success 'push HEAD with non-existent, incomplete dest' '
+	"
 
-	mk_test testrepo &&
-	git checkout master &&
-	git push testrepo HEAD:branch &&
-	check_push_result testrepo $the_commit heads/branch
+	test_expect_success "push $head with non-existent, incomplete dest" "
 
-'
+		mk_test testrepo &&
+		git checkout master &&
+		git push testrepo $head:branch &&
+		check_push_result testrepo \$the_commit heads/branch
 
-test_expect_success 'push with config remote.*.push = HEAD' '
+	"
 
-	mk_test testrepo heads/local &&
-	git checkout master &&
-	git branch -f local $the_commit &&
-	test_when_finished "git branch -D local" &&
-	(
-		cd testrepo &&
-		git checkout local &&
-		git reset --hard $the_first_commit
-	) &&
-	test_config remote.there.url testrepo &&
-	test_config remote.there.push HEAD &&
-	test_config branch.master.remote there &&
-	git push &&
-	check_push_result testrepo $the_commit heads/master &&
-	check_push_result testrepo $the_first_commit heads/local
-'
+	test_expect_success "push with config remote.*.push = $head" "
+
+		mk_test testrepo heads/local &&
+		git checkout master &&
+		git branch -f local \$the_commit &&
+		test_when_finished 'git branch -D local' &&
+		(
+			cd testrepo &&
+			git checkout local &&
+			git reset --hard \$the_first_commit
+		) &&
+		test_config remote.there.url testrepo &&
+		test_config remote.there.push $head &&
+		test_config branch.master.remote there &&
+		git push &&
+		check_push_result testrepo \$the_commit heads/master &&
+		check_push_result testrepo \$the_first_commit heads/local
+	"
+
+done
 
 test_expect_success 'push with remote.pushdefault' '
 	mk_test up_repo heads/master &&

-- 
2.29.2

Junio C Hamano· Nov 25, 2020, 23:53 UTC · re: Felipe Contreras · lore

Re: [PATCH v3 3/3] refspec: make @ a synonym of HEAD

Felipe Contreras <felipe.contreras@gmail.com> writes:

Show 20 quoted lines

> Since commit 9ba89f484e git learned how to push to a remote branch using
> the source @, for example:
>
>   git push origin @:master
>
> However, if the right-hand side is missing, the push fails:
>
>   git push origin @
>
> It is obvious what is the desired behavior, and allowing the push makes
> things more consistent.
>
> Additionally, @:master now has the same semantics as HEAD:master.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  refspec.c             |   5 +-
>  t/t5511-refspec.sh    |   2 +
>  t/t5516-fetch-push.sh | 106 ++++++++++++++++++++++--------------------
>  3 files changed, 61 insertions(+), 52 deletions(-)

Sorry, but I cannot queue this version in today's pushout due to time constraints.

> -test_expect_success 'push with HEAD' '
> +for head in HEAD @; do

Style: replace "; " with a LF.

> +	test_expect_success "push with $head" "

Don't open the test body with double-quote; keep using the original single-quote and the reference to $head is resolved while running the test, not when the test body is formulated before eval'ed.

Two benefits are

 - (obvious) you do not need extra quoting, e.g. \$the_commit etc.

 - in this case it does not matter, but it works intuitively when
   the variable has $IFS whitespace in it.  If you write in the test
   body that is enclosed in a single-quote pair,

		git push testrepo "$head"

   would correctly give the contents of variable head as a single
   element in argv[] of "git push".

For the latter reason, it is encouraged to make it a habit to do so.

Thanks.

Junio C Hamano· Nov 25, 2020, 23:42 UTC · re: Felipe Contreras · lore

Re: [PATCH v3 0/3] refspec: make @ a synonym of HEAD

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Basically $subject.

Will replace what was on 'seen'.

Dscho, heads-up as this series will add more references of 'master' in the test suite.

Thanks.

Johannes Schindelin· Nov 27, 2020, 11:07 UTC · re: Junio C Hamano · lore

Re: [PATCH v3 0/3] refspec: make @ a synonym of HEAD

Hi Junio,

On Wed, 25 Nov 2020, Junio C Hamano wrote:

> Dscho, heads-up as this series will add more references of 'master' in
> the test suite.

Thanks for the heads-up, Dscho