[(Apple,Git),05/13] t5701: git --version can have SP in it
diff mbox series

Message ID 20190129193818.8645-6-jeremyhu@apple.com
State New
Headers show
Series
  • Differences between git-2.20.1 and Apple Git-116
Related show

Commit Message

Jeremy Sequoia Jan. 29, 2019, 7:38 p.m. UTC
Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
---
 t/t5701-git-serve.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Johannes Schindelin Jan. 30, 2019, 1:36 p.m. UTC | #1
Hi Jeremy,

On Tue, 29 Jan 2019, Jeremy Huddleston Sequoia wrote:

> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>


That commit message is again very short. Only because I remember the
previous patch's commit message do I have a clue what this is about.

You definitely need to write something here about customized forks of Git
adding suffixes including spaces to the Git version.

And you will need to state where those spaces are converted to dots in
Git's capability advertisement. The reason for this requirement: should
that logic change at any stage in the future, your patch will fail,
somebody will investigate and find this commit and *needs* a helpful
commit message.

> ---
>  t/t5701-git-serve.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh
> index ae79c6bbc0..7bc25700fa 100755
> --- a/t/t5701-git-serve.sh
> +++ b/t/t5701-git-serve.sh
> @@ -7,7 +7,7 @@ test_description='test git-serve and server commands'
>  test_expect_success 'test capability advertisement' '
>  	cat >expect <<-EOF &&
>  	version 2
> -	agent=git/$(git version | cut -d" " -f3)
> +	agent=git/$(git --version | sed -e "s/git version //" -e "s/ /\./g")

This `git version` needs to be anchored, and it would be much conciser to
use `-e "y/ /./"`, which even BSD sed understands according to
https://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html

Ciao,
Johannes

>  	ls-refs
>  	fetch=shallow
>  	server-option
> -- 
> 2.20.0 (Apple Git-115)
> 
>
Jeremy Sequoia Jan. 30, 2019, 7:35 p.m. UTC | #2
> On Jan 30, 2019, at 05:36, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> 
> Hi Jeremy,
> 
> On Tue, 29 Jan 2019, Jeremy Huddleston Sequoia wrote:
> 
>> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
> 
> 
> That commit message is again very short. Only because I remember the
> previous patch's commit message do I have a clue what this is about.

Yeah, I became lax with commit messages when I was under the impression that the git community was not interested engaging with Apple.  I should have taken a pass through these commits before sending, but I half expected these patches to be ignored.  I am delighted to see that I was wrong and will go back and provide more details in the ones that I feel are upstreamable like this one.

> You definitely need to write something here about customized forks of Git
> adding suffixes including spaces to the Git version.
> 
> And you will need to state where those spaces are converted to dots in
> Git's capability advertisement. The reason for this requirement: should
> that logic change at any stage in the future, your patch will fail,
> somebody will investigate and find this commit and *needs* a helpful
> commit message.

Thanks, I'll add you to the CC of this patch, so you can critique my wording in the updated message.

> 
>> ---
>> t/t5701-git-serve.sh | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh
>> index ae79c6bbc0..7bc25700fa 100755
>> --- a/t/t5701-git-serve.sh
>> +++ b/t/t5701-git-serve.sh
>> @@ -7,7 +7,7 @@ test_description='test git-serve and server commands'
>> test_expect_success 'test capability advertisement' '
>> 	cat >expect <<-EOF &&
>> 	version 2
>> -	agent=git/$(git version | cut -d" " -f3)
>> +	agent=git/$(git --version | sed -e "s/git version //" -e "s/ /\./g")
> 
> This `git version` needs to be anchored,

Good catch, thanks.

> and it would be much conciser to
> use `-e "y/ /./"`, which even BSD sed understands according to
> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html

Also done, thanks.

> 
> Ciao,
> Johannes
> 
>> 	ls-refs
>> 	fetch=shallow
>> 	server-option
>> -- 
>> 2.20.0 (Apple Git-115)
>> 
>>

Patch
diff mbox series

diff --git a/t/t5701-git-serve.sh b/t/t5701-git-serve.sh
index ae79c6bbc0..7bc25700fa 100755
--- a/t/t5701-git-serve.sh
+++ b/t/t5701-git-serve.sh
@@ -7,7 +7,7 @@  test_description='test git-serve and server commands'
 test_expect_success 'test capability advertisement' '
 	cat >expect <<-EOF &&
 	version 2
-	agent=git/$(git version | cut -d" " -f3)
+	agent=git/$(git --version | sed -e "s/git version //" -e "s/ /\./g")
 	ls-refs
 	fetch=shallow
 	server-option