Bleadperl v5.19.2-257-gc30fc27 breaks FANGLY/Getopt-Euclid-0.4.3.tar.gz

p5pRT commented 11 years ago

Migrated from rt.perl.org#119125 (status was 'resolved')

Searchable as RT119125$

p5pRT commented 11 years ago

From @andk

git bisect

commit c30fc27b4df65a43710b25dd1d2a57d78ee2fe33 Author: David Mitchell \davem@iabyn\.com Date: Wed Jul 31 22:41:17 2013 +0100

Handle /[#]/ and /[(?#]/ with code blocks

diagnostics

http://www.cpantesters.org/cpan/report/ee513274-fa8f-11e2-af9a-37acf1ff63fb

perl -V

Summary of my perl5 (revision 5 version 19 subversion 3) configuration:
Commit id: c30fc27b4df65a43710b25dd1d2a57d78ee2fe33 Platform: osname=linux\, osvers=3.9-1-amd64\, archname=x86_64-linux uname='linux k83 3.9-1-amd64 #1 smp debian 3.9.8-1 x86_64 gnulinux ' config_args='-Dprefix=/home/sand/src/perl/repoperls/installed-perls/perl/v5.19.2-257-gc30fc27/165a -Dmyhostname=k83 -Dinstallusrbinperl=n -Uversiononly -Dusedevel -des -Ui_db -Uuseithreads -Uuselongdouble -DDEBUGGING=-g' hint=recommended\, useposix=true\, d_sigaction=define useithreads=undef\, usemultiplicity=undef useperlio=define\, d_sfio=undef\, uselargefiles=define\, usesocks=undef use64bitint=define\, use64bitall=define\, uselongdouble=undef usemymalloc=n\, bincompat5005=undef Compiler: cc='cc'\, ccflags ='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'\, optimize='-O2 -g'\, cppflags='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include' ccversion=''\, gccversion='4.8.1'\, gccosandvers='' intsize=4\, longsize=8\, ptrsize=8\, doublesize=8\, byteorder=12345678 d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=16 ivtype='long'\, ivsize=8\, nvtype='double'\, nvsize=8\, Off_t='off_t'\, lseeksize=8 alignbytes=8\, prototype=define Linker and Libraries: ld='cc'\, ldflags =' -fstack-protector -L/usr/local/lib' libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib libs=-lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc libc=\, so=so\, useshrplib=false\, libperl=libperl.a gnulibc_version='2.17' Dynamic Linking: dlsrc=dl_dlopen.xs\, dlext=so\, d_dlsymun=undef\, ccdlflags='-Wl\,-E' cccdlflags='-fPIC'\, lddlflags='-shared -O2 -g -L/usr/local/lib -fstack-protector'

Characteristics of this binary (from libperl): Compile-time options: HAS_TIMES PERLIO_LAYERS PERL_DONT_CREATE_GVSV PERL_HASH_FUNC_ONE_AT_A_TIME_HARD PERL_MALLOC_WRAP PERL_NEW_COPY_ON_WRITE PERL_PRESERVE_IVUV PERL_USE_DEVEL USE_64_BIT_ALL USE_64_BIT_INT USE_LARGE_FILES USE_LOCALE USE_LOCALE_COLLATE USE_LOCALE_CTYPE USE_LOCALE_NUMERIC USE_PERLIO USE_PERL_ATOF Built under linux Compiled at Aug 2 2013 09:57:10 @INC: /home/sand/src/perl/repoperls/installed-perls/perl/v5.19.2-257-gc30fc27/165a/lib/site_perl/5.19.3/x86_64-linux /home/sand/src/perl/repoperls/installed-perls/perl/v5.19.2-257-gc30fc27/165a/lib/site_perl/5.19.3 /home/sand/src/perl/repoperls/installed-perls/perl/v5.19.2-257-gc30fc27/165a/lib/5.19.3/x86_64-linux /home/sand/src/perl/repoperls/installed-perls/perl/v5.19.2-257-gc30fc27/165a/lib/5.19.3 .

-- andreas

p5pRT commented 11 years ago

From @rjbs

* "Andreas J. Koenig via RT" \perlbug\-followup@perl\.org [2013-08-02T04:04:53]

git bisect ---------- commit c30fc27b4df65a43710b25dd1d2a57d78ee2fe33 Author: David Mitchell \davem@iabyn\.com Date: Wed Jul 31 22:41:17 2013 +0100
Handle /\[\#\]/ and /\[\(?\#\]/ with code blocks

So\, a patch meant to 1/3 fix Damian's regexp libraries 100% breaks his getopt library. We should sort this out before 5.18.1\, where the above commit has bene applied. ☹

-- rjbs

p5pRT commented 11 years ago

The RT System itself - Status changed from 'new' to 'open'

p5pRT commented 11 years ago

From @iabyn

On Fri\, Aug 02\, 2013 at 08:39:43AM -0400\, Ricardo Signes wrote:

* "Andreas J. Koenig via RT" \perlbug\-followup@perl\.org [2013-08-02T04:04:53]
git bisect ---------- commit c30fc27b4df65a43710b25dd1d2a57d78ee2fe33 Author: David Mitchell \davem@iabyn\.com Date: Wed Jul 31 22:41:17 2013 +0100
Handle /\[\#\]/ and /\[\(?\#\]/ with code blocks
So\, a patch meant to 1/3 fix Damian's regexp libraries 100% breaks his getopt library. We should sort this out before 5.18.1\, where the above commit has bene applied. ☹

It appears to be a bug in Getopt::Euclid\, which was masked by a bug in perl that my fix above inadvertently fixed (!)

Consider the following code:

my $x = 'abc'; my $plain = qr{[$x]-[#$x]}; my $extended = qr{[$x]-[#$x]}x; print "$plain $extended\n";

print "hash in char class is not a comment\n" if "#" =~ /[#]/x;

which prior to the 'Handle /[#]/ and /[(?#]/ with code blocks' fix outputs:

(?^:[abc]-[#abc]) (?^x:[abc]-[#$x]) hash in char class is not a comment

and after\, outputs:

(?^:[abc]-[#abc]) (?^x:[abc]-[#abc]) hash in char class is not a comment

Note that in both cases\, perl agrees that a '#' within a charclass ([])\, within an extended regex (/x)\, is just a '#' char\, and not the start of a comment.

Further\, note that in the non-extended case both before and after\, '$x' within a char class is interpreted as a variable to be interpolated\, not as the literal two chars '$' and 'x.

However in the extended case\, previously a '#' before a variable name within a char class caused the variable not to be interpolated but instead to be seen as literal characters. My fix inadvertently fixed this. Note that this bug goes back to at least 5.6.1.

In the case of Getopt::Euclid\, it has a regex with the char class [@#$^*()+{}?] (it's a list of 'special' chars which will need escaping)\, where the '$' wasn't escaped\, so $^ should be interpreted the variable whose value is likely to be "STDOUT_TOP"; however\, because it was preceded by a '#'\, in old perls\, '$^' was being fortuitously left as a literal '$^' rather than being expanded to 'STDOUT_TOP'. My fix "broke" that.

The fix for Getopt::Euclid is simple; the following diff makes all tests pass under 5.18.1-RC1:

Inline Patch

```diff --- lib/Getopt/Euclid.pm- 2013-08-02 23:17:31.624828488 +0100 +++ lib/Getopt/Euclid.pm 2013-08-02 23:17:52.508088261 +0100 @@ -1055,7 +1055,7 @@ sub _escape_specials { # Escape quotemeta special characters my $arg = shift; - $arg =~ s{([@#$^*()+{}?])}{\\$1}gxms; #? + $arg =~ s{([@#\$^*()+{}?])}{\\$1}gxms; #? return $arg; } ```

The real question is whether my fix should be included in 5.18.1. It still appears to be a correct fix\, only its even more correct than I realised\, and fixes a long-standing bug as well as well as the 5.18.0 regression that it targeted. Whether this is a good thing for a maint release is\, I guess\, up for debate. My gut feeling is that it should be kept.

-- Dave's first rule of Opera: If something needs saying\, say it: don't warble it.

p5pRT commented 11 years ago

From @cpansprout

On Fri Aug 02 15:42:07 2013\, davem wrote:

The real question is whether my fix should be included in 5.18.1. It still appears to be a correct fix\, only its even more correct than I realised\, and fixes a long-standing bug as well as well as the 5.18.0 regression that it targeted. Whether this is a good thing for a maint release is\, I guess\, up for debate. My gut feeling is that it should be kept.

If we remove your fix from the maint branch\, we retain a serious regression from 5.16.

If we leave it in\, we introduce a tiny ‘regression’ that doesn’t count as such\, since it is a bug fix (#45667 or part of it--I haven’t investigated it closely).

Other maintenance releases have had bug fixes (surprise!) and every bug fix is a potentially incompatible change.

So I say leave it in. But what does my opinion count?

--

Father Chrysostomos

p5pRT commented 11 years ago

From @rjbs

* Dave Mitchell \davem@iabyn\.com [2013-08-02T18:41:27]

The real question is whether my fix should be included in 5.18.1. It still appears to be a correct fix\, only its even more correct than I realised\, and fixes a long-standing bug as well as well as the 5.18.0 regression that it targeted. Whether this is a good thing for a maint release is\, I guess\, up for debate. My gut feeling is that it should be kept.

This is my feeling\, too\, but I'll read this again in the morning when my brain is fresh.

Thanks.

-- rjbs

p5pRT commented 11 years ago

From @andk

(Andreas J. Koenig) (via RT) \perlbug\-followup@perl\.org writes:

git bisect ---------- commit c30fc27b4df65a43710b25dd1d2a57d78ee2fe33 Author: David Mitchell \davem@iabyn\.com Date: Wed Jul 31 22:41:17 2013 +0100
Handle /\[\#\]/ and /\[\(?\#\]/ with code blocks

Another affected module: ANDREWF/LaTeX-Encode-0.08.tar.gz

-- andreas

p5pRT commented 11 years ago

From @iabyn

On Sat\, Aug 03\, 2013 at 08:15:33AM +0200\, Andreas Koenig wrote:

(Andreas J. Koenig) (via RT) \perlbug\-followup@perl\.org writes:
git bisect ---------- commit c30fc27b4df65a43710b25dd1d2a57d78ee2fe33 Author: David Mitchell \davem@iabyn\.com Date: Wed Jul 31 22:41:17 2013 +0100
Handle /\[\#\]/ and /\[\(?\#\]/ with code blocks
Another affected module: ANDREWF/LaTeX-Encode-0.08.tar.gz

I haven't tried building and testing it\, but a quick grep of the src shows:

Encode/EncodingTable.pm: $encoded_char_re =~ s{ ([#$\[\]\\]) }{\\$1}gmsx;

which looks like another "bare $var inadvertently protected by a preceeding #".

I wonder how many more of these there are. I tried doing a search on grep.cpan.me with variations of \[.*?\$.*?# \, but it always ended up with the error "Something went wrong! Regexp is too greedy".

-- Hofstadter's Law: It always takes longer than you expect\, even when you take into account Hofstadter's Law.

p5pRT commented 11 years ago

From @iabyn

On Fri\, Aug 02\, 2013 at 06:09:41PM -0700\, Father Chrysostomos via RT wrote:

If we remove your fix from the maint branch\, we retain a serious regression from 5.16.

If we leave it in\, we introduce a tiny ‘regression’ that doesn’t count as such\, since it is a bug fix (#45667 or part of it--I haven’t investigated it closely).

The third option is that I could (probably) modify the fix in maint to fix the regression\, but no longer fix the more longer-standing bug.

The main thing I don't like about the fix for for the long-term bug is that it silently breaks code that used to inadvertently work; i.e. stuff like [#$X] where X is some random special character\, suddenly gets expanded to the value of $X\, where $X is an obscure perl special var.

Perhaps in blead we should have deprecation cycle\, where initially such regexes generate a warning?

-- "Procrastination grows to fill the available time" -- Mitchell's corollary to Parkinson's Law

p5pRT commented 11 years ago

From @mauke

On 03.08.2013 11:13\, Dave Mitchell wrote:

I wonder how many more of these there are. I tried doing a search on grep.cpan.me with variations of \[.*?\$.*?# \, but it always ended up with the error "Something went wrong! Regexp is too greedy".

I found lots of false positives but also:

HTML::StripScripts - https://metacpan.org/source/DRTECH/HTML-StripScripts-1.05/StripScripts.pm#L1474

DBD::PO - https://metacpan.org/source/STEFFENW/DBD-PO-2.10/lib/DBD/PO/Locale/PO.pm#L290

Mail::SpamAssassin::Plugin::FreeMail - https://metacpan.org/source/KMCGRAIL/Mail-SpamAssassin-3.3.2/lib/Mail/SpamAssassin/Plugin/FreeMail.pm#L121

Net::validMX - https://metacpan.org/source/KMCGRAIL/Net-validMX-2.2.0/lib/Net/validMX.pm#L442

Tripletail::DateTime - https://metacpan.org/source/HIO/Tripletail-0.50/lib/Tripletail/DateTime.pm#L145

On the other hand\, SGML::Parser - https://metacpan.org/source/EHOOD/perlSGML.1997Sep18/lib/SGML/Parser.pm#L216 seems to rely on /[#$namestart]/ doing interpolation.

-- Lukas Mai \plokinom@gmail\.com

p5pRT commented 11 years ago

From @rjbs

* Dave Mitchell \davem@iabyn\.com [2013-08-03T05:22:39]

On Fri\, Aug 02\, 2013 at 06:09:41PM -0700\, Father Chrysostomos via RT wrote:

If we remove your fix from the maint branch\, we retain a serious regression from 5.16.

If we leave it in\, we introduce a tiny ‘regression’ that doesn’t count as such\, since it is a bug fix (#45667 or part of it--I haven’t investigated it closely).

The third option is that I could (probably) modify the fix in maint to fix the regression\, but no longer fix the more longer-standing bug.

If you can do that\, that would be nice.

Perhaps in blead we should have deprecation cycle\, where initially such regexes generate a warning?

I think we're better off with just listening to the BBC for breakage\, so far.

-- rjbs

p5pRT commented 11 years ago

From @iabyn

On Sat\, Aug 03\, 2013 at 08:30:37AM -0400\, Ricardo Signes wrote:

* Dave Mitchell \davem@iabyn\.com [2013-08-03T05:22:39]

On Fri\, Aug 02\, 2013 at 06:09:41PM -0700\, Father Chrysostomos via RT wrote:

If we remove your fix from the maint branch\, we retain a serious regression from 5.16.

If we leave it in\, we introduce a tiny ‘regression’ that doesn’t count as such\, since it is a bug fix (#45667 or part of it--I haven’t investigated it closely).

The third option is that I could (probably) modify the fix in maint to fix the regression\, but no longer fix the more longer-standing bug.

If you can do that\, that would be nice.

Now done and pushed as davem/maint-5.18-119125

-- Spock (or Data) is fired from his high-ranking position for not being able to understand the most basic nuances of about one in three sentences that anyone says to him. -- Things That Never Happen in "Star Trek" #19

p5pRT commented 11 years ago

From @nwc10

On Fri\, Aug 02\, 2013 at 11:41:27PM +0100\, Dave Mitchell wrote:

Note that in both cases\, perl agrees that a '#' within a charclass ([])\, within an extended regex (/x)\, is just a '#' char\, and not the start of a comment.

Further\, note that in the non-extended case both before and after\, '$x' within a char class is interpreted as a variable to be interpolated\, not as the literal two chars '$' and 'x.

However in the extended case\, previously a '#' before a variable name within a char class caused the variable not to be interpolated but instead to be seen as literal characters. My fix inadvertently fixed this. Note that this bug goes back to at least 5.6.1.

A bit older than that. But it turns out that it is actually a regression:

bisect.pl --start=perl-5.000 --end=perl-5.002 --target=miniperl -e 'unless ($; =~ /[#$;]/x) {die $]}'

and the culprit is:

commit 748a93069b3d16374a9859d1456065dd3ae11394 Author: Larry Wall \lwall@netlabs\.com Date: Sun Mar 12 22:32:14 1995 -0800

Perl 5.001
[See the Changes file for a list of changes]

I suspect that it's this one:

+NETaa13369: # is now a comment character\, and \# should be left for regcomp. +From: Simon Parsons +Files patched: toke.c + It was not skipping the comment when it skipped the white space\, and construct + an opcode that tried to match a null string. Unfortunately\, the previous + star tried to use the first character of the null string to optimize where + to recurse\, so it never matched.

but we don't have any granularity on the changes. And we don't have access to that bugtracker any more :-(

The real question is whether my fix should be included in 5.18.1. It still appears to be a correct fix\, only its even more correct than I realised\, and fixes a long-standing bug as well as well as the 5.18.0 regression that it targeted. Whether this is a good thing for a maint release is\, I guess\, up for debate. My gut feeling is that it should be kept.

I like the approach that I think you've taken subsequently to this message - revert the fix to this interpolation bug in maint-5.18

I think that fixing this in a maintenance release is going to cause a few too many surprises. (At runtime)

Nicholas Clark

p5pRT commented 11 years ago

From @cpansprout

This is resolved in 5.18.1 by commit 02682386fe3e7.

--

Father Chrysostomos

p5pRT commented 11 years ago

From [Unknown Contact. See original ticket]

This is resolved in 5.18.1 by commit 02682386fe3e7.

--

Father Chrysostomos

p5pRT commented 11 years ago

@cpansprout - Status changed from 'open' to 'resolved'

p5pRT commented 11 years ago

From @andk

"Father Chrysostomos via RT" \perlbug\-comment@perl\.org writes:

This is resolved in 5.18.1 by commit 02682386fe3e7.

If it's only fixed in maint\, then it needs to stay open for blead\, no?

-- andreas

p5pRT commented 11 years ago

From @cpansprout

On Mon Aug 12 20:13:20 2013\, andreas.koenig.7os6VVqR@franz.ak.mind.de wrote:

"Father Chrysostomos via RT" \perlbug\-comment@perl\.org writes:

This is resolved in 5.18.1 by commit 02682386fe3e7.

If it's only fixed in maint\, then it needs to stay open for blead\, no?

Yes\, it does. You’re right.

The bug here is that the modules are buggy and need patches.

--

Father Chrysostomos

p5pRT commented 11 years ago

@cpansprout - Status changed from 'resolved' to 'open'

p5pRT commented 11 years ago

From @andk

The two distributions with failing tests are reported now:

Getopt-Euclid-0.4.3 https://rt.cpan.org/Ticket/Display.html?id=87804 LaTeX-Encode-0.08 https://rt.cpan.org/Ticket/Display.html?id=87805

Among the cases reported by Lukas Mai in https://rt-archive.perl.org/perl5/Ticket/Display.html?id=119125#txn-1240991 there are (if I understand correctly) three not-a-bug (neither in the past nor with bleadperl) because no /x involved:

https://metacpan.org/source/KMCGRAIL/Net-validMX-2.2.0/lib/Net/validMX.pm#L442 https://metacpan.org/source/HIO/Tripletail-0.50/lib/Tripletail/DateTime.pm#L145 https://metacpan.org/source/EHOOD/perlSGML.1997Sep18/lib/SGML/Parser.pm#L216

Three cases remain:

HTML::StripScripts - https://metacpan.org/source/DRTECH/HTML-StripScripts-1.05/StripScripts.pm#L1474

Reported at https://rt.cpan.org/Ticket/Display.html?id=87872

DBD::PO - https://metacpan.org/source/STEFFENW/DBD-PO-2.10/lib/DBD/PO/Locale/PO.pm#L290

Reported as https://rt.cpan.org/Ticket/Display.html?id=87873

Mail::SpamAssassin::Plugin::FreeMail - https://metacpan.org/source/KMCGRAIL/Mail-SpamAssassin-3.3.2/lib/Mail/SpamAssassin/Plugin/FreeMail.pm#L121

Reported as https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6968

-- andreas

p5pRT commented 11 years ago

From @cpansprout

On Thu Aug 15 06:07:27 2013\, andreas.koenig.7os6VVqR@franz.ak.mind.de wrote:

The two distributions with failing tests are reported now:

Getopt-Euclid-0.4.3 https://rt.cpan.org/Ticket/Display.html?id=87804 LaTeX-Encode-0.08 https://rt.cpan.org/Ticket/Display.html?id=87805

Among the cases reported by Lukas Mai in https://rt-archive.perl.org/perl5/Ticket/Display.html?id=119125#txn-1240991 there are (if I understand correctly) three not-a-bug (neither in the past nor with bleadperl) because no /x involved:

https://metacpan.org/source/KMCGRAIL/Net-validMX- 2.2.0/lib/Net/validMX.pm#L442 https://metacpan.org/source/HIO/Tripletail- 0.50/lib/Tripletail/DateTime.pm#L145

https://metacpan.org/source/EHOOD/perlSGML.1997Sep18/lib/SGML/Parser.pm#L216

Three cases remain:

HTML::StripScripts - https://metacpan.org/source/DRTECH/HTML-StripScripts- 1.05/StripScripts.pm#L1474

Reported at https://rt.cpan.org/Ticket/Display.html?id=87872

DBD::PO - https://metacpan.org/source/STEFFENW/DBD-PO- 2.10/lib/DBD/PO/Locale/PO.pm#L290

Reported as https://rt.cpan.org/Ticket/Display.html?id=87873

Mail::SpamAssassin::Plugin::FreeMail - https://metacpan.org/source/KMCGRAIL/Mail-SpamAssassin- 3.3.2/lib/Mail/SpamAssassin/Plugin/FreeMail.pm#L121

Reported as https://issues.apache.org/SpamAssassin/show_bug.cgi?id=6968

Thank you for taking care of that. I have added most of these to the Known Problems section of perl5200delta (the only thing it in right now :-) in commit 508d2c0af8\, so we can close this ticket.

--

Father Chrysostomos

p5pRT commented 11 years ago

@cpansprout - Status changed from 'open' to 'resolved'

p5pRT commented 10 years ago

From @iabyn

On Sat\, Aug 03\, 2013 at 04:08:22PM +0100\, Dave Mitchell wrote:

On Sat\, Aug 03\, 2013 at 08:30:37AM -0400\, Ricardo Signes wrote:

* Dave Mitchell \davem@iabyn\.com [2013-08-03T05:22:39]

On Fri\, Aug 02\, 2013 at 06:09:41PM -0700\, Father Chrysostomos via RT wrote:

If we remove your fix from the maint branch\, we retain a serious regression from 5.16.

If we leave it in\, we introduce a tiny ‘regression’ that doesn’t count as such\, since it is a bug fix (#45667 or part of it--I haven’t investigated it closely).

The third option is that I could (probably) modify the fix in maint to fix the regression\, but no longer fix the more longer-standing bug.

If you can do that\, that would be nice.

Now done and pushed as davem/maint-5.18-119125

Just reviving an old ticket. Ricardo privately pointed out that even with this fix\, 5.18.2 isn't behaving quite right.

I've just pushed out the branch smoke-me/davem/maint-5.18-charclass-hash which contains a candidate fix for 5.18.3. If fixes a 5.16.3 regression\, which I think is uncontroversial\, but also fixes a more longstanding bug\, which is a good thing in principle\, but could do with second opinions as to whether it should be added to maint. The full description is in the commit message\, but search for '*******' to see the two changes that this commit makes.

commit 40e7e8ddcdbe527e4aa0dc8f227459f5fd612139 Author: David Mitchell \davem@iabyn\.com AuthorDate: Tue Feb 18 18:29:14 2014 +0000 Commit: David Mitchell \davem@iabyn\.com CommitDate: Tue Feb 18 18:29:14 2014 +0000

RT #119125: fix two issues with/[#]/x
(This is a maint-specific patch\, not a cherry-pick from blead)
A hash within a character class in an expanded pattern is an odd beast. It is handled twice\, first by the perl toker\, which is looking for things like embedded variables that need interpolating\, and second by the regex parser. The toker only has limited knowledge of regex syntax\, and struggles to work out for things like /#$foo/x and /[#$foo]/x\, whether that's a regex comment and so whether '$foo' is part of the comment string or a variable to be interpolated.
Up until 5.18.0 inclusive it got very confused when the '#' was within a character class\, and usually got it wrong. 5.18.0 also introduced the additional complication that (?{}) code-blocks were now normally handled by the perl toker rather than by the regex parser. A side-effect of this was that if for any reason the toker didn't spot a code block (because it erroneously thought it was part of regex comment for example)\, then the literal code block text would be passed through uncompiled to the regex parser\, which would then refuse to compile unless "use re eval" was in scope.
Al these problems have been fixed in blead. However\, the fixes couldn't be fully back-ported to maint\, since there was a fair bit of code on CPAN that would (erroneously) do things like /[#$^]/ which the author expected to match one three special characters\, and indeed does on on older perls. On bleed however\, this (correctly) expands to /[#STDOUT_TOP]/ (based on what $^ is currently set to). So we decided to keep the old (broken) behaviour on maint.
These fixes and half-fixes were included in 5.18.2. However\, it turns out that 5.18.2 still has a couple of issues\, one of which is a regression from 5.16.x. The table below shows the behaviours of certain regex constructs under various flavours of perl. "5.18.3" represents the changes included in this commit\, and the entries marked "*******" represent changes in behaviour since 5.18.2 (i.e. they are what this commit fixes).
/[#$b]/x
5.16.3 - $b not expanded 5.18.0 - $b not expanded 5.18.2 - $b not expanded - keep bug for backwards compatibility 5.18.3 - $b not expanded - keep bug for backwards compatibility blead - $b expanded
/[#]$c/x
5.16.3 - $c not expanded 5.18.0 - $c not expanded 5.18.2 - $c not expanded 5.18.3 - $c expanded ******* blead - $c expanded
/[#] (?{})/x # i.e. this pattern includes a literal newline
5.16.3 - re eval not needed 5.18.0 - re eval needed 5.18.1 - re eval needed 5.18.2 - re eval needed 5.18.3 - re eval not needed ******* blead - re eval not needed

-- That he said that that that that is is is debatable\, is debatable.

p5pRT commented 10 years ago

From @jkeenan

If we're "reviving an old ticket\," then we have to change its status in RT back to "open".

p5pRT commented 10 years ago

@jkeenan - Status changed from 'resolved' to 'open'

p5pRT commented 10 years ago

From @rjbs

* Dave Mitchell \davem@iabyn\.com [2014-02-18T14:02:01]

Just reviving an old ticket. Ricardo privately pointed out that even with this fix\, 5.18.2 isn't behaving quite right.

I've just pushed out the branch smoke-me/davem/maint-5.18-charclass-hash

Thanks\, with this\, Damian's test program passes.

I will apply this to maint-5.18 before 5.18.3\, and probably quite soon.

-- rjbs

p5pRT commented 10 years ago

From @tonycoz

On Fri Feb 21 19:32:52 2014\, perl.p5p@rjbs.manxome.org wrote:

* Dave Mitchell \davem@iabyn\.com [2014-02-18T14:02:01]

Just reviving an old ticket. Ricardo privately pointed out that even with this fix\, 5.18.2 isn't behaving quite right.

I've just pushed out the branch smoke-me/davem/maint-5.18-charclass-hash

Thanks\, with this\, Damian's test program passes.

I will apply this to maint-5.18 before 5.18.3\, and probably quite soon.

Apparently you applied it as 1f621a8560b56ce47c6d6f706f31b836437b3ea5.

Tony

p5pRT commented 9 years ago

From @jkeenan

On Fri Feb 21 19:32:52 2014\, perl.p5p@rjbs.manxome.org wrote:

* Dave Mitchell \davem@iabyn\.com [2014-02-18T14:02:01]

Just reviving an old ticket. Ricardo privately pointed out that even with this fix\, 5.18.2 isn't behaving quite right.

I've just pushed out the branch smoke-me/davem/maint-5.18-charclass-hash

Thanks\, with this\, Damian's test program passes.

I will apply this to maint-5.18 before 5.18.3\, and probably quite soon.

I'm trying to determine whether this older ticket is closable.

I went to matrix.cpantesters.org and tried to locate reports for the most recent officially released versions of CPAN distributions mentioned over the course of this RT's existence. Here are my impressions:

* Getopt-Euclid 0.4.5 (latest distribution): both blead (5.21.5) and 5.18.3 doing well * Mail-SpamAssassin 3.4.0 satisfactory * DBD-PO: has never passed a majority of OSes going back to 5.10.0 * HTML::StripScripts 1.05: failing on Linux (only OS with a lot of reports) since 5.21.1 * Tripletail 0.50: failed on most OSes since 5.8.9 * LaTeX-Encode 0.091.6 (latest distribution) strong PASS on all OS * Net-validMX 2.2.0 (latest distribution): has failed on all OSes since 5.8.9 * Encode 2.62 (latest distribution): small %age of failures\, but generally strong on all OSes since 5.8.1

So\, with one exception\, these distributions are either (a) well maintained and consistently PASS a very high majority of their smoke test runs; or (b) they're unmaintained junk that P5P shouldn't worry about.

The one exception: This distribution\, though not widely smoked\, had consistent PASSes from 5.17.10 to 5.21.1. It has consistently FAILed in recent months. (See: http://matrix.cpantesters.org/?dist=HTML%3A%3AStripScripts) So this is the only one that I think P5P might be on the hook for.

Thank you very much.

-- James E Keenan (jkeenan@cpan.org)

p5pRT commented 9 years ago

From @jkeenan

On Fri Sep 26 19:11:06 2014\, jkeenan wrote:

So\, with one exception\, these distributions are either (a) well maintained and consistently PASS a very high majority of their smoke test runs; or (b) they're unmaintained junk that P5P shouldn't worry about.

The one exception: This distribution\, though not widely smoked\, had consistent PASSes from 5.17.10 to 5.21.1. It has consistently FAILed in recent months. (See: http://matrix.cpantesters.org/?dist=HTML%3A%3AStripScripts) So this is the only one that I think P5P might be on the hook for.

And on closer inspection\, I don't think we're on the hook for HTML::StripScripts\, either.

##### package HTML::StripScripts; use strict; use warnings FATAL => 'all'; #####

... and the smoke tests are die-ing on:

##### # Failed test 'use HTML::StripScripts;' # at t/10basic.t line 7. # Tried to use 'HTML::StripScripts'. # Error: Unescaped left brace in regex is deprecated\, passed through in regex; marked by \<-- HERE in m/^\s*([+-]?\d{1\,20}(?:\.\d{ \<-- HERE 1\,20)?)\s*((?:\%|\*|ex|px|pc|cm|mm|in|pt|em)?)\s*$/ at /tmp/loop_over_bdir-8660-GLX8ez/HTML-StripScripts-1.05-y8aps_/blib/lib/HTML/StripScripts.pm line 1633. # Compilation failed in require at t/10basic.t line 7. # BEGIN failed--compilation aborted at t/10basic.t line 7. # ... #####

So if the author were to correct the situation giving the warning\, this distribution would probably PASS once again (though the fatalization of warnings is sub-optimal\, IMO).

Thank you very much. Jim Keenan

-- James E Keenan (jkeenan@cpan.org)

p5pRT commented 9 years ago

From @rjbs

I think this ticket has reached the end of its usefulness.

Thanks very much for your previous investigations\, Jim.

-- rjbs

p5pRT commented 9 years ago

@rjbs - Status changed from 'open' to 'resolved'

Perl / perl5