{"thread":{"id":"31936","subject":"t9401 fails with OS X sed","startedAt":"2012-10-25T03:54:12Z","lastAt":"2012-10-26T12:38:54Z","messageCount":10,"participants":["Brian Gernhardt","Geert Bosch","Jeff King","Torsten Bögershausen","Ben Walton"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"201838","messageId":"609AC6E7-45CD-4472-B1DC-FBB785D6B815@gernhardtsoftware.com","threadId":"31936","inReplyTo":null,"subject":"t9401 fails with OS X sed","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2012-10-25T03:54:12Z","receivedAt":"2012-10-25T03:54:12Z","isPatch":false,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"I seem to write these kinds of e-mails fairly regularly...\n\nWhen running t9401-git-cvsserver-crlf:\n\nexpecting success: \n check_status_options cvswork2 textfile.c \"\" &&\n check_status_options cvswork2 binfile.bin -kb &&\n check_status_options cvswork2 .gitattributes \"\" &&\n check_status_options cvswork2 mixedUp.c -kb &&\n check_status_options cvswork2 multiline.c -kb &&\n check_status_options cvswork2 multilineTxt.c \"\" &&\n check_status_options cvswork2/subdir withCr.bin -kb &&\n check_status_options cvswork2 subdir/withCr.bin -kb &&\n check_status_options cvswork2/subdir file.h \"\" &&\n check_status_options cvswork2 subdir/file.h \"\" &&\n check_status_options cvswork2/subdir unspecified.other \"\" &&\n check_status_options cvswork2/subdir newfile.bin \"\" &&\n check_status_options cvswork2/subdir newfile.c \"\"\n\nnot ok - 12 cvs status - sticky options\n\nI have tracked it down to a sed expression that is parsing the output of cvs status:\n\n49: got=\"$(sed -n -e 's/^\\s*Sticky Options:\\s*//p' \"${WORKDIR}/status.out\")\"\n\nThe problem is that cvs outputs \"Sticky Options:\\t\\t(none)\\n\", but OS X's sed doesn't recognize the \\s shortcut. (According to re_format(5), \\s is part of the \"enhanced\" regex format, which sed doesn't use.) \n\nIt works if I change \\s to [[:space:]], but I don't know how portable that is.\n\n~~ Brian Gernhardt\n"},{"id":"201843","messageId":"F721B376-F4E6-4274-9A6E-BD1CFCBDA39F@adacore.com","threadId":"31936","inReplyTo":"609AC6E7-45CD-4472-B1DC-FBB785D6B815@gernhardtsoftware.com","subject":"Re: t9401 fails with OS X sed","fromName":"Geert Bosch","fromEmail":"bosch@adacore.com","sentAt":"2012-10-25T05:04:11Z","receivedAt":"2012-10-25T05:04:11Z","isPatch":false,"sender":{"key":"bosch@adacore.com","avatar":null},"body":"\nOn Oct 24, 2012, at 23:54, Brian Gernhardt wrote:\n\n> It works if I change \\s to [[:space:]], but I don't know how portable that is.\n\nAs \\s is shorthand for the POSIX character class [:space:], I'd say the latter\nshould be more portable: anything accepting the shorthand should also accept\nthe full character class. If not, you probably only care about horizontal tab\nand space, for which you could just use a simple regular expression. Just a\nliteral space and tab character between square brackets is probably going to be\nmost portable, though not most readable.\n\n -Geert"},{"id":"201868","messageId":"20121025084132.GB8390@sigill.intra.peff.net","threadId":"31936","inReplyTo":"F721B376-F4E6-4274-9A6E-BD1CFCBDA39F@adacore.com","subject":"Re: t9401 fails with OS X sed","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-25T08:41:34Z","receivedAt":"2012-10-25T08:41:34Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 25, 2012 at 01:04:11AM -0400, Geert Bosch wrote:\n\n> On Oct 24, 2012, at 23:54, Brian Gernhardt wrote:\n> \n> > It works if I change \\s to [[:space:]], but I don't know how portable that is.\n> \n> As \\s is shorthand for the POSIX character class [:space:], I'd say the latter\n> should be more portable: anything accepting the shorthand should also accept\n> the full character class. If not, you probably only care about horizontal tab\n> and space, for which you could just use a simple regular expression. Just a\n> literal space and tab character between square brackets is probably going to be\n> most portable, though not most readable.\n\nI agree that the POSIX character class would be more portable than \"\\s\",\nbut we do not have any existing uses of them, and I would worry a little\nabout older systems like Solaris. If we can simply use a literal space\nand tab, that seems like the safest.\n\nBrian, can you work up a patch?\n\n-Peff\n"},{"id":"201899","messageId":"508935CB.9020408@web.de","threadId":"31936","inReplyTo":"20121025084132.GB8390@sigill.intra.peff.net","subject":"Re: t9401 fails with OS X sed","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2012-10-25T12:51:23Z","receivedAt":"2012-10-25T12:51:23Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 25.10.12 10:41, Jeff King wrote:\n> On Thu, Oct 25, 2012 at 01:04:11AM -0400, Geert Bosch wrote:\n> \n>> On Oct 24, 2012, at 23:54, Brian Gernhardt wrote:\n>>\n>>> It works if I change \\s to [[:space:]], but I don't know how portable that is.\n>>\n>> As \\s is shorthand for the POSIX character class [:space:], I'd say the latter\n>> should be more portable: anything accepting the shorthand should also accept\n>> the full character class. If not, you probably only care about horizontal tab\n>> and space, for which you could just use a simple regular expression. Just a\n>> literal space and tab character between square brackets is probably going to be\n>> most portable, though not most readable.\n> \n> I agree that the POSIX character class would be more portable than \"\\s\",\n> but we do not have any existing uses of them, and I would worry a little\n> about older systems like Solaris. If we can simply use a literal space\n> and tab, that seems like the safest.\n> \n> Brian, can you work up a patch?\n> \n> -Peff\n\nWould this be portable:\n(It works on my Mac OS X box after installing cvs)\nBut I don't have solaris\n\n\ndiff --git a/t/t9401-git-cvsserver-crlf.sh b/t/t9401-git-cvsserver-crlf.sh\nindex cdb8360..f2ec9d2 100755\n--- a/t/t9401-git-cvsserver-crlf.sh\n+++ b/t/t9401-git-cvsserver-crlf.sh\n@@ -46,7 +46,7 @@ check_status_options() {\n echo \"Error from cvs status: $1 $2\" >> \"${WORKDIR}/marked.log\"\n return 1;\n fi\n- got=\"$(sed -n -e 's/^\\s*Sticky Options:\\s*//p' \"${WORKDIR}/status.out\")\"\n+ got=\"$(tr '\\t' ' ' < \"${WORKDIR}/status.out\" | sed -n -e 's/^ *Sticky Options: *//p')\"\n expect=\"$3\"\n if [ x\"$expect\" = x\"\" ] ; then\n expect=\"(none)\"\n"},{"id":"201905","messageId":"1351180699-24695-1-git-send-email-bdwalton@gmail.com","threadId":"31936","inReplyTo":"508935CB.9020408@web.de","subject":"[PATCH] Use character class for sed expression instead of \\s","fromName":"Ben Walton","fromEmail":"bdwalton@gmail.com","sentAt":"2012-10-25T15:58:19Z","receivedAt":"2012-10-25T15:58:19Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Sed on Mac OS X doesn't handle \\s in a sed expressions so use a more\nportable character set expression instead.\n\nSigned-off-by: Ben Walton \n---\n\nHi Torsten,\n\nI think this would be a nicer fix for the issue although your solution\nshould work as well.\n\nThanks\n-Ben\n\n t/t9401-git-cvsserver-crlf.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t9401-git-cvsserver-crlf.sh b/t/t9401-git-cvsserver-crlf.sh\nindex cdb8360..1c5bc84 100755\n--- a/t/t9401-git-cvsserver-crlf.sh\n+++ b/t/t9401-git-cvsserver-crlf.sh\n@@ -46,7 +46,7 @@ check_status_options() {\n \techo \"Error from cvs status: $1 $2\" >> \"${WORKDIR}/marked.log\"\n \treturn 1;\n fi\n- got=\"$(sed -n -e 's/^\\s*Sticky Options:\\s*//p' \"${WORKDIR}/status.out\")\"\n+ got=\"$(sed -n -e 's/^[ \t]*Sticky Options:[ \t]*//p' \"${WORKDIR}/status.out\")\"\n expect=\"$3\"\n if [ x\"$expect\" = x\"\" ] ; then\n \texpect=\"(none)\"\n-- \n1.7.9.5\n"},{"id":"201906","messageId":"C2AB6973-7BC2-45A4-836E-BB1FAAE7501C@gernhardtsoftware.com","threadId":"31936","inReplyTo":"1351180699-24695-1-git-send-email-bdwalton@gmail.com","subject":"Re: [PATCH] Use character class for sed expression instead of \\s","fromName":"Brian Gernhardt","fromEmail":"brian@gernhardtsoftware.com","sentAt":"2012-10-25T16:00:46Z","receivedAt":"2012-10-25T16:00:46Z","isPatch":true,"sender":{"key":"brian@gernhardtsoftware.com","avatar":"https://avatars.githubusercontent.com/u/133455?v=4"},"body":"\nOn Oct 25, 2012, at 11:58 AM, Ben Walton wrote:\n\n> Sed on Mac OS X doesn't handle \\s in a sed expressions so use a more\n> portable character set expression instead.\n> \n> Signed-off-by: Ben Walton \n\nAcked-by: Brian Gernhardt \n\nI have an identical change sitting in my git.git, I've just been too distracted by other things to commit and send it.\n\n~~ Brian Gernhardt\n"},{"id":"201909","messageId":"5089689A.9070301@web.de","threadId":"31936","inReplyTo":"C2AB6973-7BC2-45A4-836E-BB1FAAE7501C@gernhardtsoftware.com","subject":"Re: [PATCH] Use character class for sed expression instead of \\s","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2012-10-25T16:28:10Z","receivedAt":"2012-10-25T16:28:10Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 25.10.12 18:00, Brian Gernhardt wrote:\n> \n> On Oct 25, 2012, at 11:58 AM, Ben Walton wrote:\n> \n>> Sed on Mac OS X doesn't handle \\s in a sed expressions so use a more\n>> portable character set expression instead.\n>>\n>> Signed-off-by: Ben Walton \n> \n> Acked-by: Brian Gernhardt \n> \n> I have an identical change sitting in my git.git, I've just been too distracted by other things to commit and send it.\n> \n> ~~ Brian Gernhardt\n\nMuch nicer, indeed.\n\nBTW: While we are talking CVS: (I installed a fresh version)\ncvs --version\nConcurrent Versions System (CVS) 1.11.23 (client/server)\n\n\nAnd t9200 fails:\ngit checkout t9200-git-cvsexportcommit.sh\ntb@birne:~/projects/git/git.pu/t> ./t9200-git-cvsexportcommit.sh\ncvs [init aborted]: Cannot initialize repository under existing CVSROOT: `/Users/tb/projects/git/git.pu/t/trash directory.t9200-git-cvsexportcommit'\nFATAL: Unexpected exit with code 1\n\nThe following fixes it, but there are possibly better solutions.\nAny comments/suggestions ?\n\ndiff t9200-git-cvsexportcommit.sh t9200-git-cvsexportcommit.tb.sh\n28,29c28\n< mkdir \"$CVSROOT\" &&\n< cvs init &&\n---\n> (cvs init || mkdir \"$CVSROOT\" && cvs init ) &&\n"},{"id":"201915","messageId":"CAP30j15n1hVn6zptDpAfM+Aqc3LnRR4PN6jHTHpTkcjYLgPnjw@mail.gmail.com","threadId":"31936","inReplyTo":"5089689A.9070301@web.de","subject":"Re: [PATCH] Use character class for sed expression instead of \\s","fromName":"Ben Walton","fromEmail":"bdwalton@gmail.com","sentAt":"2012-10-25T18:08:53Z","receivedAt":"2012-10-25T18:08:53Z","isPatch":true,"sender":{"key":"bdwalton@gmail.com","avatar":"https://avatars.githubusercontent.com/u/396061?v=4"},"body":"Hi Torsten,\n\nOn Thu, Oct 25, 2012 at 5:28 PM, Torsten Bögershausen wrote:\n\n> BTW: While we are talking CVS: (I installed a fresh version)\n> cvs --version\n> Concurrent Versions System (CVS) 1.11.23 (client/server)\n\nI have 1.12.13-MirDebian-8 here.\n\n> And t9200 fails:\n> git checkout t9200-git-cvsexportcommit.sh\n> tb@birne:~/projects/git/git.pu/t> ./t9200-git-cvsexportcommit.sh\n> cvs [init aborted]: Cannot initialize repository under existing CVSROOT: `/Users/tb/projects/git/git.pu/t/trash directory.t9200-git-cvsexportcommit'\n> FATAL: Unexpected exit with code 1\n\nI'm not able to reproduce this manually...are you able to make it fail\nthis way outside of the test harness?\n\n$ CVSROOT=$PWD/bw\n$ export CVSROOT\n$ mkdir $CVSROOT && cvs init && echo ok\nok\n$ rm -rf $CVSROOT\n$ cvs init && echo ok\nok\n\n>> (cvs init || mkdir \"$CVSROOT\" && cvs init ) &&\n\nIf your version of cvs fails the checks above in manual testing, we\ncould see if there is a flag that works in all (old and new) versions\nto override the failure if CVSROOT exists. Otherwise, this isn't a\nbad fix, I don't think.\n\nIf your version does fail the manual checks, I think it's likely a\nregression that was introduced and later reverted. I don't see those\nstrings inside my cvs binary at all...?\n\nHTH.\n\nThanks\n-Ben\n-- \n---------------------------------------------------------------------------------------------------------------------------\nTake the risk of thinking for yourself. Much more happiness,\ntruth, beauty and wisdom will come to you that way.\n\n-Christopher Hitchens\n---------------------------------------------------------------------------------------------------------------------------\n"},{"id":"201918","messageId":"50899C83.6090008@web.de","threadId":"31936","inReplyTo":"CAP30j15n1hVn6zptDpAfM+Aqc3LnRR4PN6jHTHpTkcjYLgPnjw@mail.gmail.com","subject":"Re: [PATCH] Use character class for sed expression instead of \\s","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2012-10-25T20:09:39Z","receivedAt":"2012-10-25T20:09:39Z","isPatch":true,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 10/25/2012 08:08 PM, Ben Walton wrote:\n> Hi Torsten,\n>\n> On Thu, Oct 25, 2012 at 5:28 PM, Torsten Bögershausen wrote:\n>\n>> BTW: While we are talking CVS: (I installed a fresh version)\n>> cvs --version\n>> Concurrent Versions System (CVS) 1.11.23 (client/server)\n>\n> I have 1.12.13-MirDebian-8 here.\n>\n>> And t9200 fails:\n>> git checkout t9200-git-cvsexportcommit.sh\n>> tb@birne:~/projects/git/git.pu/t> ./t9200-git-cvsexportcommit.sh\n>> cvs [init aborted]: Cannot initialize repository under existing CVSROOT: `/Users/tb/projects/git/git.pu/t/trash directory.t9200-git-cvsexportcommit'\n>> FATAL: Unexpected exit with code 1\n>\n> I'm not able to reproduce this manually...are you able to make it fail\n> this way outside of the test harness?\n>\n> $ CVSROOT=$PWD/bw\n> $ export CVSROOT\n> $ mkdir $CVSROOT && cvs init && echo ok\n> ok\n> $ rm -rf $CVSROOT\n> $ cvs init && echo ok\n> ok\n>\n>>> (cvs init || mkdir \"$CVSROOT\" && cvs init ) &&\n>\n> If your version of cvs fails the checks above in manual testing, we\n> could see if there is a flag that works in all (old and new) versions\n> to override the failure if CVSROOT exists. Otherwise, this isn't a\n> bad fix, I don't think.\n>\n> If your version does fail the manual checks, I think it's likely a\n> regression that was introduced and later reverted. I don't see those\n> strings inside my cvs binary at all...?\n>\n> HTH.\n>\n> Thanks\n> -Ben\n>\nHej Ben,\nthanks for looking into that - here some short answers:\n\na) The manual test (as you describe it) succeeds\nb) The test case 9200 failes, and now I know why:\n\ndiff --git a/t/t9200-git-cvsexportcommit.sh b/t/t9200-git-cvsexportcommit.sh\nindex b59be9a..d2c3c37 100755\n--- a/t/t9200-git-cvsexportcommit.sh\n+++ b/t/t9200-git-cvsexportcommit.sh\n@@ -19,7 +19,7 @@ then\n test_done\n fi\n\n-CVSROOT=$PWD/cvsroot\n+CVSROOT=$PWD/xx\n CVSWORK=$PWD/cvswork\n GIT_DIR=$PWD/.git\n export CVSROOT CVSWORK GIT_DIR\n\n\n\nc) I need to send a patch tomorrow\n\nd) FYI: I compiled cvs from scratch, from a file called cvs-1.11.23.tar.gz\n and the code is in cvs-1.11.23/src/mkmodules.c:942\n\nif (root_dir && strcmp (root_dir, current_parsed_root->directory))\n error (1, 0,\n \"Cannot initialize repository under existing CVSROOT: `%s'\",\n root_dir);\n free (root_dir);\n\n/Torsten\n"},{"id":"201937","messageId":"20121026123854.GB1455@sigill.intra.peff.net","threadId":"31936","inReplyTo":"1351180699-24695-1-git-send-email-bdwalton@gmail.com","subject":"Re: [PATCH] Use character class for sed expression instead of \\s","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-10-26T12:38:54Z","receivedAt":"2012-10-26T12:38:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Oct 25, 2012 at 04:58:19PM +0100, Ben Walton wrote:\n\n> Sed on Mac OS X doesn't handle \\s in a sed expressions so use a more\n> portable character set expression instead.\n> \n> Signed-off-by: Ben Walton \n\nThanks, I think this simple solution is the best.\n\n-Peff\n"}]}