threads / patch / 36679

patchrequest-pull: resurrect for-linus -> tags/for-linus DWIM

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

## tl;dr

5 messages between May 16, 2014 and Jun 2, 2014. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Junio C Hamano· May 16, 2014, 17:57 UTC · lore

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

Show changes to 2 files +13 −1

git-request-pull.sh, t/t5150-request-pull.sh

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

Michael S. Tsirkin· May 18, 2014, 08:33 UTC · re: Junio C Hamano · lore

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

On Fri, May 16, 2014 at 10:57:50AM -0700, Junio C Hamano wrote:

Show 18 quoted lines

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

Show 50 quoted lines

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

Junio C Hamano· May 19, 2014, 16:30 UTC · re: Michael S. Tsirkin · lore

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

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

Johannes Sixt· Jun 2, 2014, 07:06 UTC · re: Junio C Hamano · lore

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

Am 5/16/2014 19:57, schrieb Junio C Hamano:

Show 15 quoted lines

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

Show changes to t/t5150-request-pull.sh +1 −1

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

Junio C Hamano· Jun 2, 2014, 18:05 UTC · re: Johannes Sixt · lore

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

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

Show 23 quoted lines

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

Show 24 quoted lines

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