Fix build [#22]

[genehack]

Not sure what the removed parenthetical is supposed to be doing, but based on the values I see in , it won't ever match.

This change fixes the build issues I see on both MacOS X and Linux.

[coveralls]

Coverage remained the same at 96.833% when pulling 85a4e96 on genehack:fix-build-22 into 0a9e56b on sass:develop.

[mgreter]

I guess I see what might be the issue. Can you try the following patch:

-       s/\$\*\.c(pp|xx)(?=\n|\Z)/-xc++ -std=c++0x \$\*\.c$1/g
+       s/\$\*\.c(pp|xx)(?=\n|\r|\Z)/-xc++ -std=c++0x \$\*\.c$1/g

The regex tries to match .cpp only before newlines (or end of file; without replacing the actual linefeed character) and it looks like I forgot that there are systems using more "esoteric" linefeeds 👊. Funny since the current regex is working on windows (at least on my multiple systems). If the diff above does not work, can you try to print the string in $_ at that stage, because it really seems to be a simple issue with the regex, and I would like to keep it "strict" to avoid "false" matches.

Thanks!

[genehack]

That change also works. If what you really care about is EOL matching, however, why not the (much easier to read, IMO):

s/\$\*\.c(pp|xx)$/-xc++ -std=c++0x \$\*\.c$1/gm

It's really not clear to me why you're including \Z, for example, unless you expect that string to completely lack a trailing newline.

Here's what's in @rv, FWIW:

.c.i:
        cc  -E -c $(PASTHRU_INC) $(INC) \
        $(CCFLAGS) $(OPTIMIZE) \
        $(PERLTYPE) $(MPOLLUTE) $(DEFINE_VERSION) \
        $(XS_DEFINE_VERSION) $(CCCDLFLAGS) "-I$(PERL_INC)" $(PASTHRU_DEFINE) $(DEFINE) $*.c > $*.i

.c.s:
        $(CCCMD) -S $(CCCDLFLAGS) "-I$(PERL_INC)" $(PASTHRU_DEFINE) $(DEFINE) $*.c

.c$(OBJ_EXT):
        $(CCCMD) $(CCCDLFLAGS) "-I$(PERL_INC)" $(PASTHRU_DEFINE) $(DEFINE) $*.c

.cpp$(OBJ_EXT):
        $(CCCMD) $(CCCDLFLAGS) "-I$(PERL_INC)" $(PASTHRU_DEFINE) $(DEFINE) $*.cpp

.cxx$(OBJ_EXT):
        $(CCCMD) $(CCCDLFLAGS) "-I$(PERL_INC)" $(PASTHRU_DEFINE) $(DEFINE) $*.cxx

.cc$(OBJ_EXT):
        $(CCCMD) $(CCCDLFLAGS) "-I$(PERL_INC)" $(PASTHRU_DEFINE) $(DEFINE) $*.cc

.C$(OBJ_EXT):
        $(CCCMD) $(CCCDLFLAGS) "-I$(PERL_INC)" $(PASTHRU_DEFINE) $(DEFINE) $*.C

[genehack]

I should also note that, based on conversation within irc.perl.org#toolchain, this bug depends on the installed version of ExtUtils::MakeMaker -- it will not trigger with version 7.10, but will will with anything more recent.

[genehack]

My apologies -- I was accidentally testing your patch, above, with EUMM 7.10.

Once I upgraded back to EUMM 7.16, both your suggested patch and my suggested alternative DO NOT work. My original patch, removing the EOL look-ahead, does work with EUMM 7.16.

[mgreter]

TBH, I always struggle to remember what $ is really matching (since depending on multiline flag and implications). So I tend to simply name the chars I want it to match, I just find it easier to read TBH ;) But I agree that what you posted above is probably the "most correct" way to do it. If you can sucessfully test it I will gladly merge the updated PR. And also thx for the info on ExtUtils::MakeMaker.

[genehack]

Per BinGOs in #toolchain, the issue is a trailing space is being added. This patch:

-               s/\$\*\.c(pp|xx)(?=\n|\Z)/-xc++ -std=c++0x \$\*\.c$1/g
+               s/\$\*\.c(pp|xx)\s*(?=\n|\Z)/-xc++ -std=c++0x \$\*\.c$1/g

works around that.

[mgreter]

Hmm, maybe there is some space before the newline. TBH if it turns out to be too complicated your first approach will probably just do it. I guess there should not be to many possibilities for false matches. But I am somewhat curious why it should not match the output you posted above for $_.

[mgreter]

Can you update and force push this PR? And maybe also include the \r just in case ;)

[genehack]

Force pushed with the addition of the \s* and the \r in the EOL look-ahead.

[mgreter]

Why did you change c++0x to c++2x? Oversight? Not sure that even exists?

[genehack]

crap, that's just a dumb typo. 1 sec.

[genehack]

Force pushed a version that doesn't have that '2' in there 8^/

[genehack]

And force-pushed one more, taking the m back off the regex modifiers. I think it's about "stop coding" o'clock now...

[mgreter]

Just read perlre again ( 😉) and the regex could probably even be further optimized (\s means the five characters [ \f\n\r\t]). Since the star makes it greedy it might eat empty lines, but this should not stop it from working. So it's good enough for me. Let's see if Travis CI is happy and I'll merge! 🚢

[coveralls]

Coverage remained the same at 96.833% when pulling fdf5180 on genehack:fix-build-22 into 0a9e56b on sass:develop.

[mgreter]

Thanks for your report, troubleshooting and PR!