[PATCH] push: Alias pushurl from push rewrites

[PATCH] push: Alias pushurl from push rewrites

[Rob Hoelz]

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(-)

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

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

[Jonathan Nieder]

Hi,

Rob Hoelz wrote:

> [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:

[...]
> --- 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.

> +	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(-)

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

[Jonathan Nieder]

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(-)

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

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

[Jonathan Nieder]

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(-)

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

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

[Jonathan Nieder]

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(-)

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

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

[Junio C Hamano]

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?

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

[Jonathan Nieder]

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.

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

[Junio C Hamano]

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.

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

[Rob Hoelz]

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?

Thanks,
Rob

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

[Junio C Hamano]

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

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

[Rob Hoelz]

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

> 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.