Re: [PATCH] test-bzr: Do not use unportable sed "\+"
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 "\+"
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 "\+"
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 "\+"
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 "\+"
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 "\+"
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 "\+"
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