{"thread":{"id":"33792","subject":"[PATCH] test-bzr: Do not use unportable sed \"\\+\"","startedAt":"2013-05-11T13:25:52Z","lastAt":"2013-05-13T18:36:33Z","messageCount":13,"participants":["Torsten Bögershausen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"217002","messageId":"201305111525.53130.tboegi@web.de","threadId":"33792","inReplyTo":null,"subject":"[PATCH] test-bzr: Do not use unportable sed \"\\+\"","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-05-11T13:25:52Z","receivedAt":"2013-05-11T13:25:52Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"Using sed -e '/[0-9]\\+//' to find \"one ore more\" digits\nis not portable.\nUse the Basic Regular Expression '/[0-9][0-9]*//' instead\n\nSigned-off-by: Torsten Bögershausen \n---\n contrib/remote-helpers/test-bzr.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh\nindex d9c32f4..5dfa070 100755\n--- a/contrib/remote-helpers/test-bzr.sh\n+++ b/contrib/remote-helpers/test-bzr.sh\n@@ -328,7 +328,7 @@ test_expect_success 'strip' '\n \n echo four >> content &&\n bzr commit -m four &&\n- bzr log --line | sed -e \"s/^[0-9]\\+: //\" > ../expected\n+ bzr log --line | sed -e \"s/^[0-9][0-9]*: //\" > ../expected\n ) &&\n \n (cd gitrepo &&\n-- \n1.8.2.1.614.g66d7af5\n"},{"id":"217033","messageId":"7vwqr5wdqm.fsf@alter.siamese.dyndns.org","threadId":"33792","inReplyTo":"201305111525.53130.tboegi@web.de","subject":"Re: [PATCH] test-bzr: Do not use unportable sed \"\\+\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-11T18:52:33Z","receivedAt":"2013-05-11T18:52:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen writes:\n\n> Using sed -e '/[0-9]\\+//' to find \"one ore more\" digits\n> is not portable.\n> Use the Basic Regular Expression '/[0-9][0-9]*//' instead\n>\n> Signed-off-by: Torsten Bögershausen \n\nThanks. Is there another one in t/t5551-http-fetch.sh that checks\nthe tags?\n\n> ---\n> contrib/remote-helpers/test-bzr.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/contrib/remote-helpers/test-bzr.sh b/contrib/remote-helpers/test-bzr.sh\n> index d9c32f4..5dfa070 100755\n> --- a/contrib/remote-helpers/test-bzr.sh\n> +++ b/contrib/remote-helpers/test-bzr.sh\n> @@ -328,7 +328,7 @@ test_expect_success 'strip' '\n> \n> echo four >> content &&\n> bzr commit -m four &&\n> - bzr log --line | sed -e \"s/^[0-9]\\+: //\" > ../expected\n> + bzr log --line | sed -e \"s/^[0-9][0-9]*: //\" > ../expected\n> ) &&\n> \n> (cd gitrepo &&\n"},{"id":"217036","messageId":"7vobchwbax.fsf@alter.siamese.dyndns.org","threadId":"33792","inReplyTo":"7vwqr5wdqm.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] test-bzr: Do not use unportable sed \"\\+\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-11T19:45:10Z","receivedAt":"2013-05-11T19:45:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Thanks. Is there another one in t/t5551-http-fetch.sh that checks\n> the tags?\n\nI think your sed will see the same breakage for the one in 5551 (my\nsed is unfortunately GNU and \".\\+\" does not break it). Could you\ntest this patch with:\n\n GIT_TEST_LONG=YesPlease GIT_TEST_HTTPD=YesPlease \\\n ./t5551-http-fetch.sh\n\nThanks.\n\n t/t5551-http-fetch.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh\nindex b23efbb..4a3184e 100755\n--- a/t/t5551-http-fetch.sh\n+++ b/t/t5551-http-fetch.sh\n@@ -209,7 +209,7 @@ test_expect_success EXPENSIVE 'create 50,000 tags in the repo' '\n \n \t# now assign tags to all the dangling commits we created above\n \ttag=$(\"$PERL_PATH\" -e \"print \\\"bla\\\" x 30\") &&\n-\tsed -e \"s/^:\\(.\\+\\) \\(.\\+\\)$/\\2 refs\\/tags\\/$tag-\\1/\" >packed-refs\n+\tsed -e \"s|^:\\([^ ]*\\) \\(.*\\)$|\\2 refs/tags/$tag-\\1|\" >packed-refs\n \t)\n '\n \n"},{"id":"217039","messageId":"518EA351.8000603@web.de","threadId":"33792","inReplyTo":"7vobchwbax.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] test-bzr: Do not use unportable sed \"\\+\"","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-05-11T20:00:17Z","receivedAt":"2013-05-11T20:00:17Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 11.05.13 21:45, Junio C Hamano wrote:\n> Junio C Hamano writes:\n> \n>> Thanks. Is there another one in t/t5551-http-fetch.sh that checks\n>> the tags?\n> \n> I think your sed will see the same breakage for the one in 5551 (my\n> sed is unfortunately GNU and \".\\+\" does not break it). Could you\n> test this patch with:\n> \n> GIT_TEST_LONG=YesPlease GIT_TEST_HTTPD=YesPlease \\\n> ./t5551-http-fetch.sh\n> \n> Thanks.\n> \n> t/t5551-http-fetch.sh | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh\n> index b23efbb..4a3184e 100755\n> --- a/t/t5551-http-fetch.sh\n> +++ b/t/t5551-http-fetch.sh\n> @@ -209,7 +209,7 @@ test_expect_success EXPENSIVE 'create 50,000 tags in the repo' '\n> \n> \t# now assign tags to all the dangling commits we created above\n> \ttag=$(\"$PERL_PATH\" -e \"print \\\"bla\\\" x 30\") &&\n> -\tsed -e \"s/^:\\(.\\+\\) \\(.\\+\\)$/\\2 refs\\/tags\\/$tag-\\1/\" >packed-refs\n> +\tsed -e \"s|^:\\([^ ]*\\) \\(.*\\)$|\\2 refs/tags/$tag-\\1|\" >packed-refs\n> \t)\n\nI did,\nthe interesting thing is that the test passes with and without your patch.\n(After enabling GIT_TEST_LONG and GIT_TEST_HTTPD in both cases)\n\n\nSide note:\nI added this line in in t/check-non-portable-shell.pl\n /sed[^\"]+\"[^\"]+\\\\([+])/ and err \"sed \\\\$1 is not portable)\";\n"},{"id":"217040","messageId":"7v8v3lwa5p.fsf@alter.siamese.dyndns.org","threadId":"33792","inReplyTo":"518EA351.8000603@web.de","subject":"Re: [PATCH] test-bzr: Do not use unportable sed \"\\+\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-11T20:09:54Z","receivedAt":"2013-05-11T20:09:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen writes:\n\n> I did,\n> the interesting thing is that the test passes with and without your patch.\n> (After enabling GIT_TEST_LONG and GIT_TEST_HTTPD in both cases)\n\nStrange. Do you see differences between the produced packed-refs\nfile?\n"},{"id":"217041","messageId":"518EA5ED.9020001@web.de","threadId":"33792","inReplyTo":"201305111525.53130.tboegi@web.de","subject":"RE: [PATCH] test-bzr: Do not use unportable sed \"\\+\"","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-05-11T20:11:25Z","receivedAt":"2013-05-11T20:11:25Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 05/11/2013 03:25 PM, Torsten Bögershausen wrote:\nSorry, I forgot to mention that there is another test case that fails\nin test-bzr.sh (Both Linux and MacOS):\n\nCloning into 'gitrepo'...\n--- ../expected 2013-05-11 20:07:17.678360248 +0000\n+++ actual 2013-05-11 20:07:21.510312073 +0000\n@@ -1,3 +1,3 @@\n-origin/HEAD\n+origin\n origin/branch\n origin/trunk\nnot ok 11 - proper bzr repo\n"},{"id":"217042","messageId":"518EAB7C.3010508@web.de","threadId":"33792","inReplyTo":"7v8v3lwa5p.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] test-bzr: Do not use unportable sed \"\\+\"","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-05-11T20:35:08Z","receivedAt":"2013-05-11T20:35:08Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 11.05.13 22:09, Junio C Hamano wrote:\n> Torsten Bögershausen writes:\n> \n>> I did,\n>> the interesting thing is that the test passes with and without your patch.\n>> (After enabling GIT_TEST_LONG and GIT_TEST_HTTPD in both cases)\n> \n> Strange. Do you see differences between the produced packed-refs\n> file?\n\nThe original version seems to look like this:\n:1 666527db455708922859283c673094002092910b\n:2 1e2acf73c6db881cfb1d56d67662e3d9260be2cf\n[snip]\n\nThe \"fixed POSIX version\" follows that style:\n666527db455708922859283c673094002092910b refs/tags/blablablablablablablablablablablablablablablablablablablablablablablablablablablablablabla-1\n1e2acf73c6db881cfb1d56d67662e3d9260be2cf refs/tags/blablablablablablablablablablablablablablablablablablablablablablablablablablablablablabla-2\n[snip]\n\n(Just to verify the changes:)\ndiff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh\nindex b23efbb..68965f0 100755\n--- a/t/t5551-http-fetch.sh\n+++ b/t/t5551-http-fetch.sh\n@@ -210,8 +210,11 @@ test_expect_success EXPENSIVE 'create 50,000 tags in the repo' '\n # now assign tags to all the dangling commits we created above\n tag=$(\"$PERL_PATH\" -e \"print \\\"bla\\\" x 30\") &&\n sed -e \"s/^:\\(.\\+\\) \\(.\\+\\)$/\\2 refs\\/tags\\/$tag-\\1/\" >packed-refs\n+ sed -e \"s|^:\\([^ ]*\\) \\(.*\\)$|\\2 refs/tags/$tag-\\1|\" packed-refs.junio\n+ sed -e \"s/^:\\(.\\+\\) \\(.\\+\\)$/\\2 refs\\/tags\\/$tag-\\1/\" packed-refs.orig\n )\n '\n+ exit 0\n"},{"id":"217057","messageId":"7vsj1svqi7.fsf@alter.siamese.dyndns.org","threadId":"33792","inReplyTo":"518EAB7C.3010508@web.de","subject":"Re: [PATCH] test-bzr: Do not use unportable sed \"\\+\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-12T03:14:24Z","receivedAt":"2013-05-12T03:14:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen writes:\n\n> On 11.05.13 22:09, Junio C Hamano wrote:\n>> Torsten Bögershausen writes:\n>> \n>>> I did,\n>>> the interesting thing is that the test passes with and without your patch.\n>>> (After enabling GIT_TEST_LONG and GIT_TEST_HTTPD in both cases)\n>> \n>> Strange. Do you see differences between the produced packed-refs\n>> file?\n>\n> The original version seems to look like this:\n> :1 666527db455708922859283c673094002092910b\n> :2 1e2acf73c6db881cfb1d56d67662e3d9260be2cf\n> [snip]\n>\n> The \"fixed POSIX version\" follows that style:\n> 666527db455708922859283c673094002092910b refs/tags/blablablablablablablablablablablablablablablablablablablablablablablablablablablablablabla-1\n> 1e2acf73c6db881cfb1d56d67662e3d9260be2cf refs/tags/blablablablablablablablablablablablablablablablablablablablablablablablablablablablablabla-2\n> [snip]\n\nThat means whatever the test that comes after that set-up step to\npopulate the packed-refs file does not need the packed-refs file.\n\nIn fact, the test only sees if \"clone\" succeeds, without checking\nwhat refs are present in the resulting repository. The original\nrepository lacks these 50000 tags due to the non-portable sed\nconstruct (it has a corrupt packed-refs file) and it may or may not\nhave other refs, but because cloning an empty repository does not\nerror out these days, the test just passes.\n\nSloppy.\n"},{"id":"217098","messageId":"7vr4hbu818.fsf_-_@alter.siamese.dyndns.org","threadId":"33792","inReplyTo":"7vsj1svqi7.fsf@alter.siamese.dyndns.org","subject":"[PATCH] t5551: do not use unportable sed '\\+'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-12T22:50:59Z","receivedAt":"2013-05-12T22:50:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"The set-up step to prepare a repository with 50000 tags used a\nnon-porable '\\+' to match one-or-more.\n\nThe error was not caught because the next test that uses that\nrepository did not even bother to check if these expected tags were\nactually cloned to the resulting repository.\n\nFix the sed construct to use BRE and update the \"clone\" test that\nwanted to test cloning from such a repository with many refs to\ncheck the resulting repository.\n\nSigned-off-by: Junio C Hamano \n---\n t/t5551-http-fetch.sh | 8 ++++++--\n 1 file changed, 6 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t5551-http-fetch.sh b/t/t5551-http-fetch.sh\nindex 47eb769..fe6637b 100755\n--- a/t/t5551-http-fetch.sh\n+++ b/t/t5551-http-fetch.sh\n@@ -184,13 +184,17 @@ test_expect_success EXPENSIVE 'create 50,000 tags in the repo' '\n \n \t# now assign tags to all the dangling commits we created above\n \ttag=$(\"$PERL_PATH\" -e \"print \\\"bla\\\" x 30\") &&\n-\tsed -e \"s/^:\\(.\\+\\) \\(.\\+\\)$/\\2 refs\\/tags\\/$tag-\\1/\" >packed-refs\n+\tsed -e \"s|^:\\([^ ]*\\) \\(.*\\)$|\\2 refs/tags/$tag-\\1|\" >packed-refs\n \t)\n '\n \n test_expect_success EXPENSIVE 'clone the 50,000 tag repo to check OS command line overflow' '\n \tgit clone $HTTPD_URL/smart/repo.git too-many-refs 2>err &&\n-\ttest_line_count = 0 err\n+\ttest_line_count = 0 err &&\n+\t(\n+\t\tcd too-many-refs &&\n+\t\ttest $(git for-each-ref refs/tags | wc -l) = 50000\n+\t)\n '\n \n stop_httpd\n-- \n1.8.3-rc1-278-g19c008b\n"},{"id":"217199","messageId":"51910418.2070904@web.de","threadId":"33792","inReplyTo":"7vr4hbu818.fsf_-_@alter.siamese.dyndns.org","subject":"Re: [PATCH] t5551: do not use unportable sed '\\+'","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-05-13T15:17:44Z","receivedAt":"2013-05-13T15:17:44Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2013-05-13 00.50, Junio C Hamano wrote:\n> Signed-off-by: Junio C Hamano \n> ---\n> t/t5551-http-fetch.sh | 8 ++++++--\n> 1 file changed, 6 insertions(+), 2 deletions(-)\n> \nThanks, works like a charm, tested on Mac OS.\n/torsten\n"},{"id":"217207","messageId":"7vtxm6n8pj.fsf@alter.siamese.dyndns.org","threadId":"33792","inReplyTo":"51910418.2070904@web.de","subject":"Re: [PATCH] t5551: do not use unportable sed '\\+'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-13T16:30:32Z","receivedAt":"2013-05-13T16:30:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen writes:\n\n> On 2013-05-13 00.50, Junio C Hamano wrote:\n>> Signed-off-by: Junio C Hamano \n>> ---\n>> t/t5551-http-fetch.sh | 8 ++++++--\n>> 1 file changed, 6 insertions(+), 2 deletions(-)\n>> \n> Thanks, works like a charm, tested on Mac OS.\n> /torsten\n\nThanks; I take it that you reverted the \"sed fix\" part and saw the\nupdated \"clone\" check fail with the platform \"sed\"?\n"},{"id":"217213","messageId":"51912FEC.6020909@web.de","threadId":"33792","inReplyTo":"7vtxm6n8pj.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] t5551: do not use unportable sed '\\+'","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2013-05-13T18:24:44Z","receivedAt":"2013-05-13T18:24:44Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 2013-05-13 18.30, Junio C Hamano wrote:\n> Torsten Bögershausen writes:\n> \n>> On 2013-05-13 00.50, Junio C Hamano wrote:\n>>> Signed-off-by: Junio C Hamano \n>>> ---\n>>> t/t5551-http-fetch.sh | 8 ++++++--\n>>> 1 file changed, 6 insertions(+), 2 deletions(-)\n>>>\n>> Thanks, works like a charm, tested on Mac OS.\n>> /torsten\n> \n> Thanks; I take it that you reverted the \"sed fix\" part and saw the\n> updated \"clone\" check fail with the platform \"sed\"?\nIn my first test with the fix the test case passed.\nThen the sed expression was only manipulated to verify that the TC failes.\nAnd now I took even the original expression and have verfied it is failing.\n\nThe short version: Yes\n"},{"id":"217215","messageId":"7vhai6n2vi.fsf@alter.siamese.dyndns.org","threadId":"33792","inReplyTo":"51912FEC.6020909@web.de","subject":"Re: [PATCH] t5551: do not use unportable sed '\\+'","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-05-13T18:36:33Z","receivedAt":"2013-05-13T18:36:33Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Torsten Bögershausen writes:\n\n> On 2013-05-13 18.30, Junio C Hamano wrote:\n>> Torsten Bögershausen writes:\n>> \n>>> On 2013-05-13 00.50, Junio C Hamano wrote:\n>>>> Signed-off-by: Junio C Hamano \n>>>> ---\n>>>> t/t5551-http-fetch.sh | 8 ++++++--\n>>>> 1 file changed, 6 insertions(+), 2 deletions(-)\n>>>>\n>>> Thanks, works like a charm, tested on Mac OS.\n>>> /torsten\n>> \n>> Thanks; I take it that you reverted the \"sed fix\" part and saw the\n>> updated \"clone\" check fail with the platform \"sed\"?\n> In my first test with the fix the test case passed.\n> Then the sed expression was only manipulated to verify that the TC failes.\n> And now I took even the original expression and have verfied it is failing.\n>\n> The short version: Yes\n\nThanks.\n"}]}