Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.94k stars 554 forks source link

[BUG] Coredump in 5.9.2 when evalling a certain string while in a coderf in @INC #7764

Closed p5pRT closed 19 years ago

p5pRT commented 19 years ago

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

Searchable as RT33927$

p5pRT commented 19 years ago

From kane@coke.xs4all.nl

Created by kane@coke.xs4all.nl

Although the description might be somewhat vague\, the problem is quite pressing for CPANPLUS (and Module​::Build\, the module that actually triggers this coredump).

This problem does not occur in any of the 5.6.x or 5.8.x series as far as i've been able to tell.

The following code is the problem​:

  $ bleadperl -Mcore -Mwarnings -e1

where core.pm looks like this​:
 
  package core;   use strict;   sub import {   require lib;   lib->import( sub { eval q[*UNIVERSAL​::VERSION = $old_version] })   }
  1;

The string being evalled is found in the M​::B source (at an attempt to identify it's version).

The 'use strict' is required for the coredump to occur. The second module being loaded can be any module. An X.pm like this would do​:   package X;   1;

Any 'use' done after the -Mcore will basically trigger this coredump.

Perl Info ``` Flags: category=core severity=high Site configuration information for perl v5.9.2: Configured by kane at Tue Jan 25 16:16:18 CET 2005. Summary of my perl5 (revision 5 version 9 subversion 2 patch 23879) configuration: Platform: osname=darwin, osvers=7.7.0, archname=darwin-2level uname='darwin coke.xs4all.nl 7.7.0 darwin kernel version 7.7.0: sun nov 7 16:06:51 pst 2004; root:xnuxnu-517.9.5.obj~1release_ppc power macintosh powerpc ' config_args='-des -Dusedevel -Dprefix=/opt/bleadperl' hint=recommended, useposix=true, d_sigaction=define usethreads=undef useithreads=undef usemultiplicity=undef useperlio=define d_sfio=undef uselargefiles=define usesocks=undef use64bitint=undef use64bitall=undef uselongdouble=undef usemymalloc=n, bincompat5005=undef Compiler: cc='cc', ccflags ='-fno-common -DPERL_DARWIN -no-cpp-precomp -fno-strict-aliasing -pipe', optimize='-Os', cppflags='-no-cpp-precomp -fno-common -DPERL_DARWIN -no-cpp-precomp -fno-strict-aliasing -pipe' ccversion='', gccversion='3.3 20030304 (Apple Computer, Inc. build 1495)', gccosandvers='' intsize=4, longsize=4, ptrsize=4, doublesize=8, byteorder=4321 d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=8 ivtype='long', ivsize=4, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8 alignbytes=8, prototype=define Linker and Libraries: ld='env MACOSX_DEPLOYMENT_TARGET=10.3 cc', ldflags ='' libpth=/usr/lib libs=-ldbm -ldl -lm -lc perllibs=-ldl -lm -lc libc=/usr/lib/libc.dylib, so=dylib, useshrplib=false, libperl=libperl.a gnulibc_version='' Dynamic Linking: dlsrc=dl_dyld.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' ' cccdlflags=' ', lddlflags=' -bundle -undefined dynamic_lookup' Locally applied patches: DEVEL22511 @INC for perl v5.9.2: /sw/lib/perl5 /sw/lib/perl5/darwin /Users/kane/sources/p4/other/archive-extract/lib /Users/kane/sources/p4/other/file-fetch/lib /Users/kane/sources/p4/other/archive-tar-new/lib /Users/kane/sources/p4/other/carp-trace/lib /Users/kane/sources/p4/other/log-message/lib /Users/kane/sources/p4/other/module-load/lib /Users/kane/sources/p4/other/params-check/lib /Users/kane/sources/p4/other/qmail-checkpassword/lib /Users/kane/sources/p4/other/module-load-conditional/lib /Users/kane/sources/p4/other/term-ui/lib /Users/kane/sources/p4/other/ipc-cmd/lib /Users/kane/sources/p4/other/config-auto/lib /Users/kane/sources/NSA/misc /Users/kane/sources/NSA /Users/kane/sources/NSA/tools /Users/kane/sources/GOD/MMDB/lib /Users/kane/sources/GOD/GOD/lib /Users/kane/sources/NSA/Apache/Frontier-RPC-0.06/lib/ /Users/kane/sources/p4/other/object-accessor/lib/ /opt/bleadperl/lib/5.9.2/darwin-2level /opt/bleadperl/lib/5.9.2 /opt/bleadperl/lib/site_perl/5.9.2/darwin-2level /opt/bleadperl/lib/site_perl/5.9.2 /opt/bleadperl/lib/site_perl . Environment for perl v5.9.2: DYLD_LIBRARY_PATH (unset) HOME=/Users/kane LANG (unset) LANGUAGE (unset) LC_ALL=C LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/sw/bin:/sw/sbin:/bin:/sbin:/usr/bin:/usr/sbin:/usr/local/bin:/opt/bin:/Users/kane/.bin:/usr/X11R6/bin PERL5LIB=/sw/lib/perl5:/sw/lib/perl5/darwin:/Users/kane/sources/p4/other/archive-extract/lib:/Users/kane/sources/p4/other/file-fetch/lib:/Users/kane/sources/p4/other/archive-tar-new/lib:/Users/kane/sources/p4/other/carp-trace/lib:/Users/kane/sources/p4/other/log-message/lib:/Users/kane/sources/p4/other/module-load/lib:/Users/kane/sources/p4/other/params-check/lib:/Users/kane/sources/p4/other/qmail-checkpassword/lib:/Users/kane/sources/p4/other/module-load-conditional/lib:/Users/kane/sources/p4/other/term-ui/lib:/Users/kane/sources/p4/other/ipc-cmd/lib:/Users/kane/sources/p4/other/config-auto/lib:/Users/kane/sources/NSA/misc:/Users/kane/sources/NSA:/Users/kane/sources/NSA/tools:/Users/kane/sources/GOD/MMDB/lib:/Users/kane/sources/GOD/GOD/lib:/Users/kane/sources/NSA/Apache/Frontier-RPC-0.06/lib/:/Users/kane/sources/p4/other/object-accessor/lib/ PERL_BADLANG (unset) SHELL=/bin/tcsh ```
p5pRT commented 19 years ago

From @JohnPeacock

kane@​coke.xs4all.nl (via RT) wrote​:

package core;
use strict;
sub import \{
    require lib;
    lib\->import\( sub \{ eval q\[\*UNIVERSAL​::VERSION = $old\_version\] \}\)
\}    
1;

Examining the code in Module​::Build​::Base\, I can see why this would coredump\, since you stripped out all of the important bits. Effectively you are nuking the &UNIVERSAL​::VERSION subroutine. To quote the original code​:

  # version.pm will change the ->VERSION method\, so we mitigate the   # potential effects here. Unfortunately local(*UNIVERSAL​::VERSION)   # will crash perl \< 5.8.1.

  my $old_version = \&UNIVERSAL​::VERSION;   eval {require version};   my $result = eval $eval;   *UNIVERSAL​::VERSION = $old_version;

Here\, $old_version is a reference to the original &UNIVERSAL​::VERSION sub and it is being captured before attempting to load the compatibility version.pm module\, then restored afterwords.

However\, in your code\, you are assigning a null scalar to the glob\, thuse wiping out the &UNIVERSAL​::VERSION sub. The next module to be use'd will henceforth call an null sub and blow bits all over the place.

If you tell me what you are actually attempting to do\, I might be able to suggest something different. I disagreed with the way that M​::B was changed as described in the comment above\, so it may be that this issue will have to be reopened with the M​::B folks. If this is related to the laudable attempt to move M​::B into the core\, the above code will have no effect whatsoever\, since the version.pm module is a stub in 5.9+; all of the UNIVERSAL​::VERSION changes are in the core already.

John

-- John Peacock Director of Information Research and Technology Rowman & Littlefield Publishing Group 4720 Boston Way Lanham\, MD 20706 301-459-3366 x.5010 fax 301-429-5747

p5pRT commented 19 years ago

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

p5pRT commented 19 years ago

From kane@xs4all.net

On Jan 25\, 2005\, at 7​:54 PM\, John Peacock wrote​:

kane@​coke.xs4all.nl (via RT) wrote​:

package core;
use strict;
sub import \{
    require lib;
    lib\->import\( sub \{ eval q\[\*UNIVERSAL&#8203;::VERSION = $old\_version\] 

}) } 1;

Examining the code in Module​::Build​::Base\, I can see why this would coredump\, since you stripped out all of the important bits.
Effectively you are nuking the &UNIVERSAL​::VERSION subroutine. I wasn't aware that method always existed\, but good to know.

[...]

If you tell me what you are actually attempting to do\, I might be able to suggest something different.

What happens in the code is this bit (where we are reading the source of a module)​:

  ### XXX stolen from module​::load​::conditional ###   while (local $_ = \<$fh> ) {   ### the following regexp comes from the   ### ExtUtils​::MakeMaker documentation.   if ( /([\$*])(([\w\​:\']*)\bVERSION)\b.*\=/ ) {

  ### this will eval the version in to $VERSION if it   ### was declared as $VERSION in the module.   ### else the result will be in $res.   ### this is a fix on skud's Module​::InstalledVersion   local $VERSION;   my $res = eval $_;

  ### default to '0.0' if there REALLY is no version   ### all to satisfy warnings   $found = $VERSION || $res || '0.0';

  ### found what we came for   last if $found;   }   }

Basically this tries to find the version of the module we're looking at. As you can see from the comments\, this technique is used in various modules\, including CPANPLUS​::inc (which conditionally loads bundled modules\, if their version is higher than what's on the os\, or the module is not on the os at all).

In Module​::Build​::Base we don't find a VERSION declaration anywhere\, until there\, so that's the string that gets evaled into a version. eval is sadly the only resort as many modules use some perl code to generate the version. Most notably is the parsing of RCS tags.

I suppose if Module​::Build​::Base would say\, at the top​:

  $VERSION = $Module​::Build​::VERSION

this would make EU​::MM\, M​::IV\, M​::L​::C and of course CPANPLUS​::inc find a 'real' version declaration\, rather than something evil that will blow away UNIVERSAL​::VERSION.

But that doesn't stop anyone else from implementing a bit of code that will make perl segfault\, so surely there's somethign we can also do from the perl world to make this DTRT.

As noted\, perl 5.6.x and 5.8.x don't suffer from this problem.

I disagreed with the way that M​::B was changed as described in the comment above\, so it may be that this issue will have to be reopened with the M​::B folks. If this is related to the laudable attempt to move M​::B into the core\, the above code will have no effect whatsoever\, since the version.pm module is a stub in 5.9+; all of the UNIVERSAL​::VERSION changes are in the core already. Actually\, it's 5.9+ that's segfaulting\, nothing else so far...

Thanks for your insights\, they certainly pointed out where the problem lies :)

--

Jos Boumans

  "Whenever you find you are on the side of the majority\,   it is time to pause and reflect." - Mark Twain

  CPANPLUS http​://cpanplus.sf.net

p5pRT commented 19 years ago

From @JohnPeacock

Jos I. Boumans wrote​:

In Module​::Build​::Base we don't find a VERSION declaration anywhere\, until there\, so that's the string that gets evaled into a version.

Well\, that's the real problem\, then. The CPANPLUS​::inc regex (and I know you inherited it from EU​::MM) is too loose\, since it is finding something that isn't a $VERSION assignment and M​:B​:Base is missing a $VERSION altogether.

I'd like to know why this regex wouldn't be suffient​:

  if ( /(\$)(([\w\​:\']*)\bVERSION)\b.*\=/ ) {

i.e. take out the glob "*" entirely. Perhaps Mr. Schwern would care to comment?

Since the $VERSION scalar must be a global scalar in the package (i.e. not 'my' but 'our' or 'use vars')\, I cannot see any reason why someone would want to assign to just the scalar portion of the glob variable by that name (since the rest of that glob is useless fluff).

I suppose if Module​::Build​::Base would say\, at the top​:

$VERSION = $Module&#8203;::Build&#8203;::VERSION

this would make EU​::MM\, M​::IV\, M​::L​::C and of course CPANPLUS​::inc find a 'real' version declaration\, rather than something evil that will blow away UNIVERSAL​::VERSION.

Yes\, that would be an appropriate fix to M​::B​::Base. Every module /should/ have a $VERSION assignment\, even if it inherits it from the "master" module in the family. It's just bad luck for you that the module doesn't have a $VERSION assignment _and_ that the way that Ken tried to protect M​::B from version.pm included something which "looked" like a $VERSION assignment.

But that doesn't stop anyone else from implementing a bit of code that will make perl segfault\, so surely there's somethign we can also do from the perl world to make this DTRT.

UNIVERSAL​::VERSION is one of the few Perl-level things exposed by the core (and actually defined using C code). I'll have to look at why Perl \< 5.9.0 _doesn't_ segfault if you stomp on that CV\, since that may tell us how to fix 5.9+ so that it is more resiliant. There are probably a couple of other things in the UNIVERSAL family which won't take kindly to glob bashing for exactly the same reason. It just so happens that everything needs to have access to that function and if it isn't there\, bad things are going to happen.

John

-- John Peacock Director of Information Research and Technology Rowman & Littlefield Publishing Group 4720 Boston Way Lanham\, MD 20706 301-459-3366 x.5010 fax 301-429-5747

p5pRT commented 19 years ago

From @JohnPeacock

David A. Golden wrote​:

I asked a question about the latest thinking on version numbers in modules on perlmonks and got a tremendously wide range of answers -- and certainly no consensus that all modules need a VERSION. In fact\, several people went the opposite route\, leaving just a single VERSION for the master module so that the distribution version is built correctly and leaving all other modules VERSION-less.

http&#8203;://perlmonks\.org/index\.pl?node\_id=417576

I'm sorry that I don't have as much time to attend to perlmonks as I should. I'll try and spend some time reading and then commenting on that thread; for better or worse\, I am the $VERSION expert\, having spent nearly two years in the core detangling the web of inconsistencies. ;(

However let me rephrase my original statement​:

All modules _in the core_ must have a $VERSION number\, even if it is just a reference to some other module's $VERSION (this is not the best but it is better than naught). The reason for this is that once modules are in the core\, they can get individually modified and their parent module might not change (or even be updated timely on CPAN for dual-life modules). Without a $VERSION in all modules in the core\, the odds of changes being missed are high and for dual-lifed modules the odds of desynchronization between core and CPAN is very high. A couple of years ago\, someone went through and added $VERSION's to all core modules; I suppose I should check to make sure that is still the case...

At the same time\, any module which is released to CPAN _should_ have a $VERSION\, even if it is basically a private module of some other module.   The reason flows out of the above paragraph - if the module would ever be considered for the core\, it needs a $VERSION. A package scalar is cheap (in the larger picture of the overhead Perl requires). If you would ever want to spin off a given sub-module into it's own distribution\, it will have to have a $VERSION\, so you might as well give it one now\, rather than waiting until later.

That being said\, if you are writing modules for your own use\, and you don't intend to release it into the public\, there is no reason for $VERSION at all. It is strictly there to facilitate good inter-module dependencies.

CPAN seems not to care if there's a version -- pulling from the suffix on the distribution tarfile\, right (or maybe using a META file)?

Actually from the Makefile.PL; if that refers to a module name\, then that file is parsed to get the $VERSION\, else that number is used as the CPAN version.

The suggestion that M​::B​::Base simply define version seems like an easy solution. It doesn't even need to be $VERSION=$Module​::Build​::VERSION -- it could be anything so long as it's early enough in the file that other tools find it and stop searching.

I agree that this would fix this immediate problem\, but not the deeper issue that M​::B​::Base is trying to ignore version.pm magic\, which isn't possible in bleadperl (since that is all there is there).

John

-- John Peacock Director of Information Research and Technology Rowman & Littlefield Publishing Group 4501 Forbes Boulevard Suite H Lanham\, MD 20706 301-459-3366 x.5010 fax 301-429-5748

p5pRT commented 19 years ago

From david@hyperbolic.net

My apologies for this comment from the peanut gallery thrown into the middle of a serious discussion\, but I found the assumption in John's statement a little surprising​:

John Peacock wrote​:

Yes\, that would be an appropriate fix to M​::B​::Base. Every module /should/ have a $VERSION assignment\, even if it inherits it from the "master" module in the family. It's just bad luck for you that the module doesn't have a $VERSION assignment _and_ that the way that Ken tried to protect M​::B from version.pm included something which "looked" like a $VERSION assignment.

I asked a question about the latest thinking on version numbers in modules on perlmonks and got a tremendously wide range of answers -- and certainly no consensus that all modules need a VERSION. In fact\, several people went the opposite route\, leaving just a single VERSION for the master module so that the distribution version is built correctly and leaving all other modules VERSION-less.

  http​://perlmonks.org/index.pl?node_id=417576

CPAN seems not to care if there's a version -- pulling from the suffix on the distribution tarfile\, right (or maybe using a META file)? So this is all just so that M​::B and EU​::MM can get a distribution version number from a file without actual requiring the file and look at the value of $VERSION directly (since the file may or may not compile in the first place).

Thus it seems odd to say that every module must have a VERSION simply because the tools that manage distributions do text-searches for them and might get confused. That doesn't strike me as very perlish. Rather\, assume that modules may or may not have a VERSION\, and just let the modules like M​::B be careful that their use of the phrase VERSION doesn't cause confusion.

The suggestion that M​::B​::Base simply define version seems like an easy solution. It doesn't even need to be $VERSION=$Module​::Build​::VERSION -- it could be anything so long as it's early enough in the file that other tools find it and stop searching.

Regards\, David Golden

p5pRT commented 19 years ago

From @kenahoo

On Jan 25\, 2005\, at 3​:26 PM\, John Peacock wrote​:

Jos I. Boumans wrote​:

I suppose if Module​::Build​::Base would say\, at the top​: $VERSION = $Module​::Build​::VERSION this would make EU​::MM\, M​::IV\, M​::L​::C and of course CPANPLUS​::inc find a 'real' version declaration\, rather than something evil that will blow away UNIVERSAL​::VERSION.

Yes\, that would be an appropriate fix to M​::B​::Base.

That won't work. All those tools evaluate the version line in isolation\, not by loading the modules and checking the value of $VERSION. In other words\, when Module​::Build​::Base's version is checked\, Module​::Build won't be loaded\, so the version would be undef.

  -Ken

p5pRT commented 19 years ago

From @JohnPeacock

Ken Williams wrote​:

That won't work. All those tools evaluate the version line in isolation\, not by loading the modules and checking the value of $VERSION. In other words\, when Module​::Build​::Base's version is checked\, Module​::Build won't be loaded\, so the version would be undef.

Ahh\, good point! That leaves targeting the regex to exclude the fake $VERSION assignment _or_ adding a real $VERSION assignment to M​::B​::Base upstream of the other usage.

Alternatively\, the code in M​::B​::Base that only /looks/ like a $VERSION assignment can be removed and the M​::B fixed to work correctly under 'use version;' since\, as I keep saying\, that code is the only code that exists in bleadperl. 'use version;' is a pragma and has no effect with bleadperl or better​:

  bootstrap version if $] \< 5.009;

I think this is likely the best route in the long term\, since M​::B is going to have to play nice with version objects if it is going to go into the core for 5.10.0...

John

-- John Peacock Director of Information Research and Technology Rowman & Littlefield Publishing Group 4501 Forbes Boulevard Suite H Lanham\, MD 20706 301-459-3366 x.5010 fax 301-429-5748

p5pRT commented 19 years ago

From @kenahoo

On Jan 26\, 2005\, at 2​:19 PM\, John Peacock wrote​:

Ahh\, good point! That leaves targeting the regex to exclude the fake $VERSION assignment _or_ adding a real $VERSION assignment to M​::B​::Base upstream of the other usage.

I sort of feel like I should slap the value of $Module​::Build​::VERSION into every .pm file that's part of the distribution. And do this for all my other distributions too. And create an option in M​::B so it does this automatically when running the 'dist' action.

I think this is likely the best route in the long term\, since M​::B is going to have to play nice with version objects if it is going to go into the core for 5.10.0...

That's certainly true. But it also has to play nice with non-version.pm environments since it's backward-compatible to 5.005.

  -Ken

p5pRT commented 19 years ago

From @JohnPeacock

Ken Williams wrote​:

I sort of feel like I should slap the value of $Module​::Build​::VERSION into every .pm file that's part of the distribution. And do this for all my other distributions too. And create an option in M​::B so it does this automatically when running the 'dist' action.

You can certainly do that\, and I would argue that you will have to do that before M​::B is accepted into the core. But I just want to make sure that everyone understands (I know you do Ken) that it is only the presence of a line that /looks/ like a $VERSION assignment that caused problems in M​::B​::Base.

That's certainly true. But it also has to play nice with non-version.pm environments since it's backward-compatible to 5.005.

As is version.pm... ;)

I know\, you don't want an external dependency\, and especially one with an XS component. I'd like to work around that with you...

John

-- John Peacock Director of Information Research and Technology Rowman & Littlefield Publishing Group 4501 Forbes Boulevard Suite H Lanham\, MD 20706 301-459-3366 x.5010 fax 301-429-5748

p5pRT commented 19 years ago

From @ysth

On Wed\, Jan 26\, 2005 at 02​:00​:57PM -0600\, Ken Williams wrote​:

On Jan 25\, 2005\, at 3​:26 PM\, John Peacock wrote​:

Jos I. Boumans wrote​:

I suppose if Module​::Build​::Base would say\, at the top​: $VERSION = $Module​::Build​::VERSION this would make EU​::MM\, M​::IV\, M​::L​::C and of course CPANPLUS​::inc find a 'real' version declaration\, rather than something evil that will blow away UNIVERSAL​::VERSION.

Yes\, that would be an appropriate fix to M​::B​::Base.

That won't work. All those tools evaluate the version line in isolation\, not by loading the modules and checking the value of $VERSION. In other words\, when Module​::Build​::Base's version is checked\, Module​::Build won't be loaded\, so the version would be undef.

$VERSION = do { require Module​::Build; $Module​::Build​::VERSION };

p5pRT commented 19 years ago

From kane@xs4all.net

On Jan 26\, 2005\, at 9​:00 PM\, Ken Williams wrote​:

On Jan 25\, 2005\, at 3​:26 PM\, John Peacock wrote​:

Jos I. Boumans wrote​:

I suppose if Module​::Build​::Base would say\, at the top​: $VERSION = $Module​::Build​::VERSION this would make EU​::MM\, M​::IV\, M​::L​::C and of course CPANPLUS​::inc find a 'real' version declaration\, rather than something evil that will blow away UNIVERSAL​::VERSION.

Yes\, that would be an appropriate fix to M​::B​::Base.

That won't work. All those tools evaluate the version line in isolation\, not by loading the modules and checking the value of $VERSION. In other words\, when Module​::Build​::Base's version is checked\, Module​::Build won't be loaded\, so the version would be undef.

Well\, the above one line fix adresses 2 issues\, and they both 'do work'​:   1) Under normal operation\, M​::B​::Base is loaded from M​::B\, so the $VERSION variable would be defined. Anyone using M​::B programatically   would find what is expected.   2) All tools using the EU​::MM 'standard' regex would find a valid version   declaration. One that does not happen to override *UNIVERSAL​::VERSION   when being evaluated in the way they have been for quite some time now.

So even though in scenario 2 we will not always get a real version number (it would only when M​::B was\, for some reason\, already loaded)\, it has the happy benefit of Doing The Right Thing when being inspected by tools that look for $VERSION declarations\, rather than override built-in methods. Also\, it conforms to the generally accepted view that 'everything should have a version number\, even if inherited'.

If nothing else\, such an addition wouldn't /hurt/ anything now\, and would stop segfaults in bleadperl for the moment (of course\, they'd need to be fixed proper anyway).

my $0.02;

--   Jos Boumans

  "Time is nature's way of making sure all the sh*t doesn't happen   at once"

  CPANPLUS http​://cpanplus.sf.net

p5pRT commented 19 years ago

From @kenahoo

On Jan 27\, 2005\, at 3​:56 AM\, Jos I. Boumans wrote​:

On Jan 26\, 2005\, at 9​:00 PM\, Ken Williams wrote​:

On Jan 25\, 2005\, at 3​:26 PM\, John Peacock wrote​:

Jos I. Boumans wrote​:

I suppose if Module​::Build​::Base would say\, at the top​: $VERSION = $Module​::Build​::VERSION this would make EU​::MM\, M​::IV\, M​::L​::C and of course CPANPLUS​::inc find a 'real' version declaration\, rather than something evil that will blow away UNIVERSAL​::VERSION.

Yes\, that would be an appropriate fix to M​::B​::Base.

That won't work. All those tools evaluate the version line in isolation\, not by loading the modules and checking the value of $VERSION. In other words\, when Module​::Build​::Base's version is checked\, Module​::Build won't be loaded\, so the version would be undef.

Well\, the above one line fix adresses 2 issues\, and they both 'do work'​: 1) Under normal operation\, M​::B​::Base is loaded from M​::B\, so the $VERSION variable would be defined. Anyone using M​::B programatically would find what is expected.

Right.

2\)    All tools using the EU&#8203;::MM 'standard' regex would find a valid 

version declaration.

Except that it won't have the right value\, it'll be undef. Tools like the META.yml builders\, search.cpan.org\, etc. that report versions of modules would all be fooled.

I think the only sane fix here is to make sure that whatever code\, be it in M​::B or CP+ or perl\, is evaluating these lines\, doesn't cause a segfault when it makes a mistake. Bad version lines are always going to be out there\, even if we legislate against them.

As for M​::B\, of course I can change it\, and I suppose I probably should. Split that code over two lines or something.

  -Ken

p5pRT commented 19 years ago

From james@mastros.biz

Yitzchak Scott-Thoennes wrote​:

On Wed\, Jan 26\, 2005 at 02​:00​:57PM -0600\, Ken Williams wrote​:

On Jan 25\, 2005\, at 3​:26 PM\, John Peacock wrote​:

Jos I. Boumans wrote​:

I suppose if Module​::Build​::Base would say\, at the top​: $VERSION = $Module​::Build​::VERSION this would make EU​::MM\, M​::IV\, M​::L​::C and of course CPANPLUS​::inc find a 'real' version declaration\, rather than something evil that will blow away UNIVERSAL​::VERSION.

Yes\, that would be an appropriate fix to M​::B​::Base.

That won't work. All those tools evaluate the version line in isolation\, not by loading the modules and checking the value of $VERSION. In other words\, when Module​::Build​::Base's version is checked\, Module​::Build won't be loaded\, so the version would be undef.

$VERSION = do { require Module​::Build; $Module​::Build​::VERSION }; IIRC\, I've read some bit of docs that says that PAUSE (or possibly search.cpan.org) not only runs only that line of code\, but runs it inside a Safe compartment\, and possibly uses some other tricks\, the exact nature of which is undocumented. It won't neccessarly be allowed to require anything\, and if it is\, Module​::Build might not actually be there. Of course\, it might not be a problem if PAUSE doesn't correctly see the version of Module​::Build​::Base\, but all in all\, it's probably best that there just be an ordinary scalar assignment there. (Possibly put there by automated means.)

  -=- James Mastros

p5pRT commented 19 years ago

From @rgs

So this patch will be solved in M​::B ?

p5pRT commented 19 years ago

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