Re: [PATCH] test-bzr: Do not use unportable sed "\+"

2013-05-11 Thread Junio C Hamano
Torsten Bögershausen  writes:

> On 11.05.13 22:09, Junio C Hamano wrote:
>> Torsten Bögershausen  writes:
>> 
>>> I did,
>>> the interesting thing is that the test passes with and without your patch.
>>> (After enabling  GIT_TEST_LONG and GIT_TEST_HTTPD in both cases)
>> 
>> Strange.  Do you see differences between the produced packed-refs
>> file?
>
> The original version seems to look like this:
> :1 666527db455708922859283c673094002092910b
> :2 1e2acf73c6db881cfb1d56d67662e3d9260be2cf
> [snip]
>
> The "fixed POSIX version" follows that style:
> 666527db455708922859283c673094002092910b 
> refs/tags/blablablablablablablablablablablablablablablablablablablablablablablablablablablablablabla-1
> 1e2acf73c6db881cfb1d56d67662e3d9260be2cf 
> refs/tags/blablablablablablablablablablablablablablablablablablablablablablablablablablablablablabla-2
> [snip]

That means whatever the test that comes after that set-up step to
populate the packed-refs file does not need the packed-refs file.

In fact, the test only sees if "clone" succeeds, without checking
what refs are present in the resulting repository.  The original
repository lacks these 5 tags due to the non-portable sed
construct (it has a corrupt packed-refs file) and it may or may not
have other refs, but because cloning an empty repository does not
error out these days, the test just passes.

Sloppy.


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] test-bzr: Do not use unportable sed "\+"

2013-05-11 Thread Torsten Bögershausen
On 11.05.13 22:09, Junio C Hamano wrote:
> Torsten Bögershausen  writes:
> 
>> I did,
>> the interesting thing is that the test passes with and without your patch.
>> (After enabling  GIT_TEST_LONG and GIT_TEST_HTTPD in both cases)
> 
> Strange.  Do you see differences between the produced packed-refs
> file?

The original version seems to look like this:
:1 666527db455708922859283c673094002092910b
:2 1e2acf73c6db881cfb1d56d67662e3d9260be2cf
[snip]

The "fixed POSIX version" follows that style:
666527db455708922859283c673094002092910b 
refs/tags/blablablablablablablablablablablablablablablablablablablablablablablablablablablablablabla-1
1e2acf73c6db881cfb1d56d67662e3d9260be2cf 
refs/tags/blablablablablablablablablablablablablablablablablablablablablablablablablablablablablabla-2
[snip]

(Just to verify the changes:)
diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
index b23efbb..68965f0 100755
--- a/t/t5551-http-fetch.sh
+++ b/t/t5551-http-fetch.sh
@@ -210,8 +210,11 @@ test_expect_success EXPENSIVE 'create 50,000 tags in the 
repo' '
# now assign tags to all the dangling commits we created above
tag=$("$PERL_PATH" -e "print \"bla\" x 30") &&
sed -e "s/^:\(.\+\) \(.\+\)$/\2 refs\/tags\/$tag-\1/" >packed-refs
+   sed -e "s|^:\([^ ]*\) \(.*\)$|\2 refs/tags/$tag-\1|" packed-refs.junio
+   sed -e "s/^:\(.\+\) \(.\+\)$/\2 refs\/tags\/$tag-\1/" packed-refs.orig
)
 '
+   exit 0

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH] test-bzr: Do not use unportable sed "\+"

2013-05-11 Thread Torsten Bögershausen

On 05/11/2013 03:25 PM, Torsten Bögershausen wrote:
Sorry, I forgot to mention that there is another test case that fails
in test-bzr.sh (Both Linux and MacOS):

Cloning into 'gitrepo'...
--- ../expected 2013-05-11 20:07:17.678360248 +
+++ actual  2013-05-11 20:07:21.510312073 +
@@ -1,3 +1,3 @@
-origin/HEAD
+origin
 origin/branch
 origin/trunk
not ok 11 - proper bzr repo











--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] test-bzr: Do not use unportable sed "\+"

2013-05-11 Thread Junio C Hamano
Torsten Bögershausen  writes:

> I did,
> the interesting thing is that the test passes with and without your patch.
> (After enabling  GIT_TEST_LONG and GIT_TEST_HTTPD in both cases)

Strange.  Do you see differences between the produced packed-refs
file?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] test-bzr: Do not use unportable sed "\+"

2013-05-11 Thread Torsten Bögershausen
On 11.05.13 21:45, Junio C Hamano wrote:
> Junio C Hamano  writes:
> 
>> Thanks.  Is there another one in t/t5551-http-fetch.sh that checks
>> the tags?
> 
> I think your sed will see the same breakage for the one in 5551 (my
> sed is unfortunately GNU and ".\+" does not break it).  Could you
> test this patch with:
> 
>  GIT_TEST_LONG=YesPlease GIT_TEST_HTTPD=YesPlease \
>  ./t5551-http-fetch.sh
> 
> Thanks.
> 
>  t/t5551-http-fetch.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
> index b23efbb..4a3184e 100755
> --- a/t/t5551-http-fetch.sh
> +++ b/t/t5551-http-fetch.sh
> @@ -209,7 +209,7 @@ test_expect_success EXPENSIVE 'create 50,000 tags in the 
> repo' '
>  
>   # now assign tags to all the dangling commits we created above
>   tag=$("$PERL_PATH" -e "print \"bla\" x 30") &&
> - sed -e "s/^:\(.\+\) \(.\+\)$/\2 refs\/tags\/$tag-\1/"  >>packed-refs
> + sed -e "s|^:\([^ ]*\) \(.*\)$|\2 refs/tags/$tag-\1|"  >>packed-refs
>   )

I did,
the interesting thing is that the test passes with and without your patch.
(After enabling  GIT_TEST_LONG and GIT_TEST_HTTPD in both cases)


Side note:
I added this line in in t/check-non-portable-shell.pl
  /sed[^"]+"[^"]+\\([+])/ and err "sed \\$1 is not portable)";






--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] test-bzr: Do not use unportable sed "\+"

2013-05-11 Thread Junio C Hamano
Junio C Hamano  writes:

> Thanks.  Is there another one in t/t5551-http-fetch.sh that checks
> the tags?

I think your sed will see the same breakage for the one in 5551 (my
sed is unfortunately GNU and ".\+" does not break it).  Could you
test this patch with:

 GIT_TEST_LONG=YesPlease GIT_TEST_HTTPD=YesPlease \
 ./t5551-http-fetch.sh

Thanks.

 t/t5551-http-fetch.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh
index b23efbb..4a3184e 100755
--- a/t/t5551-http-fetch.sh
+++ b/t/t5551-http-fetch.sh
@@ -209,7 +209,7 @@ test_expect_success EXPENSIVE 'create 50,000 tags in the 
repo' '
 
# now assign tags to all the dangling commits we created above
tag=$("$PERL_PATH" -e "print \"bla\" x 30") &&
-   sed -e "s/^:\(.\+\) \(.\+\)$/\2 refs\/tags\/$tag-\1/" >packed-refs
+   sed -e "s|^:\([^ ]*\) \(.*\)$|\2 refs/tags/$tag-\1|" >packed-refs
)
 '
 
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] test-bzr: Do not use unportable sed "\+"

2013-05-11 Thread Junio C Hamano
Torsten Bögershausen  writes:

> Using sed -e '/[0-9]\+//' to find "one ore more" digits
> is not portable.
> Use the Basic Regular Expression '/[0-9][0-9]*//' instead
>
> Signed-off-by: Torsten Bögershausen 

Thanks.  Is there another one in t/t5551-http-fetch.sh that checks
the tags?

> ---
>  contrib/remote-helpers/test-bzr.sh | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/contrib/remote-helpers/test-bzr.sh 
> b/contrib/remote-helpers/test-bzr.sh
> index d9c32f4..5dfa070 100755
> --- a/contrib/remote-helpers/test-bzr.sh
> +++ b/contrib/remote-helpers/test-bzr.sh
> @@ -328,7 +328,7 @@ test_expect_success 'strip' '
>  
>echo four >> content &&
>bzr commit -m four &&
> -  bzr log --line | sed -e "s/^[0-9]\+: //" > ../expected
> +  bzr log --line | sed -e "s/^[0-9][0-9]*: //" > ../expected
>) &&
>  
>(cd gitrepo &&
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html