Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
p5pRT opened this issue Aug 2, 2013 · 31 comments
Closed

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

p5pRT opened this issue Aug 2, 2013 · 31 comments

Comments

@p5pRT
Copy link

p5pRT commented Aug 2, 2013

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

Searchable as RT119125$

@p5pRT
Copy link
Author

p5pRT commented Aug 2, 2013

From @andk

git bisect


commit c30fc27
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​: c30fc27
  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
Copy link
Author

p5pRT commented Aug 2, 2013

From @rjbs

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

git bisect
----------
commit c30fc27
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
Copy link
Author

p5pRT commented Aug 2, 2013

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

@p5pRT
Copy link
Author

p5pRT commented Aug 2, 2013

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 c30fc27
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
--- 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
Copy link
Author

p5pRT commented Aug 3, 2013

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
Copy link
Author

p5pRT commented Aug 3, 2013

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
Copy link
Author

p5pRT commented Aug 3, 2013

From @andk

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

git bisect
----------
commit c30fc27
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
Copy link
Author

p5pRT commented Aug 3, 2013

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 c30fc27
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
Copy link
Author

p5pRT commented Aug 3, 2013

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
Copy link
Author

p5pRT commented Aug 3, 2013

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
Copy link
Author

p5pRT commented Aug 3, 2013

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
Copy link
Author

p5pRT commented Aug 3, 2013

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
Copy link
Author

p5pRT commented Aug 5, 2013

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 748a930
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
Copy link
Author

p5pRT commented Aug 13, 2013

From @cpansprout

This is resolved in 5.18.1 by commit 0268238.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Aug 13, 2013

From [Unknown Contact. See original ticket]

This is resolved in 5.18.1 by commit 0268238.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Aug 13, 2013

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

@p5pRT
Copy link
Author

p5pRT commented Aug 13, 2013

From @andk

"Father Chrysostomos via RT" <perlbug-comment@​perl.org> writes​:

This is resolved in 5.18.1 by commit 0268238.

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

--
andreas

@p5pRT
Copy link
Author

p5pRT commented Aug 13, 2013

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 0268238.

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
Copy link
Author

p5pRT commented Aug 13, 2013

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

@p5pRT
Copy link
Author

p5pRT commented Aug 24, 2013

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 508d2c0, so we can close this ticket.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Aug 24, 2013

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

@p5pRT
Copy link
Author

p5pRT commented Feb 18, 2014

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 40e7e8d
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
Copy link
Author

p5pRT commented Feb 19, 2014

From @jkeenan

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

@p5pRT
Copy link
Author

p5pRT commented Feb 19, 2014

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

@p5pRT
Copy link
Author

p5pRT commented Feb 22, 2014

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
Copy link
Author

p5pRT commented Apr 16, 2014

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 1f621a8.

Tony

@p5pRT
Copy link
Author

p5pRT commented Sep 27, 2014

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
Copy link
Author

p5pRT commented Sep 27, 2014

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
Copy link
Author

p5pRT commented Apr 6, 2015

From @rjbs

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

Thanks very much for your previous investigations, Jim.

--
rjbs

@p5pRT
Copy link
Author

p5pRT commented Apr 6, 2015

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant