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.17.3-133-g3da9985 breaks DOY/Moose-2.0603.tar.gz #12358

Closed
p5pRT opened this issue Aug 28, 2012 · 12 comments
Closed

Bleadperl v5.17.3-133-g3da9985 breaks DOY/Moose-2.0603.tar.gz #12358

p5pRT opened this issue Aug 28, 2012 · 12 comments

Comments

@p5pRT
Copy link

p5pRT commented Aug 28, 2012

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

Searchable as RT114628$

@p5pRT
Copy link
Author

p5pRT commented Aug 28, 2012

From @andk

git bisect


commit 3da9985
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Sun Aug 26 11​:10​:18 2012 -0700

  Croak for \local %{\%foo}

also affected


SPROUT/CSS-DOM-0.14.tar.gz

Sample fail reports not yet available at cpantesters

perl -V


Summary of my perl5 (revision 5 version 17 subversion 4) configuration​:
  Commit id​: 3da9985
  Platform​:
  osname=linux, osvers=3.2.0-3-amd64, archname=x86_64-linux
  uname='linux k83 3.2.0-3-amd64 #1 smp mon jul 23 02​:45​:17 utc 2012 x86_64 gnulinux '
  config_args='-Dprefix=/home/src/perl/repoperls/installed-perls/perl/v5.17.3-133-g3da9985/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.7.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 -ldb -ldl -lm -lcrypt -lutil -lc
  perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
  libc=, so=so, useshrplib=false, libperl=libperl.a
  gnulibc_version='2.13'
  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_MALLOC_WRAP 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 28 2012 05​:28​:02
  @​INC​:
  /home/src/perl/repoperls/installed-perls/perl/v5.17.3-133-g3da9985/165a/lib/site_perl/5.17.4/x86_64-linux
  /home/src/perl/repoperls/installed-perls/perl/v5.17.3-133-g3da9985/165a/lib/site_perl/5.17.4
  /home/src/perl/repoperls/installed-perls/perl/v5.17.3-133-g3da9985/165a/lib/5.17.4/x86_64-linux
  /home/src/perl/repoperls/installed-perls/perl/v5.17.3-133-g3da9985/165a/lib/5.17.4
  .

--
andreas

@p5pRT
Copy link
Author

p5pRT commented Aug 28, 2012

From @cpansprout

On Mon Aug 27 21​:49​:07 2012, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

git bisect
----------
commit 3da9985
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Sun Aug 26 11​:10​:18 2012 -0700

Croak for \\local %\{\\%foo\}

also affected
-------------
SPROUT/CSS-DOM-0.14.tar.gz

It should be an error, because the local does nothing in that case. It
actually reveals a bug in CSS​::DOM, which does something similar to this​:

$list[0] = [];
{ local @​{$list[-1]} = 3;
  use Data​::Dumper; warn Dumper \@​list }
warn Dumper \@​list
__END__
$VAR1 = [
  [
  3
  ]
  ];
$VAR1 = [
  [
  3
  ]
  ];

Localising the element, not its dereferencement, is the correct thing to do​:

$list[0] = [];
{ local $list[-1];
  @​{$list[-1]} = 3;
  use Data​::Dumper; warn Dumper \@​list }
warn Dumper \@​list
__END__
$VAR1 = [
  [
  3
  ]
  ];
$VAR1 = [
  []
  ];

That means CSS​::DOM’s test suite is not good enough.

I’m looking into Moose now.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Aug 28, 2012

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

@p5pRT
Copy link
Author

p5pRT commented Aug 28, 2012

From @cpansprout

On Mon Aug 27 21​:49​:07 2012, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:

git bisect
----------
commit 3da9985
Author​: Father Chrysostomos <sprout@​cpan.org>
Date​: Sun Aug 26 11​:10​:18 2012 -0700

Croak for \\local %\{\\%foo\}

OK, I have two patches for Moose.

The first one removes the local, making it behave in bleadperl exactly
the way it behaves in earlier versions (the local was ignored).

The second patch keeps the local, changing things to what was probably
intended. I suspect Moose’s test suite needs improvement to catch the
effective lack of local (or was it just to conserve memory?)
Unfortunately, lexical vars can’t be localised.

Choose wisely.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Aug 28, 2012

From @cpansprout

Inline Patch
diff -rup Moose-2.0603-qRHQsv-orig/lib/Moose/Exporter.pm Moose-2.0603-qRHQsv/lib/Moose/Exporter.pm
--- Moose-2.0603-qRHQsv-orig/lib/Moose/Exporter.pm	2012-06-28 09:18:50.000000000 -0700
+++ Moose-2.0603-qRHQsv/lib/Moose/Exporter.pm	2012-08-28 00:23:48.000000000 -0700
@@ -140,13 +140,13 @@ sub _make_exporter {
 }
 
 {
-    my $seen = {};
+    our %_seen;
 
     sub _follow_also {
         my $class             = shift;
         my $exporting_package = shift;
 
-        local %$seen = ( $exporting_package => 1 );
+        local %_seen = ( $exporting_package => 1 );
 
         return uniq( _follow_also_real($exporting_package) );
     }
@@ -171,9 +171,9 @@ sub _make_exporter {
         for my $package (@also) {
             die
                 "Circular reference in 'also' parameter to Moose::Exporter between $exporting_package and $package"
-                if $seen->{$package};
+                if $_seen{$package};
 
-            $seen->{$package} = 1;
+            $_seen{$package} = 1;
         }
 
         return map { $_, _follow_also_real($_) } @also;

@p5pRT
Copy link
Author

p5pRT commented Aug 28, 2012

From @cpansprout

Inline Patch
diff -rup Moose-2.0603-qRHQsv-orig/lib/Moose/Exporter.pm Moose-2.0603-qRHQsv/lib/Moose/Exporter.pm
--- Moose-2.0603-qRHQsv-orig/lib/Moose/Exporter.pm	2012-06-28 09:18:50.000000000 -0700
+++ Moose-2.0603-qRHQsv/lib/Moose/Exporter.pm	2012-08-28 00:17:15.000000000 -0700
@@ -146,7 +146,7 @@ sub _make_exporter {
         my $class             = shift;
         my $exporting_package = shift;
 
-        local %$seen = ( $exporting_package => 1 );
+        %$seen = ( $exporting_package => 1 );
 
         return uniq( _follow_also_real($exporting_package) );
     }

@p5pRT
Copy link
Author

p5pRT commented Aug 28, 2012

From bug-Moose@rt.cpan.org

<URL​: https://rt.cpan.org/Ticket/Display.html?id=79257 >

On Tue Aug 28 03​:32​:02 2012, perlbug-followup@​perl.org wrote​:

On Mon Aug 27 21​:49​:07 2012, andreas.koenig.7os6VVqR@​franz.ak.mind.de wrote​:
The first one removes the local, making it behave in bleadperl exactly
the way it behaves in earlier versions (the local was ignored).

The second patch keeps the local, changing things to what was probably
intended. I suspect Moose’s test suite needs improvement to catch the
effective lack of local (or was it just to conserve memory?)
Unfortunately, lexical vars can’t be localised.

RT switched the order of the patches. It’s the other way round.

@p5pRT
Copy link
Author

p5pRT commented Sep 19, 2012

From bug-Moose@rt.cpan.org

<URL​: https://rt.cpan.org/Ticket/Display.html?id=79257 >

Fixed in 2.0604.

@p5pRT
Copy link
Author

p5pRT commented Sep 20, 2012

From @jkeenan

On Wed Sep 19 14​:23​:25 2012, bug-Moose@​rt.cpan.org wrote​:

<URL​: https://rt.cpan.org/Ticket/Display.html?id=79257 >

Fixed in 2.0604.

Does that make this rt.perl.org ticket closable as well?

Thank you very much.
Jim Keenan

@p5pRT
Copy link
Author

p5pRT commented Sep 20, 2012

From @cpansprout

On Wed Sep 19 17​:50​:42 2012, jkeenan wrote​:

On Wed Sep 19 14​:23​:25 2012, bug-Moose@​rt.cpan.org wrote​:

<URL​: https://rt.cpan.org/Ticket/Display.html?id=79257 >

Fixed in 2.0604.

Does that make this rt.perl.org ticket closable as well?

Yes. I’ll take care of CSS​::DOM.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Sep 20, 2012

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

@p5pRT p5pRT closed this as completed Sep 20, 2012
@p5pRT
Copy link
Author

p5pRT commented Sep 20, 2012

From @andk

"James E Keenan via RT" <perlbug-followup@​perl.org> writes​:

On Wed Sep 19 14​:23​:25 2012, bug-Moose@​rt.cpan.org wrote​:

<URL​: https://rt.cpan.org/Ticket/Display.html?id=79257 >

Fixed in 2.0604.

Does that make this rt.perl.org ticket closable as well?

From my POV, yes.

--
andreas

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