git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM
@ 2014-05-16 17:57 Junio C Hamano
  2014-05-18  8:33 ` Michael S. Tsirkin
  2014-06-02  7:06 ` Johannes Sixt
  0 siblings, 2 replies; 5+ messages in thread
From: Junio C Hamano @ 2014-05-16 17:57 UTC (permalink / raw)
  To: git; +Cc: Michael S. Tsirkin

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM
  2014-05-16 17:57 [PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM Junio C Hamano
@ 2014-05-18  8:33 ` Michael S. Tsirkin
  2014-05-19 16:30   ` Junio C Hamano
  2014-06-02  7:06 ` Johannes Sixt
  1 sibling, 1 reply; 5+ messages in thread
From: Michael S. Tsirkin @ 2014-05-18  8:33 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM
  2014-05-18  8:33 ` Michael S. Tsirkin
@ 2014-05-19 16:30   ` Junio C Hamano
  0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2014-05-19 16:30 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: git

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM
  2014-05-16 17:57 [PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM Junio C Hamano
  2014-05-18  8:33 ` Michael S. Tsirkin
@ 2014-06-02  7:06 ` Johannes Sixt
  2014-06-02 18:05   ` Junio C Hamano
  1 sibling, 1 reply; 5+ messages in thread
From: Johannes Sixt @ 2014-06-02  7:06 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, Michael S. Tsirkin

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM
  2014-06-02  7:06 ` Johannes Sixt
@ 2014-06-02 18:05   ` Junio C Hamano
  0 siblings, 0 replies; 5+ messages in thread
From: Junio C Hamano @ 2014-06-02 18:05 UTC (permalink / raw)
  To: Johannes Sixt; +Cc: git, Michael S. Tsirkin

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2014-06-02 18:05 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-16 17:57 [PATCH] request-pull: resurrect for-linus -> tags/for-linus DWIM Junio C Hamano
2014-05-18  8:33 ` Michael S. Tsirkin
2014-05-19 16:30   ` Junio C Hamano
2014-06-02  7:06 ` Johannes Sixt
2014-06-02 18:05   ` Junio C Hamano

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).