Bug #71847 for ExtUtils-MakeMaker: Invalid nmake syntax in abstract

Fri Oct 21 15:29:31 2011 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Ticket created

Subject:

Invalid nmake syntax in abstract

If the module’s abstract contains $[, then nmake complains with: makefile(928) : fatal error U1001: syntax error : illegal character '[' in macro (Stupid nmake!) How does one escape dollar signs in make syntax? Would it be a good idea to escape them in ExtUtils::MM_Unix::ppd? This problem also affects modules with abstracts like this: =head1 NAME Nifty::Make::DollarRule - $(foo) rules for Nifty::Make (Just made up as an example.)

Fri Oct 21 19:30:23 2011 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Fri Oct 21 15:29:31 2011, SPROUT wrote: Show quoted text

> If the module’s abstract contains $[, then nmake complains with: > > makefile(928) : fatal error U1001: syntax error : illegal character > '[' in macro > > (Stupid nmake!) > > How does one escape dollar signs in make syntax? Would it be a good > idea to escape them in > ExtUtils::MM_Unix::ppd? This problem also affects modules with > abstracts like this: > > =head1 NAME > > Nifty::Make::DollarRule - $(foo) rules for Nifty::Make > > (Just made up as an example.)

Attached is a patch to fix this, using the $$ escape that I’ve just learnt about.

Subject:

open_MQfnpvdD.txt

From: Father Chrysostomos <sprout@cpan.org> [rt.cpan.org #71847] Escape $ signs in abstract and authors diff -rup ExtUtils-MakeMaker-6.59-W48nnf/lib/ExtUtils/MM_Unix.pm ExtUtils-MakeMaker-6.59-W48nnf-copy/lib/ExtUtils/MM_Unix.pm --- ExtUtils-MakeMaker-6.59-W48nnf/lib/ExtUtils/MM_Unix.pm 2011-08-05 04:13:10.000000000 -0700 +++ ExtUtils-MakeMaker-6.59-W48nnf-copy/lib/ExtUtils/MM_Unix.pm 2011-10-21 16:25:47.000000000 -0700 @@ -2890,10 +2890,12 @@ sub ppd { $abstract =~ s/\n/\\n/sg; $abstract =~ s/</</g; $abstract =~ s/>/>/g; + $abstract =~ s/\$/\$\$/g; my $author = join(', ',@{$self->{AUTHOR} || []}); $author =~ s/</</g; $author =~ s/>/>/g; + $author =~ s/\$/\$\$/g; my $ppd_xml = sprintf <<'PPD_HTML', $self->{VERSION}, $abstract, $author; <SOFTPKG NAME="$(DISTNAME)" VERSION="%s"> diff -rup ExtUtils-MakeMaker-6.59-W48nnf/t/basic.t ExtUtils-MakeMaker-6.59-W48nnf-copy/t/basic.t --- ExtUtils-MakeMaker-6.59-W48nnf/t/basic.t 2011-08-02 08:48:10.000000000 -0700 +++ ExtUtils-MakeMaker-6.59-W48nnf-copy/t/basic.t 2011-10-21 16:28:04.000000000 -0700 @@ -82,7 +82,8 @@ my $ppd_html; close PPD; like( $ppd_html, qr{^<SOFTPKG NAME="Big-Dummy" VERSION="0.01">}m, ' <SOFTPKG>' ); -like( $ppd_html, qr{^\s*<ABSTRACT>Try "our" hot dog's</ABSTRACT>}m, +like( $ppd_html, + qr{^\s*<ABSTRACT>Try "our" hot dog's \$andwiche\$</ABSTRACT>}m, ' <ABSTRACT>'); like( $ppd_html, qr{^\s*<AUTHOR>Michael G Schwern <schwern\@pobox.com></AUTHOR>}m, diff -rup ExtUtils-MakeMaker-6.59-W48nnf/t/lib/MakeMaker/Test/Setup/BFD.pm ExtUtils-MakeMaker-6.59-W48nnf-copy/t/lib/MakeMaker/Test/Setup/BFD.pm --- ExtUtils-MakeMaker-6.59-W48nnf/t/lib/MakeMaker/Test/Setup/BFD.pm 2011-02-07 08:36:59.000000000 -0800 +++ ExtUtils-MakeMaker-6.59-W48nnf-copy/t/lib/MakeMaker/Test/Setup/BFD.pm 2011-10-21 16:18:33.000000000 -0700 @@ -19,7 +19,7 @@ $VERSION = 0.01; =head1 NAME -Big::Dummy - Try "our" hot dog's +Big::Dummy - Try "our" hot dog's $andwiche$ =cut

Fri Oct 21 19:30:26 2011 The RT System itself - Status changed from 'new' to 'open'

Sat Oct 22 21:54:41 2011 mschwern [...] cpan.org - Correspondence added

Thanks for the patch. A bit more work was necessary. It seems the problem is endemic through the system, not just PPD, so I added dollar sign quoting to quote_literal() which handles most Makefile quoting. See https://github.com/Perl-Toolchain-Gang/ExtUtils-MakeMaker/commit/ce77326378c41d02c8204fb9dfb8aa8f117c9597

Sat Oct 22 21:54:43 2011 mschwern [...] cpan.org - Status changed from 'open' to 'resolved'

Sun Oct 23 01:46:22 2011 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Sat Oct 22 21:54:41 2011, MSCHWERN wrote: Show quoted text

> Thanks for the patch. > > A bit more work was necessary. It seems the problem is endemic > through > the system, not just PPD, so I added dollar sign quoting to > quote_literal() which handles most Makefile quoting. See > https://github.com/Perl-Toolchain-Gang/ExtUtils- > MakeMaker/commit/ce77326378c41d02c8204fb9dfb8aa8f117c9597

For abstract and authors, shouldn’t $(foo) also be escaped?

Sun Oct 23 01:46:24 2011 The RT System itself - Status changed from 'resolved' to 'open'

Sun Oct 23 03:55:48 2011 schwern [...] pobox.com - Correspondence added

Subject:	Re: [rt.cpan.org #71847] Invalid nmake syntax in abstract
Date:	Sun, 23 Oct 2011 00:55:30 -0700
To:	bug-ExtUtils-MakeMaker [...] rt.cpan.org
From:	Michael G Schwern <schwern [...] pobox.com>

On 2011.10.22 10:46 PM, Father Chrysostomos via RT wrote: Show quoted text

> For abstract and authors, shouldn’t $(foo) also be escaped?

True. Pondering how to best do that.

Sun Oct 23 14:54:49 2011 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Sun Oct 23 03:55:48 2011, schwern@pobox.com wrote: Show quoted text

> On 2011.10.22 10:46 PM, Father Chrysostomos via RT wrote:

> > For abstract and authors, shouldn’t $(foo) also be escaped?

> > True. Pondering how to best do that.

Yes, the trouble is that ppd calls echo, which calls quote_literal, which calls escape_dollarsigns. So there’s no easy way for ppd to bypass escape_dollarsigns. Let me think: $$(foo) should become $$$$(foo), not $$$(foo). So if escape_dollarsigns produces the latter, then ppd can do: s/([^\$]|\$\$)|(\$)/$1 ? $1 : '$$'/ge for @ppd_cmds; For speed, we can unroll the loop, à la the Owl Book: s/((?>[^\$]*(?:\$\$[^\$]*)*))\$/$1\$\$/g for @ppd_cmds; Not pretty, but it works.

Sun Oct 23 15:02:41 2011 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Sun Oct 23 14:54:49 2011, SPROUT wrote: Show quoted text

> On Sun Oct 23 03:55:48 2011, schwern@pobox.com wrote:

> > On 2011.10.22 10:46 PM, Father Chrysostomos via RT wrote:

> > > For abstract and authors, shouldn’t $(foo) also be escaped?

> > > > True. Pondering how to best do that.

> > Yes, the trouble is that ppd calls echo, which calls quote_literal, > which calls > escape_dollarsigns. > > So there’s no easy way for ppd to bypass escape_dollarsigns. > > Let me think: $$(foo) should become $$$$(foo), not $$$(foo). > > So if escape_dollarsigns produces the latter, then ppd can do: > > s/([^\$]|\$\$)|(\$)/$1 ? $1 : '$$'/ge for @ppd_cmds; > > For speed, we can unroll the loop, à la the Owl Book: > > s/((?>[^\$]*(?:\$\$[^\$]*)*))\$/$1\$\$/g for @ppd_cmds; > > Not pretty, but it works.

Actually, it doesn’t. That will also escape $(DISTNAME). s~((?>[^\$]*(?:\$\$[^\$]*)*))\$~$1\$\$(?=.*</A[UB])~g for @ppd_cmds; Ugh! Now that is getting really ugly. But what else can we do?

Sun Oct 23 15:41:50 2011 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Sun Oct 23 15:02:41 2011, SPROUT wrote: Show quoted text

> On Sun Oct 23 14:54:49 2011, SPROUT wrote:

> > On Sun Oct 23 03:55:48 2011, schwern@pobox.com wrote:

> > > On 2011.10.22 10:46 PM, Father Chrysostomos via RT wrote:

> > > > For abstract and authors, shouldn’t $(foo) also be escaped?

> > > > > > True. Pondering how to best do that.

> > > > Yes, the trouble is that ppd calls echo, which calls quote_literal, > > which calls > > escape_dollarsigns. > > > > So there’s no easy way for ppd to bypass escape_dollarsigns. > > > > Let me think: $$(foo) should become $$$$(foo), not $$$(foo). > > > > So if escape_dollarsigns produces the latter, then ppd can do: > > > > s/([^\$]|\$\$)|(\$)/$1 ? $1 : '$$'/ge for @ppd_cmds; > > > > For speed, we can unroll the loop, à la the Owl Book: > > > > s/((?>[^\$]*(?:\$\$[^\$]*)*))\$/$1\$\$/g for @ppd_cmds; > > > > Not pretty, but it works.

> > Actually, it doesn’t. That will also escape $(DISTNAME). > > s~((?>[^\$]*(?:\$\$[^\$]*)*))\$~$1\$\$(?=.*</A[UB])~g for @ppd_cmds; > > Ugh! Now that is getting really ugly. But what else can we do?

We can fix it. That’s what. :-) That second version still escapes $(ECHO). s~<(AUTHOR|ABSTRACT)>(.*?)</\1>~ my($tag,$tmp) = ($1,$2); $tmp =~ s/((?>[^\$]*(?:\$\$[^\$]*)*))\$/$1\$\$/g; "<$tag>$tmp</$tag>"; ~e for @ppd_cmds;

Sun Oct 23 17:33:19 2011 schwern [...] pobox.com - Correspondence added

Subject:	Re: [rt.cpan.org #71847] Invalid nmake syntax in abstract
Date:	Sun, 23 Oct 2011 14:33:07 -0700
To:	bug-ExtUtils-MakeMaker [...] rt.cpan.org
From:	Michael G Schwern <schwern [...] pobox.com>

On 2011.10.23 12:41 PM, Father Chrysostomos via RT wrote: Show quoted text

> We can fix it. That’s what. :-) > > That second version still escapes $(ECHO). > > s~<(AUTHOR|ABSTRACT)>(.*?)</\1>~ > my($tag,$tmp) = ($1,$2); > $tmp =~ s/((?>[^\$]*(?:\$\$[^\$]*)*))\$/$1\$\$/g; > "<$tag>$tmp</$tag>"; > ~e for @ppd_cmds;

I appreciate the try... that's, uhh... quite the impressive bit of regex magic. I like how you used ~ instead of {} for delimiters to maximize confusion. :P But it can't be used, sorry. MakeMaker is trying to move away from hacking already formatted content. Makes it far easier to understand and debug and generally cause less headaches. I think echo() should be changed to escape everything. That would match its existing uses, outputting PPD and the META files, and a safer default. Looking at how it's used in the wild, that would seem to be what everyone else expects (if they're thinking about escapes at all... which they're not). [1] A version of echo that preserves variables can be written when necessary. [1] http://www.google.com/codesearch#search/&q=%5C-%5C%3Eecho%5Cb%20lang:perl%20makemaker&p=1&type=cs -- 151. The proper way to report to my Commander is "Specialist Schwarz, reporting as ordered, Sir" not "You can't prove a thing!" -- The 213 Things Skippy Is No Longer Allowed To Do In The U.S. Army http://skippyslist.com/list/

Sun Oct 23 17:55:55 2011 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Sun Oct 23 17:33:19 2011, schwern@pobox.com wrote: Show quoted text

> On 2011.10.23 12:41 PM, Father Chrysostomos via RT wrote:

> > We can fix it. That’s what. :-) > > > > That second version still escapes $(ECHO). > > > > s~<(AUTHOR|ABSTRACT)>(.*?)</\1>~ > > my($tag,$tmp) = ($1,$2); > > $tmp =~ s/((?>[^\$]*(?:\$\$[^\$]*)*))\$/$1\$\$/g; > > "<$tag>$tmp</$tag>"; > > ~e for @ppd_cmds;

> > I appreciate the try... that's, uhh... quite the impressive bit of > regex > magic.

I just wanted to have a bit of fun on a Sunday afternoon. :-) Show quoted text

> I like how you used ~ instead of {} for delimiters to maximize > confusion. :P But it can't be used, sorry. MakeMaker is trying to > move away > from hacking already formatted content. Makes it far easier to > understand and > debug and generally cause less headaches. > > I think echo() should be changed to escape everything. That would > match its > existing uses, outputting PPD and the META files, and a safer default. > Looking at how it's used in the wild, that would seem to be what > everyone else > expects (if they're thinking about escapes at all... which they're > not). [1] >

ppd has other problems, too. It does s/\n/\\n/, which can’t be right. It does not escape &. The fix that has been applied is already sufficient for my purposes (see http://perl5.git.perl.org/perl.git/blob/b82b06b8ca:/ext/arybase/Makefile.PL, which can be deleted when EUMM is updated), so I think I’ll let this wait till another day. BTW, while I have your attention, have you seen <https://rt.cpan.org/Ticket/Display.html? id=67538>?

Sun Oct 23 18:09:56 2011 schwern [...] pobox.com - Correspondence added

Subject:	Re: [rt.cpan.org #71847] Invalid nmake syntax in abstract
Date:	Sun, 23 Oct 2011 15:09:42 -0700
To:	bug-ExtUtils-MakeMaker [...] rt.cpan.org
From:	Michael G Schwern <schwern [...] pobox.com>

On 2011.10.23 2:33 PM, Michael G Schwern via RT wrote: Show quoted text

> I think echo() should be changed to escape everything. That would match its > existing uses, outputting PPD and the META files, and a safer default. > Looking at how it's used in the wild, that would seem to be what everyone else > expects (if they're thinking about escapes at all... which they're not). [1]

PS I'm fixing that up right now. -- I'm pale as Formica, social skills stunted small. But I'm accurate like a pica, I know the capital of Nepal. I'm the nemesis of error, dreadful diction fears my skills, more inquisitive than Jim Lehrer, snottier than Beverly Hills. -- I.L.O.P. Secret Rap http://goats.com/archive/020830.html

Sun Oct 23 19:02:22 2011 mschwern [...] cpan.org - Correspondence added

Ok, that should do it. Improved a bunch of things in the process. The test for echo() came out pretty good. https://github.com/Perl-Toolchain-Gang/ExtUtils-MakeMaker/commits/c495e3ab4f2147c7fc37fc9b440debd0ea0552a5 covers it. If you're happy I'll release an alpha, there's a bunch of internal changes I want checked out asap.

Sun Oct 23 19:02:23 2011 mschwern [...] cpan.org - Status changed from 'open' to 'patched'

Sun Oct 23 19:34:18 2011 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Sun Oct 23 19:02:22 2011, MSCHWERN wrote: Show quoted text

> Ok, that should do it. Improved a bunch of things in the process. > The > test for echo() came out pretty good. > > https://github.com/Perl-Toolchain-Gang/ExtUtils- > MakeMaker/commits/c495e3ab4f2147c7fc37fc9b440debd0ea0552a5 > covers it. If you're happy I'll release an alpha, there's a bunch of > internal changes I want checked out asap.

Looks good to me! (P.S.: I hope tho’se “are” vegetarian hot dog’s.)

Sun Oct 23 19:34:19 2011 The RT System itself - Status changed from 'patched' to 'open'

Sun Oct 23 19:39:27 2011 mschwern [...] cpan.org - Correspondence added

On Sun Oct 23 19:34:18 2011, SPROUT wrote: Show quoted text

> (P.S.: I hope tho’se “are” vegetarian hot dog’s.)

Find a bug involving the letter V and I'll even make them vegan.

Sun Oct 23 19:39:29 2011 mschwern [...] cpan.org - Status changed from 'open' to 'resolved'