t9401 fails with OS X sed

I seem to write these kinds of e-mails fairly regularly...

When running t9401-git-cvsserver-crlf:

expecting success: 
    check_status_options cvswork2 textfile.c "" &&
    check_status_options cvswork2 binfile.bin -kb &&
    check_status_options cvswork2 .gitattributes "" &&
    check_status_options cvswork2 mixedUp.c -kb &&
    check_status_options cvswork2 multiline.c -kb &&
    check_status_options cvswork2 multilineTxt.c "" &&
    check_status_options cvswork2/subdir withCr.bin -kb &&
    check_status_options cvswork2 subdir/withCr.bin -kb &&
    check_status_options cvswork2/subdir file.h "" &&
    check_status_options cvswork2 subdir/file.h "" &&
    check_status_options cvswork2/subdir unspecified.other "" &&
    check_status_options cvswork2/subdir newfile.bin "" &&
    check_status_options cvswork2/subdir newfile.c ""

not ok - 12 cvs status - sticky options

I have tracked it down to a sed expression that is parsing the output of cvs status:

49:    got="$(sed -n -e 's/^\s*Sticky Options:\s*//p' "${WORKDIR}/status.out")"

The 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.)  

It works if I change \s to [[:space:]], but I don't know how portable that is.

~~ Brian Gernhardt

On Oct 24, 2012, at 23:54, Brian Gernhardt <brian@gernhardtsoftware.com> wrote:

As \s is shorthand for the POSIX character class [:space:], I'd say the latter should be more portable: anything accepting the shorthand should also accept the full character class. If not, you probably only care about horizontal tab and space, for which you could just use a simple regular expression. Just a literal space and tab character between square brackets is probably going to be most portable, though not most readable.

  -Geert

On Thu, Oct 25, 2012 at 01:04:11AM -0400, Geert Bosch wrote:

I agree that the POSIX character class would be more portable than "\s", but we do not have any existing uses of them, and I would worry a little about older systems like Solaris. If we can simply use a literal space and tab, that seems like the safest.

Brian, can you work up a patch?

-Peff

On 25.10.12 10:41, Jeff King wrote:

Would this be portable: (It works on my Mac OS X box after installing cvs) But I don't have solaris

Sed on Mac OS X doesn't handle \s in a sed expressions so use a more portable character set expression instead.

Signed-off-by: Ben Walton <bdwalton@gmail.com>
---

Hi Torsten,

I think this would be a nicer fix for the issue although your solution should work as well.

Thanks -Ben

 t/t9401-git-cvsserver-crlf.sh |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

On Oct 25, 2012, at 11:58 AM, Ben Walton <bdwalton@gmail.com> wrote:

Acked-by: Brian Gernhardt <brian@gernhardtsoftware.com>

I have an identical change sitting in my git.git, I've just been too distracted by other things to commit and send it.

~~ Brian Gernhardt

On 25.10.12 18:00, Brian Gernhardt wrote:

Much nicer, indeed.

BTW: While we are talking CVS: (I installed a fresh version)
cvs --version
Concurrent Versions System (CVS) 1.11.23 (client/server)

And t9200 fails:
git checkout t9200-git-cvsexportcommit.sh
tb@birne:~/projects/git/git.pu/t> ./t9200-git-cvsexportcommit.sh
cvs [init aborted]: Cannot initialize repository under existing CVSROOT: `/Users/tb/projects/git/git.pu/t/trash directory.t9200-git-cvsexportcommit'
FATAL: Unexpected exit with code 1

The following fixes it, but there are possibly better solutions. Any comments/suggestions ?

diff t9200-git-cvsexportcommit.sh t9200-git-cvsexportcommit.tb.sh 28,29c28 < mkdir "$CVSROOT" && < cvs init && ---

Hi Torsten,

On Thu, Oct 25, 2012 at 5:28 PM, Torsten Bögershausen <tboegi@web.de> wrote:

I have 1.12.13-MirDebian-8 here.

I'm not able to reproduce this manually...are you able to make it fail this way outside of the test harness?

$ CVSROOT=$PWD/bw $ export CVSROOT $ mkdir $CVSROOT && cvs init && echo ok ok $ rm -rf $CVSROOT $ cvs init && echo ok ok

If your version of cvs fails the checks above in manual testing, we could see if there is a flag that works in all (old and new) versions to override the failure if CVSROOT exists. Otherwise, this isn't a bad fix, I don't think.

If your version does fail the manual checks, I think it's likely a regression that was introduced and later reverted. I don't see those strings inside my cvs binary at all...?

HTH.

Thanks -Ben

On 10/25/2012 08:08 PM, Ben Walton wrote:

Hej Ben, thanks for looking into that - here some short answers:

a) The manual test (as you describe it) succeeds b) The test case 9200 failes, and now I know why:

On Thu, Oct 25, 2012 at 04:58:19PM +0100, Ben Walton wrote:

Thanks, I think this simple solution is the best.

-Peff