threads / patch / 33222

patchpush: Alias pushurl from push rewrites

Subject: [PATCH] push: Alias pushurl from push rewrites

## tl;dr

11 messages between Mar 18, 2013 and Mar 27, 2013. Diffs are folded; open one to read it.

replies: 10people: 3as markdown or json

Rob Hoelz· Mar 18, 2013, 21:02 UTC · lore

git push currently doesn't consider pushInsteadOf when using pushurl; this test tests that.

If you use pushurl with an alias that has a pushInsteadOf configuration value, Git does not take advantage of it. For example:

[url "git://github.com/"]
    insteadOf = github:
[url "git://github.com/myuser/"]
    insteadOf = mygithub:
[url "git@github.com:myuser/"]
    pushInsteadOf = mygithub:
[remote "origin"]
    url     = github:organization/project
    pushurl = mygithub:project
With the above configuration, the following occurs:
$ git push origin master
fatal: remote error:
  You can't push to git://github.com/myuser/project.git
  Use git@github.com:myuser/project.git

So you can see that pushurl is being followed (it's not attempting to push to git://github.com/organization/project), but insteadOf values are being used as opposed to pushInsteadOf values for expanding the pushurl alias.

This commit fixes that.
Signed-off-by: Rob Hoelz <rob@hoelz.ro>
---
 remote.c              |  2 +-
 t/t5516-fetch-push.sh | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 1 deletion(-)
Show changes to 2 files +82 −1

remote.c, t/t5516-fetch-push.sh

diff --git a/remote.c b/remote.c
index ca1f8f2..de7a915 100644
--- a/remote.c
+++ b/remote.c
@@ -465,7 +465,7 @@ static void alias_all_urls(void)
 		if (!remotes[i])
 			continue;
 		for (j = 0; j < remotes[i]->pushurl_nr; j++) {
-			remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites);
+			remotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites_push);
 		}
 		add_pushurl_aliases = remotes[i]->pushurl_nr == 0;
 		for (j = 0; j < remotes[i]->url_nr; j++) {
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b5417cc..709f506 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -244,6 +244,87 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf
 	)
 '
 
+test_expect_success 'push with pushInsteadOf and explicit pushurl (pushurl + pushInsteadOf does rewrite in this case)' '
+	mk_empty &&
+	rm -rf ro rw &&
+	TRASH="$(pwd)/" &&
+	mkdir ro &&
+	mkdir rw &&
+	git init --bare rw/testrepo &&
+	git config "url.file://$TRASH/ro/.insteadOf" ro: &&
+	git config "url.file://$TRASH/rw/.pushInsteadOf" rw: &&
+	git config remote.r.url ro:wrong &&
+	git config remote.r.pushurl rw:testrepo &&
+	git push r refs/heads/master:refs/remotes/origin/master &&
+	(
+		cd rw/testrepo &&
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+	)
+'
+
+test_expect_success 'push without pushInsteadOf and explicit pushurl (pushurl + insteadOf is used for rewrite)' '
+	mk_empty &&
+	rm -rf ro rw &&
+	TRASH="$(pwd)/" &&
+	mkdir ro &&
+	mkdir rw &&
+	git init --bare rw/testrepo &&
+	git config "url.file://$TRASH/ro/.insteadOf" ro: &&
+	git config "url.file://$TRASH/rw/.insteadOf" rw: &&
+	git config remote.r.url ro:wrong &&
+	git config remote.r.pushurl rw:testrepo &&
+	git push r refs/heads/master:refs/remotes/origin/master &&
+	(
+		cd rw/testrepo &&
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+	)
+'
+
+test_expect_success 'push with pushInsteadOf but without explicit pushurl (url + pushInsteadOf is used for rewrite)' '
+	mk_empty &&
+	rm -rf ro rw &&
+	TRASH="$(pwd)/" &&
+	mkdir ro &&
+	mkdir rw &&
+	git init --bare rw/testrepo &&
+	git config "url.file://$TRASH/ro/.insteadOf" rw: &&
+	git config "url.file://$TRASH/rw/.pushInsteadOf" rw: &&
+	git config remote.r.url rw:testrepo &&
+	git push r refs/heads/master:refs/remotes/origin/master &&
+	(
+		cd rw/testrepo &&
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+	)
+'
+
+test_expect_success 'push without pushInsteadOf and without explicit pushurl (url + insteadOf is used for rewrite)' '
+	mk_empty &&
+	rm -rf ro rw &&
+	TRASH="$(pwd)/" &&
+	mkdir ro &&
+	mkdir rw &&
+	git init --bare rw/testrepo &&
+	git config "url.file://$TRASH/ro/.insteadOf" rw: &&
+	git config remote.r.url rw:testrepo &&
+	git push r refs/heads/master:refs/remotes/origin/master &&
+	(
+		cd rw/testrepo &&
+		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
+		test "z$r" = "z$the_commit" &&
+
+		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+	)
+'
+
 test_expect_success 'push with matching heads' '
 
 	mk_test heads/master &&

-- 
1.8.2
Jonathan Nieder· Mar 18, 2013, 23:10 UTC · re: Rob Hoelz · lore

Re: [PATCH] push: Alias pushurl from push rewrites

Hi,
Rob Hoelz wrote:
Show 21 quoted lines
> [url "git://github.com/"]
>     insteadOf = github:
> [url "git://github.com/myuser/"]
>     insteadOf = mygithub:
> [url "git@github.com:myuser/"]
>     pushInsteadOf = mygithub:
> [remote "origin"]
>     url     = github:organization/project
>     pushurl = mygithub:project
>
> With the above configuration, the following occurs:
>
> $ git push origin master
> fatal: remote error:
>   You can't push to git://github.com/myuser/project.git
>   Use git@github.com:myuser/project.git
>
> So you can see that pushurl is being followed (it's not attempting to
> push to git://github.com/organization/project), but insteadOf values are
> being used as opposed to pushInsteadOf values for expanding the pushurl
> alias.

At first glance it is not always obvious how overlapping settings like these should interact. Thanks for an instructive example that makes the right behavior obvious.

Test nits:
[...]
Show 15 quoted lines
> --- a/t/t5516-fetch-push.sh
> +++ b/t/t5516-fetch-push.sh
> @@ -244,6 +244,87 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf
>  	)
>  '
>  
> +test_expect_success 'push with pushInsteadOf and explicit pushurl (pushurl + pushInsteadOf does rewrite in this case)' '
> +	mk_empty &&
> +	rm -rf ro rw &&
> +	TRASH="$(pwd)/" &&
> +	mkdir ro &&
> +	mkdir rw &&
> +	git init --bare rw/testrepo &&
> +	git config "url.file://$TRASH/ro/.insteadOf" ro: &&
> +	git config "url.file://$TRASH/rw/.pushInsteadOf" rw: &&

The surrounding tests don't do this, but I wonder if it would make sense to use test_config instead of 'git config' here.

That way, the test's settings wouldn't affect other tests, and in particular if someone later decides to refactor the file by reordering tests, she could be confident that that would not break anything.

In most of the surrounding tests it does not matter because 'git config' is run in a subdirectory that is not reused later. Patches fixing the exceptions below.

Show 10 quoted lines
> +	git config remote.r.url ro:wrong &&
> +	git config remote.r.pushurl rw:testrepo &&
> +	git push r refs/heads/master:refs/remotes/origin/master &&
> +	(
> +		cd rw/testrepo &&
> +		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
> +		test "z$r" = "z$the_commit" &&
> +
> +		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
> +	)

To produce more useful "./t5516-fetch-push.sh -v -i" output when the comparison fails:

	echo "$the_commit commit refs/remotes/origin/master" >expect &&
	(
		cd rw/testrepo &&
		git for-each-ref refs/remotes/origin
	) >actual &&
	test_cmp expect actual
Hope that helps,
Jonathan Nieder (3):
  push test: use test_config where appropriate
  push test: simplify check of push result
  push test: rely on &&-chaining instead of 'if bad; then echo Oops; fi'
 t/t5516-fetch-push.sh | 156 +++++++++++++++++++++-----------------------------
 1 file changed, 65 insertions(+), 91 deletions(-)
Jonathan Nieder· Mar 18, 2013, 23:12 UTC · re: Jonathan Nieder · lore

[PATCH 1/3] push test: use test_config when appropriate

Configuration from test_config does not last beyond the end of the current test assertion, making each test easier to think about in isolation.

This changes the meaning of some of the tests. For example, currently "push with insteadOf" passes even if the line setting "url.$TRASH.pushInsteadOf" is dropped because an url.$TRASH.insteadOf setting leaks in from a previous test.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t5516-fetch-push.sh | 27 ++++++++++-----------------
 1 file changed, 10 insertions(+), 17 deletions(-)
Show changes to t/t5516-fetch-push.sh +10 −17
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index c31e5c1c..5b89c111 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -203,7 +203,7 @@ test_expect_success 'push with wildcard' '
 test_expect_success 'push with insteadOf' '
 	mk_empty &&
 	TRASH="$(pwd)/" &&
-	git config "url.$TRASH.insteadOf" trash/ &&
+	test_config "url.$TRASH.insteadOf" trash/ &&
 	git push trash/testrepo refs/heads/master:refs/remotes/origin/master &&
 	(
 		cd testrepo &&
@@ -217,7 +217,7 @@ test_expect_success 'push with insteadOf' '
 test_expect_success 'push with pushInsteadOf' '
 	mk_empty &&
 	TRASH="$(pwd)/" &&
-	git config "url.$TRASH.pushInsteadOf" trash/ &&
+	test_config "url.$TRASH.pushInsteadOf" trash/ &&
 	git push trash/testrepo refs/heads/master:refs/remotes/origin/master &&
 	(
 		cd testrepo &&
@@ -231,9 +231,9 @@ test_expect_success 'push with pushInsteadOf' '
 test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf should not rewrite)' '
 	mk_empty &&
 	TRASH="$(pwd)/" &&
-	git config "url.trash2/.pushInsteadOf" trash/ &&
-	git config remote.r.url trash/wrong &&
-	git config remote.r.pushurl "$TRASH/testrepo" &&
+	test_config "url.trash2/.pushInsteadOf" trash/ &&
+	test_config remote.r.url trash/wrong &&
+	test_config remote.r.pushurl "$TRASH/testrepo" &&
 	git push r refs/heads/master:refs/remotes/origin/master &&
 	(
 		cd testrepo &&
@@ -489,31 +489,24 @@ test_expect_success 'push with config remote.*.push = HEAD' '
 		git checkout local &&
 		git reset --hard $the_first_commit
 	) &&
-	git config remote.there.url testrepo &&
-	git config remote.there.push HEAD &&
-	git config branch.master.remote there &&
+	test_config remote.there.url testrepo &&
+	test_config remote.there.push HEAD &&
+	test_config branch.master.remote there &&
 	git push &&
 	check_push_result $the_commit heads/master &&
 	check_push_result $the_first_commit heads/local
 '
 
-# clean up the cruft left with the previous one
-git config --remove-section remote.there
-git config --remove-section branch.master
-
 test_expect_success 'push with config remote.*.pushurl' '
 
 	mk_test heads/master &&
 	git checkout master &&
-	git config remote.there.url test2repo &&
-	git config remote.there.pushurl testrepo &&
+	test_config remote.there.url test2repo &&
+	test_config remote.there.pushurl testrepo &&
 	git push there &&
 	check_push_result $the_commit heads/master
 '
 
-# clean up the cruft left with the previous one
-git config --remove-section remote.there
-
 test_expect_success 'push with dry-run' '
 
 	mk_test heads/master &&

-- 
1.8.2.rc3
Jonathan Nieder· Mar 18, 2013, 23:13 UTC · re: Jonathan Nieder · lore

[PATCH 2/3] push test: simplify check of push result

This test checks each ref with code like the following:
	r=$(git show-ref -s --verify refs/$ref) &&
	test "z$r" = "z$the_first_commit"
Afterward it counts refs:
	test 1 = $(git for-each-ref refs/remotes/origin | wc -l)

Simpler to test the number and values of relevant refs in for-each-ref output at the same time using test_cmp. This makes the test more readable and provides more helpful "./t5516-push-push.sh -v" output when the test fails.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t5516-fetch-push.sh | 114 ++++++++++++++++++++++----------------------------
 1 file changed, 51 insertions(+), 63 deletions(-)
Show changes to t/t5516-fetch-push.sh +51 −63
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 5b89c111..2f1255d4 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -30,11 +30,10 @@ mk_test () {
 		cd testrepo &&
 		for ref in "$@"
 		do
-			r=$(git show-ref -s --verify refs/$ref) &&
-			test "z$r" = "z$the_first_commit" || {
-				echo "Oops, refs/$ref is wrong"
-				exit 1
-			}
+			echo "$the_first_commit" >expect &&
+			git show-ref -s --verify refs/$ref >actual &&
+			test_cmp expect actual ||
+			exit
 		done &&
 		git fsck --full
 	)
@@ -82,15 +81,13 @@ mk_child() {
 check_push_result () {
 	(
 		cd testrepo &&
-		it="$1" &&
-		shift
+		echo "$1" >expect &&
+		shift &&
 		for ref in "$@"
 		do
-			r=$(git show-ref -s --verify refs/$ref) &&
-			test "z$r" = "z$it" || {
-				echo "Oops, refs/$ref is wrong"
-				exit 1
-			}
+			git show-ref -s --verify refs/$ref >actual &&
+			test_cmp expect actual ||
+			exit
 		done &&
 		git fsck --full
 	)
@@ -118,10 +115,9 @@ test_expect_success 'fetch without wildcard' '
 		cd testrepo &&
 		git fetch .. refs/heads/master:refs/remotes/origin/master &&
 
-		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
-		test "z$r" = "z$the_commit" &&
-
-		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+		echo "$the_commit commit	refs/remotes/origin/master" >expect &&
+		git for-each-ref refs/remotes/origin >actual &&
+		test_cmp expect actual
 	)
 '
 
@@ -133,10 +129,9 @@ test_expect_success 'fetch with wildcard' '
 		git config remote.up.fetch "refs/heads/*:refs/remotes/origin/*" &&
 		git fetch up &&
 
-		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
-		test "z$r" = "z$the_commit" &&
-
-		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+		echo "$the_commit commit	refs/remotes/origin/master" >expect &&
+		git for-each-ref refs/remotes/origin >actual &&
+		test_cmp expect actual
 	)
 '
 
@@ -150,10 +145,9 @@ test_expect_success 'fetch with insteadOf' '
 		git config remote.up.fetch "refs/heads/*:refs/remotes/origin/*" &&
 		git fetch up &&
 
-		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
-		test "z$r" = "z$the_commit" &&
-
-		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+		echo "$the_commit commit	refs/remotes/origin/master" >expect &&
+		git for-each-ref refs/remotes/origin >actual &&
+		test_cmp expect actual
 	)
 '
 
@@ -167,10 +161,9 @@ test_expect_success 'fetch with pushInsteadOf (should not rewrite)' '
 		git config remote.up.fetch "refs/heads/*:refs/remotes/origin/*" &&
 		git fetch up &&
 
-		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
-		test "z$r" = "z$the_commit" &&
-
-		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+		echo "$the_commit commit	refs/remotes/origin/master" >expect &&
+		git for-each-ref refs/remotes/origin >actual &&
+		test_cmp expect actual
 	)
 '
 
@@ -180,10 +173,9 @@ test_expect_success 'push without wildcard' '
 	git push testrepo refs/heads/master:refs/remotes/origin/master &&
 	(
 		cd testrepo &&
-		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
-		test "z$r" = "z$the_commit" &&
-
-		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+		echo "$the_commit commit	refs/remotes/origin/master" >expect &&
+		git for-each-ref refs/remotes/origin >actual &&
+		test_cmp expect actual
 	)
 '
 
@@ -193,10 +185,9 @@ test_expect_success 'push with wildcard' '
 	git push testrepo "refs/heads/*:refs/remotes/origin/*" &&
 	(
 		cd testrepo &&
-		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
-		test "z$r" = "z$the_commit" &&
-
-		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+		echo "$the_commit commit	refs/remotes/origin/master" >expect &&
+		git for-each-ref refs/remotes/origin >actual &&
+		test_cmp expect actual
 	)
 '
 
@@ -207,10 +198,9 @@ test_expect_success 'push with insteadOf' '
 	git push trash/testrepo refs/heads/master:refs/remotes/origin/master &&
 	(
 		cd testrepo &&
-		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
-		test "z$r" = "z$the_commit" &&
-
-		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+		echo "$the_commit commit	refs/remotes/origin/master" >expect &&
+		git for-each-ref refs/remotes/origin >actual &&
+		test_cmp expect actual
 	)
 '
 
@@ -221,10 +211,9 @@ test_expect_success 'push with pushInsteadOf' '
 	git push trash/testrepo refs/heads/master:refs/remotes/origin/master &&
 	(
 		cd testrepo &&
-		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
-		test "z$r" = "z$the_commit" &&
-
-		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+		echo "$the_commit commit	refs/remotes/origin/master" >expect &&
+		git for-each-ref refs/remotes/origin >actual &&
+		test_cmp expect actual
 	)
 '
 
@@ -237,10 +226,9 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf
 	git push r refs/heads/master:refs/remotes/origin/master &&
 	(
 		cd testrepo &&
-		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
-		test "z$r" = "z$the_commit" &&
-
-		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+		echo "$the_commit commit	refs/remotes/origin/master" >expect &&
+		git for-each-ref refs/remotes/origin >actual &&
+		test_cmp expect actual
 	)
 '
 
@@ -827,9 +815,9 @@ test_expect_success 'fetch with branches' '
 	(
 		cd testrepo &&
 		git fetch branch1 &&
-		r=$(git show-ref -s --verify refs/heads/branch1) &&
-		test "z$r" = "z$the_commit" &&
-		test 1 = $(git for-each-ref refs/heads | wc -l)
+		echo "$the_commit commit	refs/heads/branch1" >expect &&
+		git for-each-ref refs/heads >actual &&
+		test_cmp expect actual
 	) &&
 	git checkout master
 '
@@ -840,9 +828,9 @@ test_expect_success 'fetch with branches containing #' '
 	(
 		cd testrepo &&
 		git fetch branch2 &&
-		r=$(git show-ref -s --verify refs/heads/branch2) &&
-		test "z$r" = "z$the_first_commit" &&
-		test 1 = $(git for-each-ref refs/heads | wc -l)
+		echo "$the_first_commit commit	refs/heads/branch2" >expect &&
+		git for-each-ref refs/heads >actual &&
+		test_cmp expect actual
 	) &&
 	git checkout master
 '
@@ -854,9 +842,9 @@ test_expect_success 'push with branches' '
 	git push branch1 &&
 	(
 		cd testrepo &&
-		r=$(git show-ref -s --verify refs/heads/master) &&
-		test "z$r" = "z$the_first_commit" &&
-		test 1 = $(git for-each-ref refs/heads | wc -l)
+		echo "$the_first_commit commit	refs/heads/master" >expect &&
+		git for-each-ref refs/heads >actual &&
+		test_cmp expect actual
 	)
 '
 
@@ -866,9 +854,9 @@ test_expect_success 'push with branches containing #' '
 	git push branch2 &&
 	(
 		cd testrepo &&
-		r=$(git show-ref -s --verify refs/heads/branch3) &&
-		test "z$r" = "z$the_first_commit" &&
-		test 1 = $(git for-each-ref refs/heads | wc -l)
+		echo "$the_first_commit commit	refs/heads/branch3" >expect &&
+		git for-each-ref refs/heads >actual &&
+		test_cmp expect actual
 	) &&
 	git checkout master
 '
@@ -951,9 +939,9 @@ test_expect_success 'push --porcelain' '
 	git push >.git/bar --porcelain  testrepo refs/heads/master:refs/remotes/origin/master &&
 	(
 		cd testrepo &&
-		r=$(git show-ref -s --verify refs/remotes/origin/master) &&
-		test "z$r" = "z$the_commit" &&
-		test 1 = $(git for-each-ref refs/remotes/origin | wc -l)
+		echo "$the_commit commit	refs/remotes/origin/master" >expect &&
+		git for-each-ref refs/remotes/origin >actual &&
+		test_cmp expect actual
 	) &&
 	test_cmp .git/foo .git/bar
 '

-- 
1.8.2.rc3
Jonathan Nieder· Mar 18, 2013, 23:14 UTC · re: Jonathan Nieder · lore

[PATCH 3/3] push test: rely on &&-chaining instead of 'if bad; then echo Oops; fi'

When it is unclear which command from a test has failed, usual practice these days is to debug by running the test again with "sh -x" instead of relying on debugging 'echo' statements.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t5516-fetch-push.sh | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)
Show changes to t/t5516-fetch-push.sh +4 −11
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 2f1255d4..0695d570 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -22,10 +22,8 @@ mk_test () {
 	(
 		for ref in "$@"
 		do
-			git push testrepo $the_first_commit:refs/$ref || {
-				echo "Oops, push refs/$ref failure"
-				exit 1
-			}
+			git push testrepo $the_first_commit:refs/$ref ||
+			exit
 		done &&
 		cd testrepo &&
 		for ref in "$@"
@@ -328,13 +326,8 @@ test_expect_success 'push with weak ambiguity (2)' '
 test_expect_success 'push with ambiguity' '
 
 	mk_test heads/frotz tags/frotz &&
-	if git push testrepo master:frotz
-	then
-		echo "Oops, should have failed"
-		false
-	else
-		check_push_result $the_first_commit heads/frotz tags/frotz
-	fi
+	test_must_fail git push testrepo master:frotz &&
+	check_push_result $the_first_commit heads/frotz tags/frotz
 
 '
 

-- 
1.8.2.rc3
Junio C Hamano· Mar 19, 2013, 01:46 UTC · re: Jonathan Nieder · lore

Re: [PATCH] push: Alias pushurl from push rewrites

Jonathan Nieder <jrnieder@gmail.com> writes:
Show 8 quoted lines
> Test nits:
> ...
> Hope that helps,
>
> Jonathan Nieder (3):
>   push test: use test_config where appropriate
>   push test: simplify check of push result
>   push test: rely on &&-chaining instead of 'if bad; then echo Oops; fi'

Are these supposed to be follow-up patches? Preparatory steps that Rob can reroll on top? Something else?

Jonathan Nieder· Mar 19, 2013, 01:55 UTC · re: Junio C Hamano · lore

Re: [PATCH] push: Alias pushurl from push rewrites

Junio C Hamano wrote:
> Jonathan Nieder <jrnieder@gmail.com> writes:
Show 11 quoted lines
>> Test nits:
>> ...
>> Hope that helps,
>>
>> Jonathan Nieder (3):
>>   push test: use test_config where appropriate
>>   push test: simplify check of push result
>>   push test: rely on &&-chaining instead of 'if bad; then echo Oops; fi'
>
> Are these supposed to be follow-up patches?  Preparatory steps that
> Rob can reroll on top?  Something else?
Preparatory steps.
Junio C Hamano· Mar 19, 2013, 18:08 UTC · re: Jonathan Nieder · lore

Re: [PATCH] push: Alias pushurl from push rewrites

Jonathan Nieder <jrnieder@gmail.com> writes:
Show 16 quoted lines
> Junio C Hamano wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>>> Test nits:
>>> ...
>>> Hope that helps,
>>>
>>> Jonathan Nieder (3):
>>>   push test: use test_config where appropriate
>>>   push test: simplify check of push result
>>>   push test: rely on &&-chaining instead of 'if bad; then echo Oops; fi'
>>
>> Are these supposed to be follow-up patches?  Preparatory steps that
>> Rob can reroll on top?  Something else?
>
> Preparatory steps.
OK, thanks.  I'll queue these first then.
Rob Hoelz· Mar 20, 2013, 12:33 UTC · re: Junio C Hamano · lore

Re: [PATCH] push: Alias pushurl from push rewrites

On 3/19/13 7:08 PM, Junio C Hamano wrote:
Show 17 quoted lines
> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Junio C Hamano wrote:
>>> Jonathan Nieder <jrnieder@gmail.com> writes:
>>>> Test nits:
>>>> ...
>>>> Hope that helps,
>>>>
>>>> Jonathan Nieder (3):
>>>>   push test: use test_config where appropriate
>>>>   push test: simplify check of push result
>>>>   push test: rely on &&-chaining instead of 'if bad; then echo Oops; fi'
>>> Are these supposed to be follow-up patches?  Preparatory steps that
>>> Rob can reroll on top?  Something else?
>> Preparatory steps.
> OK, thanks.  I'll queue these first then.
>

Should I apply these to my patch to move things along? What's the next step for me?

Thanks, Rob

Junio C Hamano· Mar 20, 2013, 14:35 UTC · re: Rob Hoelz · lore

Re: [PATCH] push: Alias pushurl from push rewrites

Rob Hoelz <rob@hoelz.ro> writes:
Show 20 quoted lines
> On 3/19/13 7:08 PM, Junio C Hamano wrote:
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>>
>>> Junio C Hamano wrote:
>>>> Jonathan Nieder <jrnieder@gmail.com> writes:
>>>>> Test nits:
>>>>> ...
>>>>> Hope that helps,
>>>>>
>>>>> Jonathan Nieder (3):
>>>>>   push test: use test_config where appropriate
>>>>>   push test: simplify check of push result
>>>>>   push test: rely on &&-chaining instead of 'if bad; then echo Oops; fi'
>>>> Are these supposed to be follow-up patches?  Preparatory steps that
>>>> Rob can reroll on top?  Something else?
>>> Preparatory steps.
>> OK, thanks.  I'll queue these first then.
>>
> Should I apply these to my patch to move things along?  What's the next
> step for me?

You would fetch from nearby git.git mirror, find the tip of Janathan's series and redo your patch on top. Perhaps the session would go like this:

    $ git fetch git://git.kernel.org/pub/scm/git/git.git/ pu
    $ git log --oneline --first-parent ..FETCH_HEAD | grep jn/push-tests
    83a072a Merge branch 'jn/push-tests' into pu
    $ git checkout -b rh/push-pushurl-pushinsteadof 83a072a
    ... redo the work, perhaps with combinations of:
    $ git cherry-pick -n $your_original_branch
    $ edit t/t5516-fetch-push.sh
    ... and then
    $ make test
    $ git commit
Rob Hoelz· Mar 27, 2013, 17:20 UTC · re: Junio C Hamano · lore

Re: [PATCH] push: Alias pushurl from push rewrites

On Wed, 20 Mar 2013 07:35:58 -0700 Junio C Hamano <gitster@pobox.com> wrote:

Show 39 quoted lines
> Rob Hoelz <rob@hoelz.ro> writes:
> 
> > On 3/19/13 7:08 PM, Junio C Hamano wrote:
> >> Jonathan Nieder <jrnieder@gmail.com> writes:
> >>
> >>> Junio C Hamano wrote:
> >>>> Jonathan Nieder <jrnieder@gmail.com> writes:
> >>>>> Test nits:
> >>>>> ...
> >>>>> Hope that helps,
> >>>>>
> >>>>> Jonathan Nieder (3):
> >>>>>   push test: use test_config where appropriate
> >>>>>   push test: simplify check of push result
> >>>>>   push test: rely on &&-chaining instead of 'if bad; then echo
> >>>>> Oops; fi'
> >>>> Are these supposed to be follow-up patches?  Preparatory steps
> >>>> that Rob can reroll on top?  Something else?
> >>> Preparatory steps.
> >> OK, thanks.  I'll queue these first then.
> >>
> > Should I apply these to my patch to move things along?  What's the
> > next step for me?
> 
> You would fetch from nearby git.git mirror, find the tip of
> Janathan's series and redo your patch on top.  Perhaps the session
> would go like this:
> 
>     $ git fetch git://git.kernel.org/pub/scm/git/git.git/ pu
>     $ git log --oneline --first-parent ..FETCH_HEAD | grep
> jn/push-tests 83a072a Merge branch 'jn/push-tests' into pu
>     $ git checkout -b rh/push-pushurl-pushinsteadof 83a072a
>     ... redo the work, perhaps with combinations of:
>     $ git cherry-pick -n $your_original_branch
>     $ edit t/t5516-fetch-push.sh
>     ... and then
>     $ make test
>     $ git commit
> 
Ok, changes applied.  New patch coming.

← back to recent threads