{"thread":{"id":"36679","subject":"[PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM","startedAt":"2014-05-16T17:57:50Z","lastAt":"2014-06-02T18:05:17Z","messageCount":5,"participants":["Junio C Hamano","Michael S. Tsirkin","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"241985","messageId":"1400263070-15312-1-git-send-email-gitster@pobox.com","threadId":"36679","inReplyTo":null,"subject":"[PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-16T17:57:50Z","receivedAt":"2014-05-16T17:57:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Older versions of Git before v1.7.10 did not DWIM\n\n $ git pull $URL for-linus\n\nto the tag \"tags/for-linus\" and the users were required to say\n\n $ git pull $URL tags/for-linus\n\ninstead. Because newer versions of Git works either way,\nrequest-pull used to show tags/for-linus when asked\n\n $ git request-pull origin/master $URL for-linus\n\nThe recent updates broke this and in the output we see \"for-linus\"\nwithout the \"tags/\" prefix.\n\nAs v1.7.10 is more than 2 years old, this should matter very little\nin practice, but resurrecting it is very simple.\n\nSigned-off-by: Junio C Hamano \n---\n\n * I _think_ the fix, without breaking the spirit of Linus's \"I do\n not want the thing DWIM based on what the remote end has\"\n original, would be as simple as this patch. We can queue it as a\n regression fix and do another round of -rc4 if those who depend\n on request-pull heavily feel strongly about it.\n\n git-request-pull.sh | 6 ++++++\n t/t5150-request-pull.sh | 8 +++++++-\n 2 files changed, 13 insertions(+), 1 deletion(-)\n\ndiff --git a/git-request-pull.sh b/git-request-pull.sh\nindex b67513a..d6648b2 100755\n--- a/git-request-pull.sh\n+++ b/git-request-pull.sh\n@@ -118,6 +118,12 @@ then\n \tstatus=1\n fi\n \n+# Special case: turn \"for_linus\" to \"tags/for_linus\" when it is correct\n+if test \"$ref\" = \"refs/tags/$pretty_remote\"\n+then\n+\tpretty_remote=tags/$pretty_remote\n+fi\n+\n url=$(git ls-remote --get-url \"$url\")\n \n git show -s --format='The following changes since commit %H:\ndiff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh\nindex 75d6b38..93e2c65 100755\n--- a/t/t5150-request-pull.sh\n+++ b/t/t5150-request-pull.sh\n@@ -223,7 +223,13 @@ test_expect_success 'pull request format' '\n \t\tgit request-pull initial \"$downstream_url\" tags/full:refs/tags/full\n \t) >request &&\n \tsed -nf fuzz.sed request.fuzzy &&\n-\ttest_i18ncmp expect request.fuzzy\n+\ttest_i18ncmp expect request.fuzzy &&\n+\n+\t(\n+\t\tcd local &&\n+\t\tgit request-pull initial \"$downstream_url\" full\n+\t) >request &&\n+\tgrep ' tags/full$'\n '\n \n test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' '\n-- \n2.0.0-rc3-434-g1ba2fe8\n"},{"id":"242081","messageId":"20140518083305.GB28462@redhat.com","threadId":"36679","inReplyTo":"1400263070-15312-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM","fromName":"Michael S. Tsirkin","fromEmail":"mst@redhat.com","sentAt":"2014-05-18T08:33:05Z","receivedAt":"2014-05-18T08:33:05Z","isPatch":true,"sender":{"key":"mst@kernel.org","avatar":null},"body":"On Fri, May 16, 2014 at 10:57:50AM -0700, Junio C Hamano wrote:\n> Older versions of Git before v1.7.10 did not DWIM\n> \n> $ git pull $URL for-linus\n> \n> to the tag \"tags/for-linus\" and the users were required to say\n> \n> $ git pull $URL tags/for-linus\n> \n> instead. Because newer versions of Git works either way,\n> request-pull used to show tags/for-linus when asked\n> \n> $ git request-pull origin/master $URL for-linus\n> \n> The recent updates broke this and in the output we see \"for-linus\"\n> without the \"tags/\" prefix.\n> \n> As v1.7.10 is more than 2 years old, this should matter very little\n> in practice, but resurrecting it is very simple.\n\nWell RHEL6 apparently comes with git 1.7.1, there are\nprobably others.\n\nThe problem isn't theorectical actually,\nthe reason I noticed the change in behaviour is because one of\nmy pull requests got bounced because of it:\nhttp://article.gmane.org/gmane.comp.emulators.qemu/273121\n\n\n> Signed-off-by: Junio C Hamano \n\n\nTested-by: Michael S. Tsirkin \n\n> ---\n> \n> * I _think_ the fix, without breaking the spirit of Linus's \"I do\n> not want the thing DWIM based on what the remote end has\"\n> original, would be as simple as this patch. We can queue it as a\n> regression fix and do another round of -rc4 if those who depend\n> on request-pull heavily feel strongly about it.\n> \n> git-request-pull.sh | 6 ++++++\n> t/t5150-request-pull.sh | 8 +++++++-\n> 2 files changed, 13 insertions(+), 1 deletion(-)\n> \n> diff --git a/git-request-pull.sh b/git-request-pull.sh\n> index b67513a..d6648b2 100755\n> --- a/git-request-pull.sh\n> +++ b/git-request-pull.sh\n> @@ -118,6 +118,12 @@ then\n> \tstatus=1\n> fi\n> \n> +# Special case: turn \"for_linus\" to \"tags/for_linus\" when it is correct\n> +if test \"$ref\" = \"refs/tags/$pretty_remote\"\n> +then\n> +\tpretty_remote=tags/$pretty_remote\n> +fi\n> +\n> url=$(git ls-remote --get-url \"$url\")\n> \n> git show -s --format='The following changes since commit %H:\n> diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh\n> index 75d6b38..93e2c65 100755\n> --- a/t/t5150-request-pull.sh\n> +++ b/t/t5150-request-pull.sh\n> @@ -223,7 +223,13 @@ test_expect_success 'pull request format' '\n> \t\tgit request-pull initial \"$downstream_url\" tags/full:refs/tags/full\n> \t) >request &&\n> \tsed -nf fuzz.sed request.fuzzy &&\n> -\ttest_i18ncmp expect request.fuzzy\n> +\ttest_i18ncmp expect request.fuzzy &&\n> +\n> +\t(\n> +\t\tcd local &&\n> +\t\tgit request-pull initial \"$downstream_url\" full\n> +\t) >request &&\n> +\tgrep ' tags/full$'\n> '\n> \n> test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' '\n> -- \n> 2.0.0-rc3-434-g1ba2fe8\n"},{"id":"242176","messageId":"xmqqlhtx3f58.fsf@gitster.dls.corp.google.com","threadId":"36679","inReplyTo":"20140518083305.GB28462@redhat.com","subject":"Re: [PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-05-19T16:30:11Z","receivedAt":"2014-05-19T16:30:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Michael S. Tsirkin\" writes:\n\n> Well RHEL6 apparently comes with git 1.7.1, there are\n> probably others.\n>\n> The problem isn't theorectical actually,\n\nThanks. Let's do the fix for 2.0 then.\n"},{"id":"243123","messageId":"538C2290.3060506@viscovery.net","threadId":"36679","inReplyTo":"1400263070-15312-1-git-send-email-gitster@pobox.com","subject":"Re: [PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2014-06-02T07:06:56Z","receivedAt":"2014-06-02T07:06:56Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 5/16/2014 19:57, schrieb Junio C Hamano:\n> --- a/t/t5150-request-pull.sh\n> +++ b/t/t5150-request-pull.sh\n> @@ -223,7 +223,13 @@ test_expect_success 'pull request format' '\n> \t\tgit request-pull initial \"$downstream_url\" tags/full:refs/tags/full\n> \t) >request &&\n> \tsed -nf fuzz.sed request.fuzzy &&\n> -\ttest_i18ncmp expect request.fuzzy\n> +\ttest_i18ncmp expect request.fuzzy &&\n> +\n> +\t(\n> +\t\tcd local &&\n> +\t\tgit request-pull initial \"$downstream_url\" full\n> +\t) >request &&\n> +\tgrep ' tags/full$'\n> '\n\nWhat's this crap? Here's a fix. Feel free to tame down the subject line\nif you think it's too strong ;)\n\n--- 8< ---\nFrom: Johannes Sixt \nSubject: [PATCH] fix brown paper bag breakage in t5150-request-pull.sh\n\nThe recent addition to the test case 'pull request format' interrupted\nthe single-quoted text, effectively adding a third argument to the\ntest_expect_success command. Since we do not have a prerequisite named\n\"pull request format\", the test is skipped, no matter what. Additionally,\nthe file name argument to the grep command is missing. Fix both issues.\n\nSigned-off-by: Johannes Sixt \n---\n t/t5150-request-pull.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh\nindex 93e2c65..82c33b8 100755\n--- a/t/t5150-request-pull.sh\n+++ b/t/t5150-request-pull.sh\n@@ -229,7 +229,7 @@ test_expect_success 'pull request format' '\n \t\tcd local &&\n \t\tgit request-pull initial \"$downstream_url\" full\n \t) >request &&\n-\tgrep ' tags/full$'\n+\tgrep \" tags/full\\$\" request\n '\n\n test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' '\n-- \n2.0.0.1326.g81a507a\n"},{"id":"243140","messageId":"xmqqfvjn5grm.fsf@gitster.dls.corp.google.com","threadId":"36679","inReplyTo":"538C2290.3060506@viscovery.net","subject":"Re: [PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-06-02T18:05:17Z","receivedAt":"2014-06-02T18:05:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> Am 5/16/2014 19:57, schrieb Junio C Hamano:\n>> --- a/t/t5150-request-pull.sh\n>> +++ b/t/t5150-request-pull.sh\n>> @@ -223,7 +223,13 @@ test_expect_success 'pull request format' '\n>> \t\tgit request-pull initial \"$downstream_url\" tags/full:refs/tags/full\n>> \t) >request &&\n>> \tsed -nf fuzz.sed request.fuzzy &&\n>> -\ttest_i18ncmp expect request.fuzzy\n>> +\ttest_i18ncmp expect request.fuzzy &&\n>> +\n>> +\t(\n>> +\t\tcd local &&\n>> +\t\tgit request-pull initial \"$downstream_url\" full\n>> +\t) >request &&\n>> +\tgrep ' tags/full$'\n>> '\n>\n> What's this crap? Here's a fix. Feel free to tame down the subject line\n> if you think it's too strong ;)\n>\n> --- 8< ---\n> From: Johannes Sixt \n> Subject: [PATCH] fix brown paper bag breakage in t5150-request-pull.sh\n\nThanks for catching; I do not think the \"brown paper bag\" is too\nstrong ;-)\n\n> The recent addition to the test case 'pull request format' interrupted\n> the single-quoted text, effectively adding a third argument to the\n> test_expect_success command. Since we do not have a prerequisite named\n> \"pull request format\", the test is skipped, no matter what. Additionally,\n> the file name argument to the grep command is missing. Fix both issues.\n>\n> Signed-off-by: Johannes Sixt \n> ---\n> t/t5150-request-pull.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh\n> index 93e2c65..82c33b8 100755\n> --- a/t/t5150-request-pull.sh\n> +++ b/t/t5150-request-pull.sh\n> @@ -229,7 +229,7 @@ test_expect_success 'pull request format' '\n> \t\tcd local &&\n> \t\tgit request-pull initial \"$downstream_url\" full\n> \t) >request &&\n> -\tgrep ' tags/full$'\n> +\tgrep \" tags/full\\$\" request\n> '\n>\n> test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' '\n"}]}