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

SelfLoader.pm 1.0904 - Whitespace in subroutine definitions #9154

Closed
p5pRT opened this issue Dec 17, 2007 · 12 comments
Closed

SelfLoader.pm 1.0904 - Whitespace in subroutine definitions #9154

p5pRT opened this issue Dec 17, 2007 · 12 comments

Comments

@p5pRT
Copy link

p5pRT commented Dec 17, 2007

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

Searchable as RT48769$

@p5pRT
Copy link
Author

p5pRT commented Dec 17, 2007

From mark.swayne@charter.net

Created by mark.swayne@charter.net

I found an interesting problem with SelfLoader.pm version 1.0904

If there is any leading whitespace before a subroutine declaration,
SelfLoader will die with the messge "Undefined subroutine blah at script
line X".
However, if a subroutine without any whitespace is successfully loaded,
any subroutines with leading whitespace that follow the loaded routine
will load normally.

There is also no mention in the module docs that leading whitespace may
be an issue. A simple warning against leading whitespace may be
sufficient to resolve this problem.

Example code and the results of running it follow​:

---Example code---
use SelfLoader;

eval {
  Foo(); # Fails
};
print "Foo Error 1​: $@​\n";

eval {
  Baz(); # Fails
};
print "Baz Error 1​: $@​\n";

eval {
  Bar(); # loads normally
};
print "Bar Error​: $@​\n";

eval {
  Foo(); # Fails
};
print "Foo Error 2​: $@​\n";

eval {
  Baz(); # loads normally
};
print "Baz Error 2​: $@​\n";

__DATA__

sub Foo {
  return scalar @​_;
}

sub Bar {
  return scalar @​_;
}

sub Baz {
  return scalar @​_;
}

--Example code output--
Foo Error 1​: Undefined subroutine main​::Foo at C​:\Documents and
Settings\Mark\test.pl line 4

Baz Error 1​: Undefined subroutine main​::Baz at C​:\Documents and
Settings\Mark\test.pl line 9

Bar Error​:
Foo Error 2​: Undefined subroutine main​::Foo at C​:\Documents and
Settings\Mark\test.pl line 19

Baz Error 2​:

Perl Info

Flags:
    category=core
    severity=low

Site configuration information for perl v5.8.8:

Configured by SYSTEM at Tue Aug 29 12:39:43 2006.

Summary of my perl5 (revision 5 version 8 subversion 8) configuration:
  Platform:
    osname=MSWin32, osvers=5.0, archname=MSWin32-x86-multi-thread
    uname=''
    config_args='undef'
    hint=recommended, useposix=true, d_sigaction=undef
    usethreads=define use5005threads=undef useithreads=define 
usemultiplicity=define
    useperlio=define d_sfio=undef uselargefiles=define usesocks=undef
    use64bitint=undef use64bitall=undef uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='gcc', ccflags ='-DNDEBUG -DWIN32 -D_CONSOLE -DNO_STRICT 
-DHAVE_DES_FCRYPT -DNO_HASH_SEED -DUSE_SITECUSTOMIZE 
-DPERL_IMPLICIT_CONTEXT -DPERL_IMPLICIT_SYS -DUSE_PERLIO 
-DPERL_MSVCRT_READFIX -DHASATTRIBUTE -fno-strict-aliasing',
    optimize='-O2',
    cppflags='-DWIN32'
    ccversion='12.00.8804', gccversion='3.4.2 (mingw-special)', 
gccosandvers=''
    intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=1234
    d_longlong=undef, longlongsize=8, d_longdbl=define, longdblsize=10
    ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='__int64', 
lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='gcc', ldflags ='-nologo -nodefaultlib -debug -opt:ref,icf  
-libpath:"C:\Perl\lib\CORE"  -machine:x86'
    libpth=\lib
    libs=-lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 -ladvapi32 
-lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr -lwinmm 
-lversion -lodbc32 -lodbccp32 -lmsvcrt
    perllibs=-lkernel32 -luser32 -lgdi32 -lwinspool -lcomdlg32 
-ladvapi32 -lshell32 -lole32 -loleaut32 -lnetapi32 -luuid -lws2_32 -lmpr 
-lwinmm -lversion -lodbc32 -lodbccp32 -lmsvcrt
    libc=msvcrt.lib, so=dll, useshrplib=yes, libperl=perl58.lib
    gnulibc_version=''
  Dynamic Linking:
    dlsrc=dl_win32.xs, dlext=dll, d_dlsymun=undef, ccdlflags=' '
    cccdlflags=' ', lddlflags='-mdll -L"C:\Perl\lib\CORE"'

Locally applied patches:
    ACTIVEPERL_LOCAL_PATCHES_ENTRY
    Iin_load_module moved for compatibility with build 806
    Avoid signal flag SA_RESTART for older versions of HP-UX
    PerlEx support in CGI::Carp
    Less verbose ExtUtils::Install and Pod::Find
    Patch for CAN-2005-0448 from Debian with modifications
    Rearrange @INC so that 'site' is searched before 'perl'
    Partly reverted 24733 to preserve binary compatibility
    28671 Define PERL_NO_DEV_RANDOM on Windows
    28376 Add error checks after execing PL_cshname or PL_sh_path
    28305 Pod::Html should not convert \"foo\" into ``foo''
    27736 Make perl_fini() run with Sun WorkShop compiler
    27619 Bug in Term::ReadKey being triggered by a bug in Term::ReadLine
    27549 Move DynaLoader.o into libperl.so
    27528 win32_pclose() error exit doesn't unlock mutex
    27527 win32_async_check() can loop indefinitely
    27515 ignore directories when searching @INC
    27359 Fix -d:Foo=bar syntax
    27210 Fix quote typo in c2ph
    27203 Allow compiling swigged C++ code
    27200 Make stat() on Windows handle trailing slashes correctly
    27194 Get perl_fini() running on HP-UX again
    27133 Initialise lastparen in the regexp structure
    27034 Avoid \"Prototype mismatch\" warnings with autouse
    26970 Make Passive mode the default for Net::FTP
    26921 Avoid getprotobyname/number calls in IO::Socket::INET
    26897,26903 Make common IPPROTO_* constants always available
    26670 Make '-s' on the shebang line parse -foo=bar switches
    26536 INSTALLSCRIPT versus INSTALLDIRS
    26379 Fix alarm() for Windows 2003
    26087 Storable 0.1 compatibility
    25861 IO::File performace issue
    25084 long groups entry could cause memory exhaustion
    24699 ICMP_UNREACHABLE handling in Net::Ping


@INC for perl v5.8.8:
    C:\Program Files\ActiveState Perl Dev Kit 6.0 Deployment\lib\
    C:/Perl/site/lib
    C:/Perl/lib
    .


Environment for perl v5.8.8:
    HOME (unset)
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=C:\Program Files\ActiveState Perl Dev Kit 6.0 
Deployment\bin\;C:\Program Files\SDCC\bin;C:\Program 
Files\BitScope;C:\Perl\bin\;C:\Perl\site\bin\;C:\WINDOWS\system32;C:\WINDOWS;C:\WINDOWS\System32\Wbem;C:\Program 
Files\Common Files\GTK\2.0\bin;C:\PROGRA~1\ATT\Graphviz\bin;C:\Program 
Files\doxygen\bin;C:\Program Files\GnuWin32\bin;C:\Program 
Files\gs\gs8.53\bin;C:\Program 
Files\TortoiseCVS;C:\PROGRA~1\ATT\Graphviz\bin;C:\Program 
Files\QuickTime\QTSystem\;C:\Program Files\VTK 5.0\bin;C:\Program 
Files\OpenDX\bin_intelnt;C:\Documents and 
Settings\Mark\bin;C:\MinGW\bin;C:\Program Files\ATMEL 
Corporation\AT91-ISP v1.7\SAM-BA v2.3;C:\Program Files\ATMEL 
Corporation\AT91-ISP v1.7\Library\
    PERL5LIB=C:\Program Files\ActiveState Perl Dev Kit 6.0 Deployment\lib\
    PERL_BADLANG (unset)
    SHELL (unset)


@p5pRT
Copy link
Author

p5pRT commented Dec 18, 2007

From l2ot9pa02@sneakemail.com

Hi p5p,

Mark Swayne wrote​:
[...]

If there is any leading whitespace before a subroutine declaration,
SelfLoader will die with the messge "Undefined subroutine blah at script
line X".
However, if a subroutine without any whitespace is successfully loaded,
any subroutines with leading whitespace that follow the loaded routine
will load normally.

There is also no mention in the module docs that leading whitespace may
be an issue. A simple warning against leading whitespace may be
sufficient to resolve this problem.
[...]

This problem is still present in the SelfLoader that is part of blead.
The attached patch (add a \s* to the regex) fixes the issue. The second
part of the patch adds a test case.

Question​: As with other core modules, should SelfLoader be dual-lived?
There is some differences between 5.10/blead and 5.8.8. Some of those
are incompatible with pre-5.9.5 because of new regexp features, but I
don't think that's much of a problem to work around. Let me know if you
want me to do this.

Best regards,
Steffen

P.S.​: There's also an AutoLoader patch of mine waiting for application,
but that certainly shouldn't be applied before 5.10.

@p5pRT
Copy link
Author

p5pRT commented Dec 18, 2007

From l2ot9pa02@sneakemail.com

SelfLoader.pm.diff
--- SelfLoader.pm.orig	2007-02-27 15:10:29.000000000 +0100
+++ SelfLoader.pm	2007-12-18 11:47:14.000000000 +0100
@@ -88,7 +88,7 @@
 
     local($/) = "\n";
     while(defined($line = <$fh>) and $line !~ m/^__END__/) {
-	if ($line =~ m/^sub\s+([\w:]+)\s*((?:\([\\\$\@\%\&\*\;]*\))?(?:$attr_list)?)/) {
+	if ($line =~ m/^\s*sub\s+([\w:]+)\s*((?:\([\\\$\@\%\&\*\;]*\))?(?:$attr_list)?)/) {
             push(@stubs, $self->_add_to_cache($name, $currpack, \@lines, $protoype));
             $protoype = $2;
             @lines = ($line);

@p5pRT
Copy link
Author

p5pRT commented Dec 18, 2007

From l2ot9pa02@sneakemail.com

SelfLoader.t.diff
--- SelfLoader.t.orig	2006-06-13 21:29:18.000000000 +0200
+++ SelfLoader.t	2007-12-18 11:47:14.000000000 +0100
@@ -13,7 +13,7 @@
     @INC = $dir;
     push @INC, '../lib';
 
-    print "1..19\n";
+    print "1..20\n";
 
     # First we must set up some selfloader files
     mkdir $dir, 0755            or die "Can't mkdir $dir: $!";
@@ -40,7 +40,6 @@
 
 package sheep;
 sub bleat { shift; shift || "baa" }
-
 __END__
 sub never { die "D'oh" }
 EOT
@@ -56,6 +55,7 @@
 
 sub new { bless {}, shift }
 sub a;
+sub with_whitespace_in_front;
 
 1;
 __DATA__
@@ -63,6 +63,10 @@
 sub a { 'a Bar'; }
 sub b { 'b Bar' }
 
+ sub with_whitespace_in_front {
+  "with_whitespace_in_front Bar"
+}
+
 __END__ DATA
 sub never { die "D'oh" }
 EOT
@@ -147,16 +151,20 @@
 print "not " unless $bar->c() eq 'c Baz';
 print "ok 12\n";
 
+# check that subs with whitespace in front work
+print "not " unless $bar->with_whitespace_in_front() eq 'with_whitespace_in_front Bar';
+print "ok 13\n";
+
 # This selfloads Bar::a because it is stubbed. It also stubs Bar::b as a side
 # effect
 print "not " unless $bar->a() eq 'a Bar';
-print "ok 13\n";
+print "ok 14\n";
 
 print "not " unless $bar->b() eq 'b Bar';
-print "ok 14\n";
+print "ok 15\n";
 
 print "not " unless $bar->c() eq 'c Baz';
-print "ok 15\n";
+print "ok 16\n";
 
 
 
@@ -166,18 +174,18 @@
     $foo->never;
 };
 if ($@ =~ /^Undefined subroutine/) {
-    print "ok 16\n";
+    print "ok 17\n";
 } else {
-    print "not ok 16 $@\n";
+    print "not ok 17 $@\n";
 }
 
 # Try to read from the data file handle
 my $foodata = <Foo::DATA>;
 close Foo::DATA;
 if (defined $foodata) {
-    print "not ok 17 # $foodata\n";
+    print "not ok 18 # $foodata\n";
 } else {
-    print "ok 17\n";
+    print "ok 18\n";
 }
 
 # Check that __END__ DATA is honoured
@@ -186,18 +194,18 @@
     $bar->never;
 };
 if ($@ =~ /^Undefined subroutine/) {
-    print "ok 18\n";
+    print "ok 19\n";
 } else {
-    print "not ok 18 $@\n";
+    print "not ok 19 $@\n";
 }
 
 # Try to read from the data file handle
 my $bardata = <Bar::DATA>;
 close Bar::DATA;
 if ($bardata ne "sub never { die \"D'oh\" }\n") {
-    print "not ok 19 # $bardata\n";
+    print "not ok 20 # $bardata\n";
 } else {
-    print "ok 19\n";
+    print "ok 20\n";
 }
 
 # cleanup

@p5pRT
Copy link
Author

p5pRT commented Dec 18, 2007

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

@p5pRT
Copy link
Author

p5pRT commented Dec 20, 2007

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

@p5pRT p5pRT closed this as completed Dec 20, 2007
@p5pRT
Copy link
Author

p5pRT commented Dec 20, 2007

From @rgs

On 18/12/2007, Steffen Mueller <l2ot9pa02@​sneakemail.com> wrote​:

This problem is still present in the SelfLoader that is part of blead.
The attached patch (add a \s* to the regex) fixes the issue. The second
part of the patch adds a test case.

Thanks, applied as #32665, and $VERSION++.

Question​: As with other core modules, should SelfLoader be dual-lived?
There is some differences between 5.10/blead and 5.8.8. Some of those
are incompatible with pre-5.9.5 because of new regexp features, but I
don't think that's much of a problem to work around. Let me know if you
want me to do this.

I see no reason why it couldn't be dual-lived.

@p5pRT
Copy link
Author

p5pRT commented Dec 20, 2007

From @rgs

On 20/12/2007, Steffen Mueller <l2ot9pa02@​sneakemail.com> wrote​:

Hi Rafael,

Rafael Garcia-Suarez wrote​:

I see no reason why it couldn't be dual-lived.

there actually are some problems​:

- SelfLoader in 5.10 is version 1.11.
- SelfLoader in 5.11 is now version 1.12.
- SelfLoader in 5.8.8 is 1.09x or so.
- SelfLoader > 1.09x contains 5.10 specific regex features.

Hence, backporting means having a CPAN release which doesn't use 5.10
specific features. It also means having a version number which makes non
5.10 perls upgrade and 5.10 and up no upgrade.

Why so? you could test $] and use different code. (and release 1.13)

Therefore, Any such dual-lived releases need, forever, have versions
between 1.09x and probably 1.10 because I think that's where the 5.10
specific changes came in. It also means that when there are changes to
the core/blead SelfLoader, they cannot be backported for 5.10.x because
if I release a SelfLoader with a version > 1.12 to CPAN, all old perls
will try (and fail) to update.

Or am I missing something? One might just have two tracks, one between
1.09 and 1.10 for perl<5.10 and one with versions following blead which
has a glaring "use 5.009;" in the Makefile.PL. Problem is, old perls
won't update at all if the newest version fails.

but the cpan toolchain doesn't support branches. (yet)

Of course, this is a broader issue than just SelfLoader. It's bound to
crop up in different places as we dual-live more modules.

Like, when we'll have Perl 6 and Perl 5 in production.

@p5pRT
Copy link
Author

p5pRT commented Dec 20, 2007

From @nwc10

On Thu, Dec 20, 2007 at 10​:55​:05AM +0100, Rafael Garcia-Suarez wrote​:

On 18/12/2007, Steffen Mueller <l2ot9pa02@​sneakemail.com> wrote​:

This problem is still present in the SelfLoader that is part of blead.
The attached patch (add a \s* to the regex) fixes the issue. The second
part of the patch adds a test case.

Thanks, applied as #32665, and $VERSION++.

Question​: As with other core modules, should SelfLoader be dual-lived?
There is some differences between 5.10/blead and 5.8.8. Some of those
are incompatible with pre-5.9.5 because of new regexp features, but I
don't think that's much of a problem to work around. Let me know if you
want me to do this.

I see no reason why it couldn't be dual-lived.

I'd find this really useful as then 5.8.9 could have the same version as
5.10 and blead.

Nicholas Clark

@p5pRT
Copy link
Author

p5pRT commented Dec 20, 2007

From l2ot9pa02@sneakemail.com

Hi Rafael,

Rafael Garcia-Suarez wrote​:

I see no reason why it couldn't be dual-lived.

there actually are some problems​:

- SelfLoader in 5.10 is version 1.11.
- SelfLoader in 5.11 is now version 1.12.
- SelfLoader in 5.8.8 is 1.09x or so.
- SelfLoader > 1.09x contains 5.10 specific regex features.

Hence, backporting means having a CPAN release which doesn't use 5.10
specific features. It also means having a version number which makes non
5.10 perls upgrade and 5.10 and up no upgrade.

Therefore, Any such dual-lived releases need, forever, have versions
between 1.09x and probably 1.10 because I think that's where the 5.10
specific changes came in. It also means that when there are changes to
the core/blead SelfLoader, they cannot be backported for 5.10.x because
if I release a SelfLoader with a version > 1.12 to CPAN, all old perls
will try (and fail) to update.

Or am I missing something? One might just have two tracks, one between
1.09 and 1.10 for perl<5.10 and one with versions following blead which
has a glaring "use 5.009;" in the Makefile.PL. Problem is, old perls
won't update at all if the newest version fails.

Of course, this is a broader issue than just SelfLoader. It's bound to
crop up in different places as we dual-live more modules.

Best regards,
Steffen

@p5pRT
Copy link
Author

p5pRT commented Dec 20, 2007

From l2ot9pa02@sneakemail.com

Hi Rafael, hi p5p,

Rafael Garcia-Suarez wrote​:

On 20/12/2007, Steffen Mueller <l2ot9pa02@​sneakemail.com> wrote​:

Rafael Garcia-Suarez wrote​:

I see no reason why it couldn't be dual-lived.
there actually are some problems​:

- SelfLoader in 5.10 is version 1.11.
- SelfLoader in 5.11 is now version 1.12.
- SelfLoader in 5.8.8 is 1.09x or so.
- SelfLoader > 1.09x contains 5.10 specific regex features.

Hence, backporting means having a CPAN release which doesn't use 5.10
specific features. It also means having a version number which makes non
5.10 perls upgrade and 5.10 and up no upgrade.

Why so? you could test $] and use different code. (and release 1.13)

I would think there are 5.10 features which don't even compile on 5.8.
So I would have to test $] and then eval code. Is that the way to go?

[...]

but the cpan toolchain doesn't support branches. (yet)

I'm not holding my breath! :)

Best regards,
Steffen

@p5pRT
Copy link
Author

p5pRT commented Dec 22, 2007

From l2ot9pa02@sneakemail.com

Steffen Mueller wrote​:

Rafael Garcia-Suarez wrote​:

On 20/12/2007, Steffen Mueller <l2ot9pa02@​sneakemail.com> wrote​:

Rafael Garcia-Suarez wrote​:

I see no reason why it couldn't be dual-lived.
there actually are some problems​:

- SelfLoader in 5.10 is version 1.11.
- SelfLoader in 5.11 is now version 1.12.
- SelfLoader in 5.8.8 is 1.09x or so.
- SelfLoader > 1.09x contains 5.10 specific regex features.

Hence, backporting means having a CPAN release which doesn't use 5.10
specific features. It also means having a version number which makes non
5.10 perls upgrade and 5.10 and up no upgrade.

Why so? you could test $] and use different code. (and release 1.13)

I would think there are 5.10 features which don't even compile on 5.8.
So I would have to test $] and then eval code. Is that the way to go?

Alternatively, one could test $] during Makefile.PL and install the
correct implementation.

I guess both approaches come with their own set of problems. The
aforementioned route would mean that there are actually two modules of
the same name and version which contain different code. Slightly scary.

Steffen

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