[PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM

[PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM

[Junio C Hamano]

Older versions of Git before v1.7.10 did not DWIM

    $ git pull $URL for-linus

to the tag "tags/for-linus" and the users were required to say

    $ git pull $URL tags/for-linus

instead.  Because newer versions of Git works either way,
request-pull used to show tags/for-linus when asked

    $ git request-pull origin/master $URL for-linus

The recent updates broke this and in the output we see "for-linus"
without the "tags/" prefix.

As v1.7.10 is more than 2 years old, this should matter very little
in practice, but resurrecting it is very simple.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * I _think_ the fix, without breaking the spirit of Linus's "I do
   not want the thing DWIM based on what the remote end has"
   original, would be as simple as this patch.  We can queue it as a
   regression fix and do another round of -rc4 if those who depend
   on request-pull heavily feel strongly about it.

 git-request-pull.sh     | 6 ++++++
 t/t5150-request-pull.sh | 8 +++++++-
 2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/git-request-pull.sh b/git-request-pull.sh
index b67513a..d6648b2 100755
--- a/git-request-pull.sh
+++ b/git-request-pull.sh
@@ -118,6 +118,12 @@ then
 	status=1
 fi
 
+# Special case: turn "for_linus" to "tags/for_linus" when it is correct
+if test "$ref" = "refs/tags/$pretty_remote"
+then
+	pretty_remote=tags/$pretty_remote
+fi
+
 url=$(git ls-remote --get-url "$url")
 
 git show -s --format='The following changes since commit %H:
diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index 75d6b38..93e2c65 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -223,7 +223,13 @@ test_expect_success 'pull request format' '
 		git request-pull initial "$downstream_url" tags/full:refs/tags/full
 	) >request &&
 	sed -nf fuzz.sed <request >request.fuzzy &&
-	test_i18ncmp expect request.fuzzy
+	test_i18ncmp expect request.fuzzy &&
+
+	(
+		cd local &&
+		git request-pull initial "$downstream_url" full
+	) >request &&
+	grep ' tags/full$'
 '
 
 test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' '
-- 
2.0.0-rc3-434-g1ba2fe8

Re: [PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM

[Michael S. Tsirkin]

On Fri, May 16, 2014 at 10:57:50AM -0700, Junio C Hamano wrote:
> Older versions of Git before v1.7.10 did not DWIM
> 
>     $ git pull $URL for-linus
> 
> to the tag "tags/for-linus" and the users were required to say
> 
>     $ git pull $URL tags/for-linus
> 
> instead.  Because newer versions of Git works either way,
> request-pull used to show tags/for-linus when asked
> 
>     $ git request-pull origin/master $URL for-linus
> 
> The recent updates broke this and in the output we see "for-linus"
> without the "tags/" prefix.
> 
> As v1.7.10 is more than 2 years old, this should matter very little
> in practice, but resurrecting it is very simple.

Well RHEL6 apparently comes with git 1.7.1, there are
probably others.

The problem isn't theorectical actually,
the reason I noticed the change in behaviour is because one of
my pull requests got bounced because of it:
http://article.gmane.org/gmane.comp.emulators.qemu/273121


> Signed-off-by: Junio C Hamano <gitster@pobox.com>


Tested-by: Michael S. Tsirkin <mst@redhat.com>

> ---
> 
>  * I _think_ the fix, without breaking the spirit of Linus's "I do
>    not want the thing DWIM based on what the remote end has"
>    original, would be as simple as this patch.  We can queue it as a
>    regression fix and do another round of -rc4 if those who depend
>    on request-pull heavily feel strongly about it.
> 
>  git-request-pull.sh     | 6 ++++++
>  t/t5150-request-pull.sh | 8 +++++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/git-request-pull.sh b/git-request-pull.sh
> index b67513a..d6648b2 100755
> --- a/git-request-pull.sh
> +++ b/git-request-pull.sh
> @@ -118,6 +118,12 @@ then
>  	status=1
>  fi
>  
> +# Special case: turn "for_linus" to "tags/for_linus" when it is correct
> +if test "$ref" = "refs/tags/$pretty_remote"
> +then
> +	pretty_remote=tags/$pretty_remote
> +fi
> +
>  url=$(git ls-remote --get-url "$url")
>  
>  git show -s --format='The following changes since commit %H:
> diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
> index 75d6b38..93e2c65 100755
> --- a/t/t5150-request-pull.sh
> +++ b/t/t5150-request-pull.sh
> @@ -223,7 +223,13 @@ test_expect_success 'pull request format' '
>  		git request-pull initial "$downstream_url" tags/full:refs/tags/full
>  	) >request &&
>  	sed -nf fuzz.sed <request >request.fuzzy &&
> -	test_i18ncmp expect request.fuzzy
> +	test_i18ncmp expect request.fuzzy &&
> +
> +	(
> +		cd local &&
> +		git request-pull initial "$downstream_url" full
> +	) >request &&
> +	grep ' tags/full$'
>  '
>  
>  test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' '
> -- 
> 2.0.0-rc3-434-g1ba2fe8

Re: [PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM

[Junio C Hamano]

"Michael S. Tsirkin" <mst@redhat.com> writes:

> Well RHEL6 apparently comes with git 1.7.1, there are
> probably others.
>
> The problem isn't theorectical actually,

Thanks.  Let's do the fix for 2.0 then.

Re: [PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM

[Johannes Sixt]

Am 5/16/2014 19:57, schrieb Junio C Hamano:
> --- a/t/t5150-request-pull.sh
> +++ b/t/t5150-request-pull.sh
> @@ -223,7 +223,13 @@ test_expect_success 'pull request format' '
>  		git request-pull initial "$downstream_url" tags/full:refs/tags/full
>  	) >request &&
>  	sed -nf fuzz.sed <request >request.fuzzy &&
> -	test_i18ncmp expect request.fuzzy
> +	test_i18ncmp expect request.fuzzy &&
> +
> +	(
> +		cd local &&
> +		git request-pull initial "$downstream_url" full
> +	) >request &&
> +	grep ' tags/full$'
>  '

What's this crap? Here's a fix. Feel free to tame down the subject line
if you think it's too strong ;)

--- 8< ---
From: Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH] fix brown paper bag breakage in t5150-request-pull.sh

The recent addition to the test case 'pull request format' interrupted
the single-quoted text, effectively adding a third argument to the
test_expect_success command. Since we do not have a prerequisite named
"pull request format", the test is skipped, no matter what. Additionally,
the file name argument to the grep command is missing. Fix both issues.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 t/t5150-request-pull.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
index 93e2c65..82c33b8 100755
--- a/t/t5150-request-pull.sh
+++ b/t/t5150-request-pull.sh
@@ -229,7 +229,7 @@ test_expect_success 'pull request format' '
 		cd local &&
 		git request-pull initial "$downstream_url" full
 	) >request &&
-	grep ' tags/full$'
+	grep " tags/full\$" request
 '

 test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' '
-- 
2.0.0.1326.g81a507a

Re: [PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM

[Junio C Hamano]

Johannes Sixt <j.sixt@viscovery.net> writes:

> Am 5/16/2014 19:57, schrieb Junio C Hamano:
>> --- a/t/t5150-request-pull.sh
>> +++ b/t/t5150-request-pull.sh
>> @@ -223,7 +223,13 @@ test_expect_success 'pull request format' '
>>  		git request-pull initial "$downstream_url" tags/full:refs/tags/full
>>  	) >request &&
>>  	sed -nf fuzz.sed <request >request.fuzzy &&
>> -	test_i18ncmp expect request.fuzzy
>> +	test_i18ncmp expect request.fuzzy &&
>> +
>> +	(
>> +		cd local &&
>> +		git request-pull initial "$downstream_url" full
>> +	) >request &&
>> +	grep ' tags/full$'
>>  '
>
> What's this crap? Here's a fix. Feel free to tame down the subject line
> if you think it's too strong ;)
>
> --- 8< ---
> From: Johannes Sixt <j6t@kdbg.org>
> Subject: [PATCH] fix brown paper bag breakage in t5150-request-pull.sh

Thanks for catching; I do not think the "brown paper bag" is too
strong ;-)

> The recent addition to the test case 'pull request format' interrupted
> the single-quoted text, effectively adding a third argument to the
> test_expect_success command. Since we do not have a prerequisite named
> "pull request format", the test is skipped, no matter what. Additionally,
> the file name argument to the grep command is missing. Fix both issues.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  t/t5150-request-pull.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/t/t5150-request-pull.sh b/t/t5150-request-pull.sh
> index 93e2c65..82c33b8 100755
> --- a/t/t5150-request-pull.sh
> +++ b/t/t5150-request-pull.sh
> @@ -229,7 +229,7 @@ test_expect_success 'pull request format' '
>  		cd local &&
>  		git request-pull initial "$downstream_url" full
>  	) >request &&
> -	grep ' tags/full$'
> +	grep " tags/full\$" request
>  '
>
>  test_expect_success 'request-pull ignores OPTIONS_KEEPDASHDASH poison' '