{"thread":{"id":"33209","subject":"[PATCH] remote..pushurl does not consider aliases when pushing","startedAt":"2013-03-17T21:40:02Z","lastAt":"2013-03-17T22:47:05Z","messageCount":3,"participants":["Rob Hoelz","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"211529","messageId":"20130317224002.366f54f7@hoelz.ro","threadId":"33209","inReplyTo":null,"subject":"[PATCH] remote..pushurl does not consider aliases when pushing","fromName":"Rob Hoelz","fromEmail":"rob@hoelz.ro","sentAt":"2013-03-17T21:40:02Z","receivedAt":"2013-03-17T21:40:02Z","isPatch":true,"sender":{"key":"rob@hoelz.ro","avatar":"https://gravatar.com/avatar/c62607c6cef695f7a6cbd0f1f3ebd359e8c471af271581a6d7e29e3fab6fb788?d=mp&s=160"},"body":"Hi everyone! I found a bug in Git today and wrote up a fix; I did my best to conform to the rules layed out in Documentation/SubmittingPatches, but please let me know if I need to change anything to get my work merged. =) I have CC'ed Josh Triplet, as\nhe was the last one to touch the line I modified. I hope my commit messages explain the problem I encountered well enough; if not,\nI can always go back and amend them.\n\nPatches follow.\n\n-Rob\n\n>From 5007b11e86c0835807632cb41e6cfa75ce9a1aa1 Mon Sep 17 00:00:00 2001\nFrom: Rob Hoelz \nDate: Sun, 17 Mar 2013 21:49:20 +0100\nSubject: [PATCH 1/2] Add test for pushInsteadOf + pushurl\n\ngit push currently doesn't consider pushInsteadOf when\nusing pushurl; this test tests that.\n\nSigned-off-by: Rob Hoelz \n---\n t/t5500-push-pushurl.sh | 37 +++++++++++++++++++++++++++++++++++++\n 1 file changed, 37 insertions(+)\n create mode 100644 t/t5500-push-pushurl.sh\n\ndiff --git a/t/t5500-push-pushurl.sh b/t/t5500-push-pushurl.sh\nnew file mode 100644\nindex 0000000..74d4ff6\n--- /dev/null\n+++ b/t/t5500-push-pushurl.sh\n@@ -0,0 +1,37 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2013 Rob Hoelz\n+#\n+\n+test_description='Test that remote..pushurl uses pushInsteadOf'\n+. ./test-lib.sh\n+\n+wd=$(pwd)\n+test_expect_success 'setup test repositories' '\n+\tmkdir ro &&\n+\tmkdir rw &&\n+\n+\tgit init --bare ro/repo &&\n+\tgit init --bare rw/repo &&\n+\tgit init test-repo\n+'\n+\n+test_expect_success 'setup remote config' \"\n+\tcd test-repo &&\n+\tgit config 'url.file://$wd/ro/.insteadOf' herero: &&\n+\tgit config 'url.file://$wd/rw/.pushInsteadOf' hererw: &&\n+\tgit remote add origin herero:repo &&\n+\tgit config remote.origin.pushurl hererw:repo\n+\"\n+\n+test_expect_success 'test commit and push' '\n+\ttest_commit one &&\n+\tgit push origin master:master\n+'\n+\n+test_expect_success 'check for commits in rw repo' '\n+\tcd ../rw/repo &&\n+\tgit log --pretty=oneline | grep -q .\n+'\n+\n+test_done\n-- \n1.8.2\n\n>From 0cbd1aab6bdc0c2f8893ed8b9a8e3eb0126917d1 Mon Sep 17 00:00:00 2001\nFrom: Rob Hoelz \nDate: Sun, 17 Mar 2013 16:34:35 +0100\nSubject: [PATCH 2/2] push: Alias pushurl from push rewrites\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\nThis commit fixes that.\n\nSigned-off-by: Rob Hoelz \n---\n remote.c | 2 +-\n 1 file changed, 1 insertion(+), 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++) {\n-- \n1.8.2\n"},{"id":"211533","messageId":"7vppyxptbr.fsf@alter.siamese.dyndns.org","threadId":"33209","inReplyTo":"20130317224002.366f54f7@hoelz.ro","subject":"Re: [PATCH] remote..pushurl does not consider aliases when pushing","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-17T22:14:32Z","receivedAt":"2013-03-17T22:14:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Rob Hoelz writes:\n\n> Hi everyone! I found a bug in Git today and wrote up a fix; I did my best to conform to the rules layed out in Documentation/SubmittingPatches, but please let me know if I need to change anything to get my work merged. =) I have CC'ed Josh Triplet, as\n> he was the last one to touch the line I modified. I hope my commit messages explain the problem I encountered well enough; if not,\n> I can always go back and amend them.\n>\n> Patches follow.\n>\n> -Rob\n\n\nPlease read Documentation/SubmittingPatches and follow it. The\nabove is most likely to be the cover letter of a two-patch series\n(meaning you will be sending three pieces of e-mail messages), or\nperhaps out of band comment below the three-dash line of a single\npatch (you will send only one piece of e-mail message).\n\nSee recent patches on the list from list regulars for good examples,\ne.g.\n\n http://thread.gmane.org/gmane.comp.version-control.git/218350\n http://thread.gmane.org/gmane.comp.version-control.git/218177/focus=218361\n\n> From 5007b11e86c0835807632cb41e6cfa75ce9a1aa1 Mon Sep 17 00:00:00 2001\n> From: Rob Hoelz \n> Date: Sun, 17 Mar 2013 21:49:20 +0100\n> Subject: [PATCH 1/2] Add test for pushInsteadOf + pushurl\n>\n> git push currently doesn't consider pushInsteadOf when\n> using pushurl; this test tests that.\n>\n> Signed-off-by: Rob Hoelz \n> ---\n> t/t5500-push-pushurl.sh | 37 +++++++++++++++++++++++++++++++++++++\n> 1 file changed, 37 insertions(+)\n> create mode 100644 t/t5500-push-pushurl.sh\n\nThe number 5500 is already taken. Please do not add a duplicate.\n\nI also wonder if we need to waste a new test number for this;\nperhaps adding new tests to 5516 that already tests insteadOf might\nbe a better fit, but I didn't carefully read it.\n\n> diff --git a/t/t5500-push-pushurl.sh b/t/t5500-push-pushurl.sh\n> new file mode 100644\n\nTest scripts are supposed to be executable.\n\n> +test_expect_success 'test commit and push' '\n> +\ttest_commit one &&\n> +\tgit push origin master:master\n> +'\n> +\n> +test_expect_success 'check for commits in rw repo' '\n> +\tcd ../rw/repo &&\n> +\tgit log --pretty=oneline | grep -q .\n> +'\n\nAre both expected to succeed in patch 1/2 without any code change?\n\nIf you were doing a large code change, it is a good series structure\nto have tests first that are marked as \"expect_failure\" in an early\npatch, and then in a later patch that changes the code to fix it,\nupdate the tests that start to pass to \"expect_success\".\n\nI personally do not think you need such a two-step approach for\nsomething small like this; instead you can just have a single patch\nthat adds a set of tests that expect success and code change.\n\nThanks.\n"},{"id":"211537","messageId":"20130317234705.654fc987@hoelz.ro","threadId":"33209","inReplyTo":"7vppyxptbr.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] remote..pushurl does not consider aliases when pushing","fromName":"Rob Hoelz","fromEmail":"rob@hoelz.ro","sentAt":"2013-03-17T22:47:05Z","receivedAt":"2013-03-17T22:47:05Z","isPatch":true,"sender":{"key":"rob@hoelz.ro","avatar":"https://gravatar.com/avatar/c62607c6cef695f7a6cbd0f1f3ebd359e8c471af271581a6d7e29e3fab6fb788?d=mp&s=160"},"body":"On Sun, 17 Mar 2013 15:14:32 -0700\nJunio C Hamano wrote:\n\n> Rob Hoelz writes:\n> \n> > Hi everyone! I found a bug in Git today and wrote up a fix; I did\n> > my best to conform to the rules layed out in\n> > Documentation/SubmittingPatches, but please let me know if I need\n> > to change anything to get my work merged. =) I have CC'ed Josh\n> > Triplet, as he was the last one to touch the line I modified. I\n> > hope my commit messages explain the problem I encountered well\n> > enough; if not, I can always go back and amend them.\n> >\n> > Patches follow.\n> >\n> > -Rob\n> \n> \n> Please read Documentation/SubmittingPatches and follow it. The\n> above is most likely to be the cover letter of a two-patch series\n> (meaning you will be sending three pieces of e-mail messages), or\n> perhaps out of band comment below the three-dash line of a single\n> patch (you will send only one piece of e-mail message).\n> \n> See recent patches on the list from list regulars for good examples,\n> e.g.\n> \n> http://thread.gmane.org/gmane.comp.version-control.git/218350\n> http://thread.gmane.org/gmane.comp.version-control.git/218177/focus=218361\n> \n> > From 5007b11e86c0835807632cb41e6cfa75ce9a1aa1 Mon Sep 17 00:00:00\n> > 2001 From: Rob Hoelz \n> > Date: Sun, 17 Mar 2013 21:49:20 +0100\n> > Subject: [PATCH 1/2] Add test for pushInsteadOf + pushurl\n> >\n> > git push currently doesn't consider pushInsteadOf when\n> > using pushurl; this test tests that.\n> >\n> > Signed-off-by: Rob Hoelz \n> > ---\n> > t/t5500-push-pushurl.sh | 37 +++++++++++++++++++++++++++++++++++++\n> > 1 file changed, 37 insertions(+)\n> > create mode 100644 t/t5500-push-pushurl.sh\n> \n> The number 5500 is already taken. Please do not add a duplicate.\n> \n> I also wonder if we need to waste a new test number for this;\n> perhaps adding new tests to 5516 that already tests insteadOf might\n> be a better fit, but I didn't carefully read it.\n> \n> > diff --git a/t/t5500-push-pushurl.sh b/t/t5500-push-pushurl.sh\n> > new file mode 100644\n> \n> Test scripts are supposed to be executable.\n> \n> > +test_expect_success 'test commit and push' '\n> > +\ttest_commit one &&\n> > +\tgit push origin master:master\n> > +'\n> > +\n> > +test_expect_success 'check for commits in rw repo' '\n> > +\tcd ../rw/repo &&\n> > +\tgit log --pretty=oneline | grep -q .\n> > +'\n> \n> Are both expected to succeed in patch 1/2 without any code change?\n> \n> If you were doing a large code change, it is a good series structure\n> to have tests first that are marked as \"expect_failure\" in an early\n> patch, and then in a later patch that changes the code to fix it,\n> update the tests that start to pass to \"expect_success\".\n> \n> I personally do not think you need such a two-step approach for\n> something small like this; instead you can just have a single patch\n> that adds a set of tests that expect success and code change.\n> \n> Thanks.\n> \n\nThanks for the feeback; another reply with the new patch follows.\n"}]}