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.21.8-197-g5fe499a breaks 5 x CPAN #14474

Closed
p5pRT opened this issue Feb 5, 2015 · 16 comments
Closed

Bleadperl v5.21.8-197-g5fe499a breaks 5 x CPAN #14474

p5pRT opened this issue Feb 5, 2015 · 16 comments

Comments

@p5pRT
Copy link

p5pRT commented Feb 5, 2015

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

Searchable as RT123739$

@p5pRT
Copy link
Author

p5pRT commented Feb 5, 2015

From @andk

bisect


commit 5fe499a
Author​: Tony Cook <tony@​develop-help.com>
Date​: Wed Feb 4 16​:10​:20 2015 +1100

  [perl #123218] "preserve" $/ if set to a bad value

affected


TLINDEN/Config-General-2.56.tar.gz
KRYDE/File-Locate-Iterator-23.tar.gz
PETDANCE/File-Next-1.12.tar.gz
NUFFIN/IO-Handle-Util-0.01.tar.gz
ROODE/Iterator-IO-0.02.tar.gz

sample reports


http​://www.cpantesters.org/cpan/report/b6273d06-ac60-11e4-a95c-f07e8971dd2f
http​://www.cpantesters.org/cpan/report/ccf88464-acb4-11e4-97ac-326d8971dd2f

perl -V


Summary of my perl5 (revision 5 version 21 subversion 9) configuration​:
  Commit id​: 5fe499a
  Platform​:
  osname=linux, osvers=3.16.0-4-amd64, archname=x86_64-linux
  uname='linux k83 3.16.0-4-amd64 #1 smp debian 3.16.7-ckt2-1 (2014-12-08) x86_64 gnulinux '
  config_args='-Dprefix=/home/sand/src/perl/repoperls/installed-perls/perl/v5.21.8-197-g5fe499a/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
  use64bitint=define, use64bitall=define, uselongdouble=undef
  usemymalloc=n, bincompat5005=undef
  Compiler​:
  cc='cc', ccflags ='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2',
  optimize='-O2 -g',
  cppflags='-fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include'
  ccversion='', gccversion='4.9.2', gccosandvers=''
  intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678, doublekind=3
  d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16, longdblkind=3
  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-strong -L/usr/local/lib'
  libpth=/usr/local/lib /usr/lib/gcc/x86_64-linux-gnu/4.9/include-fixed /usr/include/x86_64-linux-gnu /usr/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib
  libs=-lpthread -lnsl -lgdbm -ldb -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
  perllibs=-lpthread -lnsl -ldl -lm -lcrypt -lutil -lc
  libc=libc-2.19.so, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.19'
  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-strong'

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_LOCALE_TIME
  USE_PERLIO USE_PERL_ATOF
  Built under linux
  Compiled at Feb 5 2015 07​:32​:38
  @​INC​:
  /home/sand/src/perl/repoperls/installed-perls/perl/v5.21.8-197-g5fe499a/165a/lib/site_perl/5.21.9/x86_64-linux
  /home/sand/src/perl/repoperls/installed-perls/perl/v5.21.8-197-g5fe499a/165a/lib/site_perl/5.21.9
  /home/sand/src/perl/repoperls/installed-perls/perl/v5.21.8-197-g5fe499a/165a/lib/5.21.9/x86_64-linux
  /home/sand/src/perl/repoperls/installed-perls/perl/v5.21.8-197-g5fe499a/165a/lib/5.21.9
  .

--
andreas

@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2015

From @andk

also affected​:

ROBAU/Slurp-0.4.tar.gz
http​://www.cpantesters.org/cpan/report/34ff8532-ac8b-11e4-8409-b4808971dd2f

HMBRAND/Spreadsheet-Read-0.56.tgz
http​://www.cpantesters.org/cpan/report/33fa3180-acf6-11e4-81c6-2a548971dd2f
(requires something like Text​::CSV installed)

--
andreas

@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2015

From @Tux

On Sun, 08 Feb 2015 08​:11​:49 +0100, Andreas Koenig
<andreas.koenig.7os6VVqR@​franz.ak.mind.de> wrote​:

also affected​:

ROBAU/Slurp-0.4.tar.gz
http​://www.cpantesters.org/cpan/report/34ff8532-ac8b-11e4-8409-b4808971dd2f

HMBRAND/Spreadsheet-Read-0.56.tgz
http​://www.cpantesters.org/cpan/report/33fa3180-acf6-11e4-81c6-2a548971dd2f
(requires something like Text​::CSV installed)

Text​::CSV_XS still PASSes, but with a new worrying message, but only
sometimes​:

t/91_csv_cb.t ..... 1/20 Attempt to free unreferenced scalar​: SV 0x1210148, Perl interpreter​: 0xbe2010 during global destruction.

Which it does not if run with prove (20 runs in a row) or on second or
third run with make test.

And yest, Text​::CSV_XS does change $/

As Spreadsheet​::Read doesn't do anything "special" with $/, it looks
like the context might be of influence. I'll try to make it reproducable

--
H.Merijn Brand http​://tux.nl Perl Monger http​://amsterdam.pm.org/
using perl5.00307 .. 5.21 porting perl5 on HP-UX, AIX, and openSUSE
http​://mirrors.develooper.com/hpux/ http​://www.test-smoke.org/
http​://qa.perl.org http​://www.goldmark.org/jeff/stupid-disclaimers/

@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2015

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

@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2015

From @jkeenan

On Sun Feb 08 03​:31​:42 2015, hmbrand wrote​:

On Sun, 08 Feb 2015 08​:11​:49 +0100, Andreas Koenig
<andreas.koenig.7os6VVqR@​franz.ak.mind.de> wrote​:

also affected​:

ROBAU/Slurp-0.4.tar.gz
http​://www.cpantesters.org/cpan/report/34ff8532-ac8b-11e4-8409-
b4808971dd2f

HMBRAND/Spreadsheet-Read-0.56.tgz
http​://www.cpantesters.org/cpan/report/33fa3180-acf6-11e4-81c6-
2a548971dd2f
(requires something like Text​::CSV installed)

Text​::CSV_XS still PASSes, but with a new worrying message, but only
sometimes​:

t/91_csv_cb.t ..... 1/20 Attempt to free unreferenced scalar​: SV
0x1210148, Perl interpreter​: 0xbe2010 during global destruction.

Which it does not if run with prove (20 runs in a row) or on second or
third run with make test.

And yes, Text​::CSV_XS does change $/

It would be unfortunate if Text​::CSV_XS's behavior were adversely affected. I have one CPAN library based on Text​::CSV and in its use I have never had to pay attention to the difference between the pure-perl and XS versions. And we have code at $job which uses Text​::CSV, again without having to trouble itself as to which implementation is being invoked.

Hence, this problem would be a blocker for us to go to perl-5.22. I encourage those who have the knowledge to deal with this problem to prioritize doing so.

Thank you very much.

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

@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2015

From @Tux

On Sun, 8 Feb 2015 06​:12​:54 -0800, "James E Keenan via RT"
<perlbug-followup@​perl.org> wrote​:

On Sun Feb 08 03​:31​:42 2015, hmbrand wrote​:

On Sun, 08 Feb 2015 08​:11​:49 +0100, Andreas Koenig
<andreas.koenig.7os6VVqR@​franz.ak.mind.de> wrote​:

also affected​:

ROBAU/Slurp-0.4.tar.gz
http​://www.cpantesters.org/cpan/report/34ff8532-ac8b-11e4-8409-
b4808971dd2f

HMBRAND/Spreadsheet-Read-0.56.tgz
http​://www.cpantesters.org/cpan/report/33fa3180-acf6-11e4-81c6-
2a548971dd2f
(requires something like Text​::CSV installed)

Text​::CSV_XS still PASSes, but with a new worrying message, but only
sometimes​:

t/91_csv_cb.t ..... 1/20 Attempt to free unreferenced scalar​: SV
0x1210148, Perl interpreter​: 0xbe2010 during global destruction.

Which it does not if run with prove (20 runs in a row) or on second or
third run with make test.

And yes, Text​::CSV_XS does change $/

It would be unfortunate if Text​::CSV_XS's behavior were adversely affected. I have one CPAN library based on Text​::CSV and in its use I have never had to pay attention to the difference between the pure-perl and XS versions. And we have code at $job which uses Text​::CSV, again without having to trouble itself as to which implementation is being invoked.

Hence, this problem would be a blocker for us to go to perl-5.22. I encourage those who have the knowledge to deal with this problem to prioritize doing so.

The patch resets $/ to PL_rs *ON EVERY GET*
This will break everything that fumbles (legally) with $/ in XS, even
if only temporary, as some *other* module may *read* $/

I opt to revert and find a better solution

--
H.Merijn Brand http​://tux.nl Perl Monger http​://amsterdam.pm.org/
using perl5.00307 .. 5.21 porting perl5 on HP-UX, AIX, and openSUSE
http​://mirrors.develooper.com/hpux/ http​://www.test-smoke.org/
http​://qa.perl.org http​://www.goldmark.org/jeff/stupid-disclaimers/

@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2015

From @cpansprout

On Sun Feb 08 06​:35​:05 2015, hmbrand wrote​:

The patch resets $/ to PL_rs *ON EVERY GET*
This will break everything that fumbles (legally) with $/ in XS, even
if only temporary, as some *other* module may *read* $/

I opt to revert and find a better solution

So do I, *but* your code would be more robust if you did SvSETMAGIC on $/ after fiddling with it. (Am I right in assuming you skipped that?)

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2015

From @Tux

On Sun, 8 Feb 2015 07​:14​:11 -0800, "Father Chrysostomos via RT"
<perlbug-followup@​perl.org> wrote​:

On Sun Feb 08 06​:35​:05 2015, hmbrand wrote​:

The patch resets $/ to PL_rs *ON EVERY GET*
This will break everything that fumbles (legally) with $/ in XS, even
if only temporary, as some *other* module may *read* $/

I opt to revert and find a better solution

So do I, *but* your code would be more robust if you did SvSETMAGIC
on $/ after fiddling with it. (Am I right in assuming you skipped
that?)

  if (csv.eolx || csv.eol_is_cr) {
  /* local $\ = $eol */
  SAVEGENERICSV (PL_rs);
  PL_rs = newSVpvn ((char *)csv.eol, csv.eol_len);
  }

It doesn't need setmagic, but removemagic, right?

--
H.Merijn Brand http​://tux.nl Perl Monger http​://amsterdam.pm.org/
using perl5.00307 .. 5.21 porting perl5 on HP-UX, AIX, and openSUSE
http​://mirrors.develooper.com/hpux/ http​://www.test-smoke.org/
http​://qa.perl.org http​://www.goldmark.org/jeff/stupid-disclaimers/

@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2015

From @Tux

On Sun, 8 Feb 2015 17​:39​:42 +0100, "H.Merijn Brand"
<h.m.brand@​xs4all.nl> wrote​:

So do I, *but* your code would be more robust if you did SvSETMAGIC
on $/ after fiddling with it. (Am I right in assuming you skipped
that?)

if \(csv\.eolx || csv\.eol\_is\_cr\) \{
/\* local $\\ = $eol \*/
SAVEGENERICSV \(PL\_rs\);
PL\_rs = newSVpvn \(\(char \*\)csv\.eol\, csv\.eol\_len\);
\}

It doesn't need setmagic, but removemagic, right?

FWIW, adding MAGIC still passes Text​::CSV_XS' test suite, but still
fail as before on Spreadsheet​::Read

  if (csv.eolx || csv.eol_is_cr) {
  /* local $\ = $eol */
  SAVEGENERICSV (PL_rs);
  PL_rs = newSVpvn ((char *)csv.eol, csv.eol_len);
  SvSETMAGIC (PL_rs);
  }

--
H.Merijn Brand http​://tux.nl Perl Monger http​://amsterdam.pm.org/
using perl5.00307 .. 5.21 porting perl5 on HP-UX, AIX, and openSUSE
http​://mirrors.develooper.com/hpux/ http​://www.test-smoke.org/
http​://qa.perl.org http​://www.goldmark.org/jeff/stupid-disclaimers/

@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2015

From @Tux

On Sun, 8 Feb 2015 06​:12​:54 -0800, "James E Keenan via RT"
<perlbug-followup@​perl.org> wrote​:

On Sun Feb 08 03​:31​:42 2015, hmbrand wrote​:

On Sun, 08 Feb 2015 08​:11​:49 +0100, Andreas Koenig
<andreas.koenig.7os6VVqR@​franz.ak.mind.de> wrote​:

also affected​:

ROBAU/Slurp-0.4.tar.gz
http​://www.cpantesters.org/cpan/report/34ff8532-ac8b-11e4-8409-
b4808971dd2f

HMBRAND/Spreadsheet-Read-0.56.tgz
http​://www.cpantesters.org/cpan/report/33fa3180-acf6-11e4-81c6-
2a548971dd2f
(requires something like Text​::CSV installed)

Text​::CSV_XS still PASSes, but with a new worrying message, but
only sometimes​:

t/91_csv_cb.t ..... 1/20 Attempt to free unreferenced scalar​: SV
0x1210148, Perl interpreter​: 0xbe2010 during global destruction.

Which it does not if run with prove (20 runs in a row) or on second
or third run with make test.

And yes, Text​::CSV_XS does change $/

It would be unfortunate if Text​::CSV_XS's behavior were adversely
affected. I have one CPAN library based on Text​::CSV and in its
use I have never had to pay attention to the difference between
the pure-perl and XS versions. And we have code at $job which
uses Text​::CSV, again without having to trouble itself as to which
implementation is being invoked.

Hence, this problem would be a blocker for us to go to perl-5.22.
I encourage those who have the knowledge to deal with this problem
to prioritize doing so.

Thank you very much.

I have a fix for Spreadsheet​::Read, and I think Text​::CSV_XS is not
"more" in trouble than any module doing «local $/;»

I found a useless line in Spreadsheet​::Read that does «local $/ = $

When I remove that, all tests pass. Can anyone come up with a test case
that stacks localizing $/ to - possible different - values to prove the
current state being wrong?

I'll release Spreadsheet​::Read ASAP

--
H.Merijn Brand http​://tux.nl Perl Monger http​://amsterdam.pm.org/
using perl5.00307 .. 5.21 porting perl5 on HP-UX, AIX, and openSUSE
http​://mirrors.develooper.com/hpux/ http​://www.test-smoke.org/
http​://qa.perl.org http​://www.goldmark.org/jeff/stupid-disclaimers/

@p5pRT
Copy link
Author

p5pRT commented Feb 8, 2015

From @cpansprout

On Sun Feb 08 08​:46​:38 2015, hmbrand wrote​:

On Sun, 8 Feb 2015 17​:39​:42 +0100, "H.Merijn Brand"
<h.m.brand@​xs4all.nl> wrote​:

So do I, *but* your code would be more robust if you did SvSETMAGIC
on $/ after fiddling with it. (Am I right in assuming you skipped
that?)

if \(csv\.eolx || csv\.eol\_is\_cr\) \{
/\* local $\\ = $eol \*/
SAVEGENERICSV \(PL\_rs\);
PL\_rs = newSVpvn \(\(char \*\)csv\.eol\, csv\.eol\_len\);
\}

It doesn't need setmagic, but removemagic, right?

FWIW, adding MAGIC still passes Text​::CSV_XS' test suite, but still
fail as before on Spreadsheet​::Read

if \(csv\.eolx || csv\.eol\_is\_cr\) \{
/\* local $\\ = $eol \*/
SAVEGENERICSV \(PL\_rs\);
PL\_rs = newSVpvn \(\(char \*\)csv\.eol\, csv\.eol\_len\);
SvSETMAGIC \(PL\_rs\);
\}

OK, the weird thing going on here is that $/ and PL_rs are separate SVs. According to perl’s previous behaviour, reading $/ would *not* read PL_rs, but setting $/ *would* set PL_rs.

If you are localising just PL_rs, then you will change the behaviour of <>, but anyone reading $/ will see the old value.

In any case, this change happened too late in the developement series, so I am going to change it back (and fix the bug another way). So you don’t need to fix any of the modules.

But you may want to consider whether the localised PL_rs is giving you the behaviour you want. In particular, I would appreciate feedback *from you* regarding what perl should be doing.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Feb 9, 2015

From @cpansprout

On Sun Feb 08 12​:39​:30 2015, sprout wrote​:

In any case, this change happened too late in the developement series,
so I am going to change it back (and fix the bug another way).

I have done so in ddce084. I hope it fixes all the CPAN failures.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Feb 9, 2015

From @Tux

On Sun, 8 Feb 2015 12​:39​:30 -0800, "Father Chrysostomos via RT"
<perlbug-followup@​perl.org> wrote​:

On Sun Feb 08 08​:46​:38 2015, hmbrand wrote​:

On Sun, 8 Feb 2015 17​:39​:42 +0100, "H.Merijn Brand"
<h.m.brand@​xs4all.nl> wrote​:

So do I, *but* your code would be more robust if you did SvSETMAGIC
on $/ after fiddling with it. (Am I right in assuming you skipped
that?)

if \(csv\.eolx || csv\.eol\_is\_cr\) \{
/\* local $\\ = $eol \*/
SAVEGENERICSV \(PL\_rs\);
PL\_rs = newSVpvn \(\(char \*\)csv\.eol\, csv\.eol\_len\);
\}

It doesn't need setmagic, but removemagic, right?

FWIW, adding MAGIC still passes Text​::CSV_XS' test suite, but still
fail as before on Spreadsheet​::Read

if \(csv\.eolx || csv\.eol\_is\_cr\) \{
/\* local $\\ = $eol \*/
SAVEGENERICSV \(PL\_rs\);
PL\_rs = newSVpvn \(\(char \*\)csv\.eol\, csv\.eol\_len\);
SvSETMAGIC \(PL\_rs\);
\}

OK, the weird thing going on here is that $/ and PL_rs are separate SVs.
According to perl’s previous behaviour, reading $/ would *not* read
PL_rs, but setting $/ *would* set PL_rs.

Ah, right. that is inconsistent and should be avoided

If you are localising just PL_rs, then you will change the behaviour of
<>, but anyone reading $/ will see the old value.

That is bad

In any case, this change happened too late in the developement series,
so I am going to change it back (and fix the bug another way). So you
don’t need to fix any of the modules.

I already fixed the module by removing the useless

  local $/ = $/;

But you may want to consider whether the localised PL_rs is giving you
the behaviour you want. In particular, I would appreciate feedback
*from you* regarding what perl should be doing.

Reading your comments, we need a API call for XS to set $/ and/or
local $/ preferably (of course) with a Devel​::PPPort counterpart

What I think it should at least pass is the eol.t below, but that
passed already (no XS involved there)

--8<--- eol.t
#!perl

use 5.20.1;
use warnings;
use Test​::More;
use DP;

my $deol = $/;
my $line = 1;
my $test = "";
my @​eol = ($deol, "\r", "\r\n", "--", "\n", $deol);

foreach my $eol (@​eol) {
  $test .= "line ".$line++.$eol for 1..3;
  }
say DDisplay $test;

$line = 1;
open my $fh, "<", \$test;
foreach my $eol (@​eol) {
  local $/ = $eol;
  is (scalar <$fh>, "line ".$line++.$eol);
  my $x = $/; # read
  is (scalar <$fh>, "line ".$line++.$eol);
  local $/ = $/; # copy with self
  is (scalar <$fh>, "line ".$line++.$eol);
  }
close $fh;

$line = 1;
open $fh, "<", \$test;

EOL​: {
{ my $eol = shift @​eol or last EOL;
  local $/ = $eol;
  is (scalar <$fh>, "line ".$line++.$eol);
  my $x = $/; # read
  is (scalar <$fh>, "line ".$line++.$eol);
  local $/ = $/; # copy with self
  is (scalar <$fh>, "line ".$line++.$eol);
{ my $eol = shift @​eol or last EOL;
  local $/ = $eol;
  is (scalar <$fh>, "line ".$line++.$eol);
  my $x = $/; # read
  is (scalar <$fh>, "line ".$line++.$eol);
  local $/ = $/; # copy with self
  is (scalar <$fh>, "line ".$line++.$eol);
{ my $eol = shift @​eol or last EOL;
  local $/ = $eol;
  is (scalar <$fh>, "line ".$line++.$eol);
  my $x = $/; # read
  is (scalar <$fh>, "line ".$line++.$eol);
  local $/ = $/; # copy with self
  is (scalar <$fh>, "line ".$line++.$eol);
{ my $eol = shift @​eol or last EOL;
  local $/ = $eol;
  is (scalar <$fh>, "line ".$line++.$eol);
  my $x = $/; # read
  is (scalar <$fh>, "line ".$line++.$eol);
  local $/ = $/; # copy with self
  is (scalar <$fh>, "line ".$line++.$eol);
{ my $eol = shift @​eol or last EOL;
  local $/ = $eol;
  is (scalar <$fh>, "line ".$line++.$eol);
  my $x = $/; # read
  is (scalar <$fh>, "line ".$line++.$eol);
  local $/ = $/; # copy with self
-->8---

--
H.Merijn Brand http​://tux.nl Perl Monger http​://amsterdam.pm.org/
using perl5.00307 .. 5.21 porting perl5 on HP-UX, AIX, and openSUSE
http​://mirrors.develooper.com/hpux/ http​://www.test-smoke.org/
http​://qa.perl.org http​://www.goldmark.org/jeff/stupid-disclaimers/

@p5pRT
Copy link
Author

p5pRT commented Feb 9, 2015

From @Tux

On Sun, 8 Feb 2015 12​:39​:30 -0800, "Father Chrysostomos via RT"
<perlbug-followup@​perl.org> wrote​:

OK, the weird thing going on here is that $/ and PL_rs are separate SVs. According to perl’s previous behaviour, reading $/ would *not* read PL_rs, but setting $/ *would* set PL_rs.

If you are localising just PL_rs, then you will change the behaviour of <>, but anyone reading $/ will see the old value.

In any case, this change happened too late in the developement series, so I am going to change it back (and fix the bug another way). So you don’t need to fix any of the modules.

But you may want to consider whether the localised PL_rs is giving you the behaviour you want. In particular, I would appreciate feedback *from you* regarding what perl should be doing.

Here is an example of a test file that IMHO should PASS​:

--8<--- eol.t
#!perl

use 5.20.1;
use warnings;
use Test​::More;
use Data​::Peek;

my $deol = $/;
my $line = 1;
my $test = "";
my @​eol = ($deol, "\r", "\r\n", "--", "\n", $deol);

foreach my $eol (@​eol) {
  $test .= "line ".$line++.$eol for 1..3;
  }
diag (DDisplay $test);

$line = 1;
open my $fh, "<", \$test;
foreach my $eol (@​eol) {
  local $/ = $eol;
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after local \$/");
  is ($/, $eol, DDisplay ($eol)."\tread \$/");
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after read");
  local $/ = $/; # copy with self
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after local \$/ = \$/");
  }
close $fh;

$test .= "line ".$line++.$_ for reverse @​eol;

$line = 1;
open $fh, "<", \$test;

EOL​:
{ my $eol = shift @​eol or last EOL;
  local $/ = $eol;
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after local \$/");
  is ($/, $eol, DDisplay ($eol)."\tread \$/");
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after read");
  local $/ = $/; # copy with self
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after local \$/ = \$/");
{ my $eol = shift @​eol or last EOL;
  local $/ = $eol;
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after local \$/");
  is ($/, $eol, DDisplay ($eol)."\tread \$/");
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after read");
  local $/ = $/; # copy with self
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after local \$/ = \$/");
{ my $eol = shift @​eol or last EOL;
  local $/ = $eol;
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after local \$/");
  is ($/, $eol, DDisplay ($eol)."\tread \$/");
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after read");
  local $/ = $/; # copy with self
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after local \$/ = \$/");
{ my $eol = shift @​eol or last EOL;
  local $/ = $eol;
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after local \$/");
  is ($/, $eol, DDisplay ($eol)."\tread \$/");
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after read");
  local $/ = $/; # copy with self
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after local \$/ = \$/");
{ my $eol = shift @​eol or last EOL;
  local $/ = $eol;
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after local \$/");
  is ($/, $eol, DDisplay ($eol)."\tread \$/");
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after read");
  local $/ = $/; # copy with self
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after local \$/ = \$/");
{ my $eol = shift @​eol or last EOL;
  local $/ = $eol;
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after local \$/");
  is ($/, $eol, DDisplay ($eol)."\tread \$/");
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after read");
  local $/ = $/; # copy with self
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after local \$/ = \$/");
} is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after scope end");
  is ($/, $eol, DDisplay ($eol)."\t\$/ after scope end");
} is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after scope end");
  is ($/, $eol, DDisplay ($eol)."\t\$/ after scope end");
} is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after scope end");
  is ($/, $eol, DDisplay ($eol)."\t\$/ after scope end");
} is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after scope end");
  is ($/, $eol, DDisplay ($eol)."\t\$/ after scope end");
} is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after scope end");
  is ($/, $eol, DDisplay ($eol)."\t\$/ after scope end");
}

done_testing;
-->8---

It currently fails alike under perl-5.20.1 x86_64-linux-thread-multi-ld
and blead​: v5.21.8-263-ga53bfda built for x86_64-linux-thread-multi-ld

$ perl eol.t
# "line 1\nline 2\nline 3\nline 4\rline 5\rline 6\rline 7\r\nline 8\r\nline 9\r\nline 10--line 11--line 12--line 13\nline 14\nline 15\nline 16\nline 17\nline 18\n"
ok 1 - "\n" <> after local $/
ok 2 - "\n" read $/
ok 3 - "\n" <> after read
ok 4 - "\n" <> after local $/ = $/
ok 5 - "\r" <> after local $/
ok 6 - "\r" read $/
ok 7 - "\r" <> after read
ok 8 - "\r" <> after local $/ = $/
ok 9 - "\r\n" <> after local $/
ok 10 - "\r\n" read $/
ok 11 - "\r\n" <> after read
ok 12 - "\r\n" <> after local $/ = $/
ok 13 - "--" <> after local $/
ok 14 - "--" read $/
ok 15 - "--" <> after read
ok 16 - "--" <> after local $/ = $/
ok 17 - "\n" <> after local $/
ok 18 - "\n" read $/
ok 19 - "\n" <> after read
ok 20 - "\n" <> after local $/ = $/
ok 21 - "\n" <> after local $/
ok 22 - "\n" read $/
ok 23 - "\n" <> after read
ok 24 - "\n" <> after local $/ = $/
ok 25 - "\n" <> after local $/
ok 26 - "\n" read $/
ok 27 - "\n" <> after read
ok 28 - "\n" <> after local $/ = $/
ok 29 - "\r" <> after local $/
ok 30 - "\r" read $/
ok 31 - "\r" <> after read
ok 32 - "\r" <> after local $/ = $/
ok 33 - "\r\n" <> after local $/
ok 34 - "\r\n" read $/
ok 35 - "\r\n" <> after read
ok 36 - "\r\n" <> after local $/ = $/
ok 37 - "--" <> after local $/
ok 38 - "--" read $/
ok 39 - "--" <> after read
ok 40 - "--" <> after local $/ = $/
ok 41 - "\n" <> after local $/
ok 42 - "\n" read $/
ok 43 - "\n" <> after read
ok 44 - "\n" <> after local $/ = $/
ok 45 - "\n" <> after local $/
ok 46 - "\n" read $/
ok 47 - "\n" <> after read
ok 48 - "\n" <> after local $/ = $/
ok 49 - "\n" <> after scope end
ok 50 - "\n" $/ after scope end
not ok 51 - "--" <> after scope end
# Failed test '"--" <> after scope end'
# at eol.t line 80.
# got​: 'line 20
# line 21--'
# expected​: 'line 20--'
ok 52 - "--" $/ after scope end
not ok 53 - "\r\n" <> after scope end
# Failed test '"\r\n" <> after scope end'
# at eol.t line 82.
# got​: 'line 22
# '
# expected​: 'line 21
# '
ok 54 - "\r\n" $/ after scope end
not ok 55 - "\r" <> after scope end
# Failed test '"\r" <> after scope end'
# at eol.t line 84.
' got​: 'line 23
' expected​: 'line 22
ok 56 - "\r" $/ after scope end
not ok 57 - "\n" <> after scope end
# Failed test '"\n" <> after scope end'
# at eol.t line 86.
# got​: 'line 24
# '
# expected​: 'line 23
# '
ok 58 - "\n" $/ after scope end
1..58
# Looks like you failed 4 tests of 58.

$ perl5.21.9 eol.t
# "line 1\nline 2\nline 3\nline 4\rline 5\rline 6\rline 7\r\nline 8\r\nline 9\r\nline 10--line 11--line 12--line 13\nline 14\nline 15\nline 16\nline 17\nline 18\n"
ok 1 - "\n" <> after local $/
ok 2 - "\n" read $/
ok 3 - "\n" <> after read
ok 4 - "\n" <> after local $/ = $/
ok 5 - "\r" <> after local $/
ok 6 - "\r" read $/
ok 7 - "\r" <> after read
ok 8 - "\r" <> after local $/ = $/
ok 9 - "\r\n" <> after local $/
ok 10 - "\r\n" read $/
ok 11 - "\r\n" <> after read
ok 12 - "\r\n" <> after local $/ = $/
ok 13 - "--" <> after local $/
ok 14 - "--" read $/
ok 15 - "--" <> after read
ok 16 - "--" <> after local $/ = $/
ok 17 - "\n" <> after local $/
ok 18 - "\n" read $/
ok 19 - "\n" <> after read
ok 20 - "\n" <> after local $/ = $/
ok 21 - "\n" <> after local $/
ok 22 - "\n" read $/
ok 23 - "\n" <> after read
ok 24 - "\n" <> after local $/ = $/
ok 25 - "\n" <> after local $/
ok 26 - "\n" read $/
ok 27 - "\n" <> after read
ok 28 - "\n" <> after local $/ = $/
ok 29 - "\r" <> after local $/
ok 30 - "\r" read $/
ok 31 - "\r" <> after read
ok 32 - "\r" <> after local $/ = $/
ok 33 - "\r\n" <> after local $/
ok 34 - "\r\n" read $/
ok 35 - "\r\n" <> after read
ok 36 - "\r\n" <> after local $/ = $/
ok 37 - "--" <> after local $/
ok 38 - "--" read $/
ok 39 - "--" <> after read
ok 40 - "--" <> after local $/ = $/
ok 41 - "\n" <> after local $/
ok 42 - "\n" read $/
ok 43 - "\n" <> after read
ok 44 - "\n" <> after local $/ = $/
ok 45 - "\n" <> after local $/
ok 46 - "\n" read $/
ok 47 - "\n" <> after read
ok 48 - "\n" <> after local $/ = $/
ok 49 - "\n" <> after scope end
ok 50 - "\n" $/ after scope end
not ok 51 - "--" <> after scope end
# Failed test '"--" <> after scope end'
# at eol.t line 80.
# got​: 'line 20
# line 21--'
# expected​: 'line 20--'
#
ok 52 - "--" $/ after scope end
not ok 53 - "\r\n" <> after scope end
# Failed test '"\r\n" <> after scope end'
# at eol.t line 82.
# got​: 'line 22
# '
# expected​: 'line 21
# '
#
ok 54 - "\r\n" $/ after scope end
not ok 55 - "\r" <> after scope end
# Failed test '"\r" <> after scope end'
# at eol.t line 84.
' got​: 'line 23
' expected​: 'line 22
#
ok 56 - "\r" $/ after scope end
not ok 57 - "\n" <> after scope end
# Failed test '"\n" <> after scope end'
# at eol.t line 86.
# got​: 'line 24
# '
# expected​: 'line 23
# '
ok 58 - "\n" $/ after scope end
1..58
# Looks like you failed 4 tests of 58.

--
H.Merijn Brand http​://tux.nl Perl Monger http​://amsterdam.pm.org/
using perl5.00307 .. 5.21 porting perl5 on HP-UX, AIX, and openSUSE
http​://mirrors.develooper.com/hpux/ http​://www.test-smoke.org/
http​://qa.perl.org http​://www.goldmark.org/jeff/stupid-disclaimers/

@p5pRT
Copy link
Author

p5pRT commented Feb 17, 2015

From @tonycoz

On Mon Feb 09 05​:20​:09 2015, hmbrand wrote​:

On Sun, 8 Feb 2015 12​:39​:30 -0800, "Father Chrysostomos via RT"
<perlbug-followup@​perl.org> wrote​:

OK, the weird thing going on here is that $/ and PL_rs are separate
SVs. According to perl’s previous behaviour, reading $/ would *not*
read PL_rs, but setting $/ *would* set PL_rs.

If you are localising just PL_rs, then you will change the behaviour
of <>, but anyone reading $/ will see the old value.

In any case, this change happened too late in the developement
series, so I am going to change it back (and fix the bug another
way). So you don’t need to fix any of the modules.

But you may want to consider whether the localised PL_rs is giving
you the behaviour you want. In particular, I would appreciate
feedback *from you* regarding what perl should be doing.

Here is an example of a test file that IMHO should PASS​:

There's a bug in your test code.

The inner part of the string created has *four* lines ending with $deol - the three created by the initial loop, and the fourth by the iteration over the end-of-lines in reverse, but your test code only consumes 3 of them.

If you change​:

  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after local \$/ = \$/");
{ my $eol = shift @​eol or last EOL;
  local $/ = $eol;
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after local \$/");
  is ($/, $eol, DDisplay ($eol)."\tread \$/");
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after read");
  local $/ = $/; # copy with self
  is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after local \$/ = \$/");
+ is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after local \$/ = \$/");
} is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after scope end");
  is ($/, $eol, DDisplay ($eol)."\t\$/ after scope end");

The test passes.

Closing this ticket.

Tony

@p5pRT
Copy link
Author

p5pRT commented Feb 17, 2015

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

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

No branches or pull requests

1 participant