{"thread":{"id":"33222","subject":"[PATCH] push: Alias pushurl from push rewrites","startedAt":"2013-03-18T21:02:24Z","lastAt":"2013-03-27T17:20:59Z","messageCount":11,"participants":["Rob Hoelz","Jonathan Nieder","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"211605","messageId":"20130318220224.3b23a381@hoelz.ro","threadId":"33222","inReplyTo":null,"subject":"[PATCH] push: Alias pushurl from push rewrites","fromName":"Rob Hoelz","fromEmail":"rob@hoelz.ro","sentAt":"2013-03-18T21:02:24Z","receivedAt":"2013-03-18T21:02:24Z","isPatch":true,"sender":{"key":"rob@hoelz.ro","avatar":"https://gravatar.com/avatar/c62607c6cef695f7a6cbd0f1f3ebd359e8c471af271581a6d7e29e3fab6fb788?d=mp&s=160"},"body":"git push currently doesn't consider pushInsteadOf when\nusing pushurl; this test tests that.\n\nIf you use pushurl with an alias that has a pushInsteadOf configuration\nvalue, Git does not take advantage of it.  For example:\n\n[url \"git://github.com/\"]\n    insteadOf = github:\n[url \"git://github.com/myuser/\"]\n    insteadOf = mygithub:\n[url \"git@github.com:myuser/\"]\n    pushInsteadOf = mygithub:\n[remote \"origin\"]\n    url     = github:organization/project\n    pushurl = mygithub:project\n\nWith the above configuration, the following occurs:\n\n$ git push origin master\nfatal: remote error:\n  You can't push to git://github.com/myuser/project.git\n  Use git@github.com:myuser/project.git\n\nSo you can see that pushurl is being followed (it's not attempting to\npush to git://github.com/organization/project), but insteadOf values are\nbeing used as opposed to pushInsteadOf values for expanding the pushurl\nalias.\n\nThis commit fixes that.\n\nSigned-off-by: Rob Hoelz <rob@hoelz.ro>\n---\n remote.c              |  2 +-\n t/t5516-fetch-push.sh | 81 +++++++++++++++++++++++++++++++++++++++++++++++++++\n 2 files changed, 82 insertions(+), 1 deletion(-)\n\ndiff --git a/remote.c b/remote.c\nindex ca1f8f2..de7a915 100644\n--- a/remote.c\n+++ b/remote.c\n@@ -465,7 +465,7 @@ static void alias_all_urls(void)\n \t\tif (!remotes[i])\n \t\t\tcontinue;\n \t\tfor (j = 0; j < remotes[i]->pushurl_nr; j++) {\n-\t\t\tremotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites);\n+\t\t\tremotes[i]->pushurl[j] = alias_url(remotes[i]->pushurl[j], &rewrites_push);\n \t\t}\n \t\tadd_pushurl_aliases = remotes[i]->pushurl_nr == 0;\n \t\tfor (j = 0; j < remotes[i]->url_nr; j++) {\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex b5417cc..709f506 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -244,6 +244,87 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf\n \t)\n '\n \n+test_expect_success 'push with pushInsteadOf and explicit pushurl (pushurl + pushInsteadOf does rewrite in this case)' '\n+\tmk_empty &&\n+\trm -rf ro rw &&\n+\tTRASH=\"$(pwd)/\" &&\n+\tmkdir ro &&\n+\tmkdir rw &&\n+\tgit init --bare rw/testrepo &&\n+\tgit config \"url.file://$TRASH/ro/.insteadOf\" ro: &&\n+\tgit config \"url.file://$TRASH/rw/.pushInsteadOf\" rw: &&\n+\tgit config remote.r.url ro:wrong &&\n+\tgit config remote.r.pushurl rw:testrepo &&\n+\tgit push r refs/heads/master:refs/remotes/origin/master &&\n+\t(\n+\t\tcd rw/testrepo &&\n+\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n+\t\ttest \"z$r\" = \"z$the_commit\" &&\n+\n+\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n+\t)\n+'\n+\n+test_expect_success 'push without pushInsteadOf and explicit pushurl (pushurl + insteadOf is used for rewrite)' '\n+\tmk_empty &&\n+\trm -rf ro rw &&\n+\tTRASH=\"$(pwd)/\" &&\n+\tmkdir ro &&\n+\tmkdir rw &&\n+\tgit init --bare rw/testrepo &&\n+\tgit config \"url.file://$TRASH/ro/.insteadOf\" ro: &&\n+\tgit config \"url.file://$TRASH/rw/.insteadOf\" rw: &&\n+\tgit config remote.r.url ro:wrong &&\n+\tgit config remote.r.pushurl rw:testrepo &&\n+\tgit push r refs/heads/master:refs/remotes/origin/master &&\n+\t(\n+\t\tcd rw/testrepo &&\n+\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n+\t\ttest \"z$r\" = \"z$the_commit\" &&\n+\n+\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n+\t)\n+'\n+\n+test_expect_success 'push with pushInsteadOf but without explicit pushurl (url + pushInsteadOf is used for rewrite)' '\n+\tmk_empty &&\n+\trm -rf ro rw &&\n+\tTRASH=\"$(pwd)/\" &&\n+\tmkdir ro &&\n+\tmkdir rw &&\n+\tgit init --bare rw/testrepo &&\n+\tgit config \"url.file://$TRASH/ro/.insteadOf\" rw: &&\n+\tgit config \"url.file://$TRASH/rw/.pushInsteadOf\" rw: &&\n+\tgit config remote.r.url rw:testrepo &&\n+\tgit push r refs/heads/master:refs/remotes/origin/master &&\n+\t(\n+\t\tcd rw/testrepo &&\n+\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n+\t\ttest \"z$r\" = \"z$the_commit\" &&\n+\n+\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n+\t)\n+'\n+\n+test_expect_success 'push without pushInsteadOf and without explicit pushurl (url + insteadOf is used for rewrite)' '\n+\tmk_empty &&\n+\trm -rf ro rw &&\n+\tTRASH=\"$(pwd)/\" &&\n+\tmkdir ro &&\n+\tmkdir rw &&\n+\tgit init --bare rw/testrepo &&\n+\tgit config \"url.file://$TRASH/ro/.insteadOf\" rw: &&\n+\tgit config remote.r.url rw:testrepo &&\n+\tgit push r refs/heads/master:refs/remotes/origin/master &&\n+\t(\n+\t\tcd rw/testrepo &&\n+\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n+\t\ttest \"z$r\" = \"z$the_commit\" &&\n+\n+\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n+\t)\n+'\n+\n test_expect_success 'push with matching heads' '\n \n \tmk_test heads/master &&\n-- \n1.8.2\n"},{"id":"211619","messageId":"20130318231043.GD5062@elie.Belkin","threadId":"33222","inReplyTo":"20130318220224.3b23a381@hoelz.ro","subject":"Re: [PATCH] push: Alias pushurl from push rewrites","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-18T23:10:43Z","receivedAt":"2013-03-18T23:10:43Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nRob Hoelz wrote:\n\n> [url \"git://github.com/\"]\n>     insteadOf = github:\n> [url \"git://github.com/myuser/\"]\n>     insteadOf = mygithub:\n> [url \"git@github.com:myuser/\"]\n>     pushInsteadOf = mygithub:\n> [remote \"origin\"]\n>     url     = github:organization/project\n>     pushurl = mygithub:project\n>\n> With the above configuration, the following occurs:\n>\n> $ git push origin master\n> fatal: remote error:\n>   You can't push to git://github.com/myuser/project.git\n>   Use git@github.com:myuser/project.git\n>\n> So you can see that pushurl is being followed (it's not attempting to\n> push to git://github.com/organization/project), but insteadOf values are\n> being used as opposed to pushInsteadOf values for expanding the pushurl\n> alias.\n\nAt first glance it is not always obvious how overlapping settings like\nthese should interact.  Thanks for an instructive example that makes\nthe right behavior obvious.\n\nTest nits:\n\n[...]\n> --- a/t/t5516-fetch-push.sh\n> +++ b/t/t5516-fetch-push.sh\n> @@ -244,6 +244,87 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf\n>  \t)\n>  '\n>  \n> +test_expect_success 'push with pushInsteadOf and explicit pushurl (pushurl + pushInsteadOf does rewrite in this case)' '\n> +\tmk_empty &&\n> +\trm -rf ro rw &&\n> +\tTRASH=\"$(pwd)/\" &&\n> +\tmkdir ro &&\n> +\tmkdir rw &&\n> +\tgit init --bare rw/testrepo &&\n> +\tgit config \"url.file://$TRASH/ro/.insteadOf\" ro: &&\n> +\tgit config \"url.file://$TRASH/rw/.pushInsteadOf\" rw: &&\n\nThe surrounding tests don't do this, but I wonder if it would make\nsense to use test_config instead of 'git config' here.\n\nThat way, the test's settings wouldn't affect other tests, and in\nparticular if someone later decides to refactor the file by reordering\ntests, she could be confident that that would not break anything.\n\nIn most of the surrounding tests it does not matter because 'git\nconfig' is run in a subdirectory that is not reused later.  Patches\nfixing the exceptions below.\n\n> +\tgit config remote.r.url ro:wrong &&\n> +\tgit config remote.r.pushurl rw:testrepo &&\n> +\tgit push r refs/heads/master:refs/remotes/origin/master &&\n> +\t(\n> +\t\tcd rw/testrepo &&\n> +\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n> +\t\ttest \"z$r\" = \"z$the_commit\" &&\n> +\n> +\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n> +\t)\n\nTo produce more useful \"./t5516-fetch-push.sh -v -i\" output when the\ncomparison fails:\n\n\techo \"$the_commit commit refs/remotes/origin/master\" >expect &&\n\t(\n\t\tcd rw/testrepo &&\n\t\tgit for-each-ref refs/remotes/origin\n\t) >actual &&\n\ttest_cmp expect actual\n\nHope that helps,\n\nJonathan Nieder (3):\n  push test: use test_config where appropriate\n  push test: simplify check of push result\n  push test: rely on &&-chaining instead of 'if bad; then echo Oops; fi'\n\n t/t5516-fetch-push.sh | 156 +++++++++++++++++++++-----------------------------\n 1 file changed, 65 insertions(+), 91 deletions(-)\n"},{"id":"211616","messageId":"20130318231211.GE5062@elie.Belkin","threadId":"33222","inReplyTo":"20130318231043.GD5062@elie.Belkin","subject":"[PATCH 1/3] push test: use test_config when appropriate","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-18T23:12:11Z","receivedAt":"2013-03-18T23:12:11Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Configuration from test_config does not last beyond the end of the\ncurrent test assertion, making each test easier to think about in\nisolation.\n\nThis changes the meaning of some of the tests.  For example, currently\n\"push with insteadOf\" passes even if the line setting\n\"url.$TRASH.pushInsteadOf\" is dropped because an url.$TRASH.insteadOf\nsetting leaks in from a previous test.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n t/t5516-fetch-push.sh | 27 ++++++++++-----------------\n 1 file changed, 10 insertions(+), 17 deletions(-)\n\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex c31e5c1c..5b89c111 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -203,7 +203,7 @@ test_expect_success 'push with wildcard' '\n test_expect_success 'push with insteadOf' '\n \tmk_empty &&\n \tTRASH=\"$(pwd)/\" &&\n-\tgit config \"url.$TRASH.insteadOf\" trash/ &&\n+\ttest_config \"url.$TRASH.insteadOf\" trash/ &&\n \tgit push trash/testrepo refs/heads/master:refs/remotes/origin/master &&\n \t(\n \t\tcd testrepo &&\n@@ -217,7 +217,7 @@ test_expect_success 'push with insteadOf' '\n test_expect_success 'push with pushInsteadOf' '\n \tmk_empty &&\n \tTRASH=\"$(pwd)/\" &&\n-\tgit config \"url.$TRASH.pushInsteadOf\" trash/ &&\n+\ttest_config \"url.$TRASH.pushInsteadOf\" trash/ &&\n \tgit push trash/testrepo refs/heads/master:refs/remotes/origin/master &&\n \t(\n \t\tcd testrepo &&\n@@ -231,9 +231,9 @@ test_expect_success 'push with pushInsteadOf' '\n test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf should not rewrite)' '\n \tmk_empty &&\n \tTRASH=\"$(pwd)/\" &&\n-\tgit config \"url.trash2/.pushInsteadOf\" trash/ &&\n-\tgit config remote.r.url trash/wrong &&\n-\tgit config remote.r.pushurl \"$TRASH/testrepo\" &&\n+\ttest_config \"url.trash2/.pushInsteadOf\" trash/ &&\n+\ttest_config remote.r.url trash/wrong &&\n+\ttest_config remote.r.pushurl \"$TRASH/testrepo\" &&\n \tgit push r refs/heads/master:refs/remotes/origin/master &&\n \t(\n \t\tcd testrepo &&\n@@ -489,31 +489,24 @@ test_expect_success 'push with config remote.*.push = HEAD' '\n \t\tgit checkout local &&\n \t\tgit reset --hard $the_first_commit\n \t) &&\n-\tgit config remote.there.url testrepo &&\n-\tgit config remote.there.push HEAD &&\n-\tgit config branch.master.remote there &&\n+\ttest_config remote.there.url testrepo &&\n+\ttest_config remote.there.push HEAD &&\n+\ttest_config branch.master.remote there &&\n \tgit push &&\n \tcheck_push_result $the_commit heads/master &&\n \tcheck_push_result $the_first_commit heads/local\n '\n \n-# clean up the cruft left with the previous one\n-git config --remove-section remote.there\n-git config --remove-section branch.master\n-\n test_expect_success 'push with config remote.*.pushurl' '\n \n \tmk_test heads/master &&\n \tgit checkout master &&\n-\tgit config remote.there.url test2repo &&\n-\tgit config remote.there.pushurl testrepo &&\n+\ttest_config remote.there.url test2repo &&\n+\ttest_config remote.there.pushurl testrepo &&\n \tgit push there &&\n \tcheck_push_result $the_commit heads/master\n '\n \n-# clean up the cruft left with the previous one\n-git config --remove-section remote.there\n-\n test_expect_success 'push with dry-run' '\n \n \tmk_test heads/master &&\n-- \n1.8.2.rc3\n"},{"id":"211617","messageId":"20130318231340.GF5062@elie.Belkin","threadId":"33222","inReplyTo":"20130318231043.GD5062@elie.Belkin","subject":"[PATCH 2/3] push test: simplify check of push result","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-18T23:13:41Z","receivedAt":"2013-03-18T23:13:41Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"This test checks each ref with code like the following:\n\n\tr=$(git show-ref -s --verify refs/$ref) &&\n\ttest \"z$r\" = \"z$the_first_commit\"\n\nAfterward it counts refs:\n\n\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n\nSimpler to test the number and values of relevant refs in for-each-ref\noutput at the same time using test_cmp.  This makes the test more\nreadable and provides more helpful \"./t5516-push-push.sh -v\" output\nwhen the test fails.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n t/t5516-fetch-push.sh | 114 ++++++++++++++++++++++----------------------------\n 1 file changed, 51 insertions(+), 63 deletions(-)\n\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 5b89c111..2f1255d4 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -30,11 +30,10 @@ mk_test () {\n \t\tcd testrepo &&\n \t\tfor ref in \"$@\"\n \t\tdo\n-\t\t\tr=$(git show-ref -s --verify refs/$ref) &&\n-\t\t\ttest \"z$r\" = \"z$the_first_commit\" || {\n-\t\t\t\techo \"Oops, refs/$ref is wrong\"\n-\t\t\t\texit 1\n-\t\t\t}\n+\t\t\techo \"$the_first_commit\" >expect &&\n+\t\t\tgit show-ref -s --verify refs/$ref >actual &&\n+\t\t\ttest_cmp expect actual ||\n+\t\t\texit\n \t\tdone &&\n \t\tgit fsck --full\n \t)\n@@ -82,15 +81,13 @@ mk_child() {\n check_push_result () {\n \t(\n \t\tcd testrepo &&\n-\t\tit=\"$1\" &&\n-\t\tshift\n+\t\techo \"$1\" >expect &&\n+\t\tshift &&\n \t\tfor ref in \"$@\"\n \t\tdo\n-\t\t\tr=$(git show-ref -s --verify refs/$ref) &&\n-\t\t\ttest \"z$r\" = \"z$it\" || {\n-\t\t\t\techo \"Oops, refs/$ref is wrong\"\n-\t\t\t\texit 1\n-\t\t\t}\n+\t\t\tgit show-ref -s --verify refs/$ref >actual &&\n+\t\t\ttest_cmp expect actual ||\n+\t\t\texit\n \t\tdone &&\n \t\tgit fsck --full\n \t)\n@@ -118,10 +115,9 @@ test_expect_success 'fetch without wildcard' '\n \t\tcd testrepo &&\n \t\tgit fetch .. refs/heads/master:refs/remotes/origin/master &&\n \n-\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n-\t\ttest \"z$r\" = \"z$the_commit\" &&\n-\n-\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n+\t\techo \"$the_commit commit\trefs/remotes/origin/master\" >expect &&\n+\t\tgit for-each-ref refs/remotes/origin >actual &&\n+\t\ttest_cmp expect actual\n \t)\n '\n \n@@ -133,10 +129,9 @@ test_expect_success 'fetch with wildcard' '\n \t\tgit config remote.up.fetch \"refs/heads/*:refs/remotes/origin/*\" &&\n \t\tgit fetch up &&\n \n-\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n-\t\ttest \"z$r\" = \"z$the_commit\" &&\n-\n-\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n+\t\techo \"$the_commit commit\trefs/remotes/origin/master\" >expect &&\n+\t\tgit for-each-ref refs/remotes/origin >actual &&\n+\t\ttest_cmp expect actual\n \t)\n '\n \n@@ -150,10 +145,9 @@ test_expect_success 'fetch with insteadOf' '\n \t\tgit config remote.up.fetch \"refs/heads/*:refs/remotes/origin/*\" &&\n \t\tgit fetch up &&\n \n-\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n-\t\ttest \"z$r\" = \"z$the_commit\" &&\n-\n-\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n+\t\techo \"$the_commit commit\trefs/remotes/origin/master\" >expect &&\n+\t\tgit for-each-ref refs/remotes/origin >actual &&\n+\t\ttest_cmp expect actual\n \t)\n '\n \n@@ -167,10 +161,9 @@ test_expect_success 'fetch with pushInsteadOf (should not rewrite)' '\n \t\tgit config remote.up.fetch \"refs/heads/*:refs/remotes/origin/*\" &&\n \t\tgit fetch up &&\n \n-\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n-\t\ttest \"z$r\" = \"z$the_commit\" &&\n-\n-\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n+\t\techo \"$the_commit commit\trefs/remotes/origin/master\" >expect &&\n+\t\tgit for-each-ref refs/remotes/origin >actual &&\n+\t\ttest_cmp expect actual\n \t)\n '\n \n@@ -180,10 +173,9 @@ test_expect_success 'push without wildcard' '\n \tgit push testrepo refs/heads/master:refs/remotes/origin/master &&\n \t(\n \t\tcd testrepo &&\n-\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n-\t\ttest \"z$r\" = \"z$the_commit\" &&\n-\n-\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n+\t\techo \"$the_commit commit\trefs/remotes/origin/master\" >expect &&\n+\t\tgit for-each-ref refs/remotes/origin >actual &&\n+\t\ttest_cmp expect actual\n \t)\n '\n \n@@ -193,10 +185,9 @@ test_expect_success 'push with wildcard' '\n \tgit push testrepo \"refs/heads/*:refs/remotes/origin/*\" &&\n \t(\n \t\tcd testrepo &&\n-\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n-\t\ttest \"z$r\" = \"z$the_commit\" &&\n-\n-\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n+\t\techo \"$the_commit commit\trefs/remotes/origin/master\" >expect &&\n+\t\tgit for-each-ref refs/remotes/origin >actual &&\n+\t\ttest_cmp expect actual\n \t)\n '\n \n@@ -207,10 +198,9 @@ test_expect_success 'push with insteadOf' '\n \tgit push trash/testrepo refs/heads/master:refs/remotes/origin/master &&\n \t(\n \t\tcd testrepo &&\n-\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n-\t\ttest \"z$r\" = \"z$the_commit\" &&\n-\n-\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n+\t\techo \"$the_commit commit\trefs/remotes/origin/master\" >expect &&\n+\t\tgit for-each-ref refs/remotes/origin >actual &&\n+\t\ttest_cmp expect actual\n \t)\n '\n \n@@ -221,10 +211,9 @@ test_expect_success 'push with pushInsteadOf' '\n \tgit push trash/testrepo refs/heads/master:refs/remotes/origin/master &&\n \t(\n \t\tcd testrepo &&\n-\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n-\t\ttest \"z$r\" = \"z$the_commit\" &&\n-\n-\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n+\t\techo \"$the_commit commit\trefs/remotes/origin/master\" >expect &&\n+\t\tgit for-each-ref refs/remotes/origin >actual &&\n+\t\ttest_cmp expect actual\n \t)\n '\n \n@@ -237,10 +226,9 @@ test_expect_success 'push with pushInsteadOf and explicit pushurl (pushInsteadOf\n \tgit push r refs/heads/master:refs/remotes/origin/master &&\n \t(\n \t\tcd testrepo &&\n-\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n-\t\ttest \"z$r\" = \"z$the_commit\" &&\n-\n-\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n+\t\techo \"$the_commit commit\trefs/remotes/origin/master\" >expect &&\n+\t\tgit for-each-ref refs/remotes/origin >actual &&\n+\t\ttest_cmp expect actual\n \t)\n '\n \n@@ -827,9 +815,9 @@ test_expect_success 'fetch with branches' '\n \t(\n \t\tcd testrepo &&\n \t\tgit fetch branch1 &&\n-\t\tr=$(git show-ref -s --verify refs/heads/branch1) &&\n-\t\ttest \"z$r\" = \"z$the_commit\" &&\n-\t\ttest 1 = $(git for-each-ref refs/heads | wc -l)\n+\t\techo \"$the_commit commit\trefs/heads/branch1\" >expect &&\n+\t\tgit for-each-ref refs/heads >actual &&\n+\t\ttest_cmp expect actual\n \t) &&\n \tgit checkout master\n '\n@@ -840,9 +828,9 @@ test_expect_success 'fetch with branches containing #' '\n \t(\n \t\tcd testrepo &&\n \t\tgit fetch branch2 &&\n-\t\tr=$(git show-ref -s --verify refs/heads/branch2) &&\n-\t\ttest \"z$r\" = \"z$the_first_commit\" &&\n-\t\ttest 1 = $(git for-each-ref refs/heads | wc -l)\n+\t\techo \"$the_first_commit commit\trefs/heads/branch2\" >expect &&\n+\t\tgit for-each-ref refs/heads >actual &&\n+\t\ttest_cmp expect actual\n \t) &&\n \tgit checkout master\n '\n@@ -854,9 +842,9 @@ test_expect_success 'push with branches' '\n \tgit push branch1 &&\n \t(\n \t\tcd testrepo &&\n-\t\tr=$(git show-ref -s --verify refs/heads/master) &&\n-\t\ttest \"z$r\" = \"z$the_first_commit\" &&\n-\t\ttest 1 = $(git for-each-ref refs/heads | wc -l)\n+\t\techo \"$the_first_commit commit\trefs/heads/master\" >expect &&\n+\t\tgit for-each-ref refs/heads >actual &&\n+\t\ttest_cmp expect actual\n \t)\n '\n \n@@ -866,9 +854,9 @@ test_expect_success 'push with branches containing #' '\n \tgit push branch2 &&\n \t(\n \t\tcd testrepo &&\n-\t\tr=$(git show-ref -s --verify refs/heads/branch3) &&\n-\t\ttest \"z$r\" = \"z$the_first_commit\" &&\n-\t\ttest 1 = $(git for-each-ref refs/heads | wc -l)\n+\t\techo \"$the_first_commit commit\trefs/heads/branch3\" >expect &&\n+\t\tgit for-each-ref refs/heads >actual &&\n+\t\ttest_cmp expect actual\n \t) &&\n \tgit checkout master\n '\n@@ -951,9 +939,9 @@ test_expect_success 'push --porcelain' '\n \tgit push >.git/bar --porcelain  testrepo refs/heads/master:refs/remotes/origin/master &&\n \t(\n \t\tcd testrepo &&\n-\t\tr=$(git show-ref -s --verify refs/remotes/origin/master) &&\n-\t\ttest \"z$r\" = \"z$the_commit\" &&\n-\t\ttest 1 = $(git for-each-ref refs/remotes/origin | wc -l)\n+\t\techo \"$the_commit commit\trefs/remotes/origin/master\" >expect &&\n+\t\tgit for-each-ref refs/remotes/origin >actual &&\n+\t\ttest_cmp expect actual\n \t) &&\n \ttest_cmp .git/foo .git/bar\n '\n-- \n1.8.2.rc3\n"},{"id":"211618","messageId":"20130318231426.GG5062@elie.Belkin","threadId":"33222","inReplyTo":"20130318231043.GD5062@elie.Belkin","subject":"[PATCH 3/3] push test: rely on &&-chaining instead of 'if bad; then echo Oops; fi'","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-18T23:14:26Z","receivedAt":"2013-03-18T23:14:26Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"When it is unclear which command from a test has failed, usual\npractice these days is to debug by running the test again with \"sh -x\"\ninstead of relying on debugging 'echo' statements.\n\nSigned-off-by: Jonathan Nieder <jrnieder@gmail.com>\n---\n t/t5516-fetch-push.sh | 15 ++++-----------\n 1 file changed, 4 insertions(+), 11 deletions(-)\n\ndiff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh\nindex 2f1255d4..0695d570 100755\n--- a/t/t5516-fetch-push.sh\n+++ b/t/t5516-fetch-push.sh\n@@ -22,10 +22,8 @@ mk_test () {\n \t(\n \t\tfor ref in \"$@\"\n \t\tdo\n-\t\t\tgit push testrepo $the_first_commit:refs/$ref || {\n-\t\t\t\techo \"Oops, push refs/$ref failure\"\n-\t\t\t\texit 1\n-\t\t\t}\n+\t\t\tgit push testrepo $the_first_commit:refs/$ref ||\n+\t\t\texit\n \t\tdone &&\n \t\tcd testrepo &&\n \t\tfor ref in \"$@\"\n@@ -328,13 +326,8 @@ test_expect_success 'push with weak ambiguity (2)' '\n test_expect_success 'push with ambiguity' '\n \n \tmk_test heads/frotz tags/frotz &&\n-\tif git push testrepo master:frotz\n-\tthen\n-\t\techo \"Oops, should have failed\"\n-\t\tfalse\n-\telse\n-\t\tcheck_push_result $the_first_commit heads/frotz tags/frotz\n-\tfi\n+\ttest_must_fail git push testrepo master:frotz &&\n+\tcheck_push_result $the_first_commit heads/frotz tags/frotz\n \n '\n \n-- \n1.8.2.rc3\n"},{"id":"211622","messageId":"7v38vsma9o.fsf@alter.siamese.dyndns.org","threadId":"33222","inReplyTo":"20130318231043.GD5062@elie.Belkin","subject":"Re: [PATCH] push: Alias pushurl from push rewrites","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-19T01:46:43Z","receivedAt":"2013-03-19T01:46:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Test nits:\n> ...\n> Hope that helps,\n>\n> Jonathan Nieder (3):\n>   push test: use test_config where appropriate\n>   push test: simplify check of push result\n>   push test: rely on &&-chaining instead of 'if bad; then echo Oops; fi'\n\nAre these supposed to be follow-up patches?  Preparatory steps that\nRob can reroll on top?  Something else?\n"},{"id":"211624","messageId":"20130319015541.GH5062@elie.Belkin","threadId":"33222","inReplyTo":"7v38vsma9o.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] push: Alias pushurl from push rewrites","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2013-03-19T01:55:41Z","receivedAt":"2013-03-19T01:55:41Z","isPatch":true,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Junio C Hamano wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n\n>> Test nits:\n>> ...\n>> Hope that helps,\n>>\n>> Jonathan Nieder (3):\n>>   push test: use test_config where appropriate\n>>   push test: simplify check of push result\n>>   push test: rely on &&-chaining instead of 'if bad; then echo Oops; fi'\n>\n> Are these supposed to be follow-up patches?  Preparatory steps that\n> Rob can reroll on top?  Something else?\n\nPreparatory steps.\n"},{"id":"211697","messageId":"7vwqt3i7p9.fsf@alter.siamese.dyndns.org","threadId":"33222","inReplyTo":"20130319015541.GH5062@elie.Belkin","subject":"Re: [PATCH] push: Alias pushurl from push rewrites","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-19T18:08:02Z","receivedAt":"2013-03-19T18:08:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jonathan Nieder <jrnieder@gmail.com> writes:\n\n> Junio C Hamano wrote:\n>> Jonathan Nieder <jrnieder@gmail.com> writes:\n>\n>>> Test nits:\n>>> ...\n>>> Hope that helps,\n>>>\n>>> Jonathan Nieder (3):\n>>>   push test: use test_config where appropriate\n>>>   push test: simplify check of push result\n>>>   push test: rely on &&-chaining instead of 'if bad; then echo Oops; fi'\n>>\n>> Are these supposed to be follow-up patches?  Preparatory steps that\n>> Rob can reroll on top?  Something else?\n>\n> Preparatory steps.\n\nOK, thanks.  I'll queue these first then.\n"},{"id":"211743","messageId":"5149AC88.9090206@hoelz.ro","threadId":"33222","inReplyTo":"7vwqt3i7p9.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] push: Alias pushurl from push rewrites","fromName":"Rob Hoelz","fromEmail":"rob@hoelz.ro","sentAt":"2013-03-20T12:33:12Z","receivedAt":"2013-03-20T12:33:12Z","isPatch":true,"sender":{"key":"rob@hoelz.ro","avatar":"https://gravatar.com/avatar/c62607c6cef695f7a6cbd0f1f3ebd359e8c471af271581a6d7e29e3fab6fb788?d=mp&s=160"},"body":"On 3/19/13 7:08 PM, Junio C Hamano wrote:\n> Jonathan Nieder <jrnieder@gmail.com> writes:\n>\n>> Junio C Hamano wrote:\n>>> Jonathan Nieder <jrnieder@gmail.com> writes:\n>>>> Test nits:\n>>>> ...\n>>>> Hope that helps,\n>>>>\n>>>> Jonathan Nieder (3):\n>>>>   push test: use test_config where appropriate\n>>>>   push test: simplify check of push result\n>>>>   push test: rely on &&-chaining instead of 'if bad; then echo Oops; fi'\n>>> Are these supposed to be follow-up patches?  Preparatory steps that\n>>> Rob can reroll on top?  Something else?\n>> Preparatory steps.\n> OK, thanks.  I'll queue these first then.\n>\nShould I apply these to my patch to move things along?  What's the next\nstep for me?\n\nThanks,\nRob\n"},{"id":"211756","messageId":"7vk3p2gmup.fsf@alter.siamese.dyndns.org","threadId":"33222","inReplyTo":"5149AC88.9090206@hoelz.ro","subject":"Re: [PATCH] push: Alias pushurl from push rewrites","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-20T14:35:58Z","receivedAt":"2013-03-20T14:35:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rob Hoelz <rob@hoelz.ro> writes:\n\n> On 3/19/13 7:08 PM, Junio C Hamano wrote:\n>> Jonathan Nieder <jrnieder@gmail.com> writes:\n>>\n>>> Junio C Hamano wrote:\n>>>> Jonathan Nieder <jrnieder@gmail.com> writes:\n>>>>> Test nits:\n>>>>> ...\n>>>>> Hope that helps,\n>>>>>\n>>>>> Jonathan Nieder (3):\n>>>>>   push test: use test_config where appropriate\n>>>>>   push test: simplify check of push result\n>>>>>   push test: rely on &&-chaining instead of 'if bad; then echo Oops; fi'\n>>>> Are these supposed to be follow-up patches?  Preparatory steps that\n>>>> Rob can reroll on top?  Something else?\n>>> Preparatory steps.\n>> OK, thanks.  I'll queue these first then.\n>>\n> Should I apply these to my patch to move things along?  What's the next\n> step for me?\n\nYou would fetch from nearby git.git mirror, find the tip of\nJanathan's series and redo your patch on top.  Perhaps the session\nwould go like this:\n\n    $ git fetch git://git.kernel.org/pub/scm/git/git.git/ pu\n    $ git log --oneline --first-parent ..FETCH_HEAD | grep jn/push-tests\n    83a072a Merge branch 'jn/push-tests' into pu\n    $ git checkout -b rh/push-pushurl-pushinsteadof 83a072a\n    ... redo the work, perhaps with combinations of:\n    $ git cherry-pick -n $your_original_branch\n    $ edit t/t5516-fetch-push.sh\n    ... and then\n    $ make test\n    $ git commit\n"},{"id":"212413","messageId":"20130327122059.7e9da729@hoelz.ro","threadId":"33222","inReplyTo":"7vk3p2gmup.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] push: Alias pushurl from push rewrites","fromName":"Rob Hoelz","fromEmail":"rob@hoelz.ro","sentAt":"2013-03-27T17:20:59Z","receivedAt":"2013-03-27T17:20:59Z","isPatch":true,"sender":{"key":"rob@hoelz.ro","avatar":"https://gravatar.com/avatar/c62607c6cef695f7a6cbd0f1f3ebd359e8c471af271581a6d7e29e3fab6fb788?d=mp&s=160"},"body":"On Wed, 20 Mar 2013 07:35:58 -0700\nJunio C Hamano <gitster@pobox.com> wrote:\n\n> Rob Hoelz <rob@hoelz.ro> writes:\n> \n> > On 3/19/13 7:08 PM, Junio C Hamano wrote:\n> >> Jonathan Nieder <jrnieder@gmail.com> writes:\n> >>\n> >>> Junio C Hamano wrote:\n> >>>> Jonathan Nieder <jrnieder@gmail.com> writes:\n> >>>>> Test nits:\n> >>>>> ...\n> >>>>> Hope that helps,\n> >>>>>\n> >>>>> Jonathan Nieder (3):\n> >>>>>   push test: use test_config where appropriate\n> >>>>>   push test: simplify check of push result\n> >>>>>   push test: rely on &&-chaining instead of 'if bad; then echo\n> >>>>> Oops; fi'\n> >>>> Are these supposed to be follow-up patches?  Preparatory steps\n> >>>> that Rob can reroll on top?  Something else?\n> >>> Preparatory steps.\n> >> OK, thanks.  I'll queue these first then.\n> >>\n> > Should I apply these to my patch to move things along?  What's the\n> > next step for me?\n> \n> You would fetch from nearby git.git mirror, find the tip of\n> Janathan's series and redo your patch on top.  Perhaps the session\n> would go like this:\n> \n>     $ git fetch git://git.kernel.org/pub/scm/git/git.git/ pu\n>     $ git log --oneline --first-parent ..FETCH_HEAD | grep\n> jn/push-tests 83a072a Merge branch 'jn/push-tests' into pu\n>     $ git checkout -b rh/push-pushurl-pushinsteadof 83a072a\n>     ... redo the work, perhaps with combinations of:\n>     $ git cherry-pick -n $your_original_branch\n>     $ edit t/t5516-fetch-push.sh\n>     ... and then\n>     $ make test\n>     $ git commit\n> \n\nOk, changes applied.  New patch coming.\n"}]}