Bug #28579 for base: stop setting $VERSION

Tue Jul 31 08:21:54 2007 rjbs [...] cpan.org - Ticket created

Subject:

stop setting $VERSION

There is no clear documentation as to *why* $VERSION gets set by base.pm. It gets set to a non-number. It gets set in the /base/ class, meaning that using base.pm can alter the class you're subclassing. It can cause ->VERSION and $VERSION to disagree. Stop it, or give me a really good reason, in the documentation. Demonstration of behavior attached. -- rjbs

Subject:

b-ver.pl

package X; sub VERSION { return 1; } package Y; use base X; print "Y->VERSION ", Y->VERSION, "\n";; print "\$X::VERSION $X::VERSION\n"; print "\$Y::VERSION $Y::VERSION\n";

Tue Jul 31 12:00:53 2007 jpeacock [...] cpan.org - Correspondence added

From:

JPEACOCK [...] cpan.org

On Tue Jul 31 08:21:54 2007, RJBS wrote: Show quoted text

> There is no clear documentation as to *why* $VERSION gets set by > base.pm. It gets set to a non-number.

The $VERSION scalar gets set to a number if and only if the base class doesn't defined a $VERSION scalar. Defining a VERSION sub is *not* the same thing, and won't work for other purposes (like using version objects). Where did you get the impression you could provide a VERSION sub instead of a $VERSION scalar? UNIVERSAL::VERSION is merely there as a convenience sub to return the $VERSION scalar (and to handle the trivial comparison logic that import() requires). Additionally, I see no evidence that would lead me to expect setting a $VERSION scalar in the base class has any bearing on the derived class, which needs it's own $VERSION scalar. The whole discussion of the $VERSION in the base class is relative to the loading of the base class itself, and has nothing whatsoever to do with the derived class. NOTABUG John

Tue Jul 31 12:00:55 2007 The RT System itself - Status changed from 'new' to 'open'

Tue Jul 31 12:41:13 2007 rjbs [...] cpan.org - Correspondence added

From:

rjbs [...] cpan.org

On Tue Jul 31 12:00:53 2007, JPEACOCK wrote: Show quoted text

> On Tue Jul 31 08:21:54 2007, RJBS wrote:

> > There is no clear documentation as to *why* $VERSION gets set by > > base.pm. It gets set to a non-number.

> > The $VERSION scalar gets set to a number if and only if the base class > doesn't defined a $VERSION scalar.

Let's assume, for the sake of non-argument, that everything else I said was stupid and wrong. You still did not answer the central question: WHY is $VERSION getting set? Is it some ridiculous method of making a note that the module was, in fact, loaded? Surely some variables like "\0\0loaded\0\0" or something else stupid would even be better than $VERSION. Why? Because subclassing a class should not alter the package implementing that class.

Tue Jul 31 17:23:30 2007 jpeacock [...] cpan.org - Correspondence added

From:

JPEACOCK [...] cpan.org

On Tue Jul 31 12:41:13 2007, RJBS wrote: Show quoted text

> WHY is $VERSION getting set? Is it some ridiculous method of making a > note that the module was, in fact, loaded? Surely some variables like > "\0\0loaded\0\0" or something else stupid would even be better than > $VERSION.

I suspect (but do not know for sure) that it was intended to be used with classes which would fill in when a given "base" class was itself virtual. Since base.pm was introduced in the Late Cretaceous period (5.004_04), it may have also been an early attempt at OO that never quite panned out. In any case, the only time that $VERSION gets set is if the base class author didn't include a $VERSION. That is to say that well-written classes won't ever have a problem at all. John

Wed Aug 01 01:56:22 2007 ANDK [...] cpan.org - Correspondence added

Iremember that I was responsible for adding the $VERSION="-1" and that it fixed some very silly bug in base.pm at that time. It was heavily discussed at P5P and nobody came up with a better solution but I do not remember the details of the bug. I cannot agree on the argument that some silly variable like $\0\0loaded\0\0 could do better. It's exactly one variable we can expect by every module author to be defined and this is $VERSION and we do not interfere if the author chooses to define $VERSION himself. Please read the P5P archives to repeat all the arguments there were about it.

Sat Aug 11 17:02:21 2007 mschwern [...] cpan.org - Correspondence added

If it had a use in the past it doesn't anymore. There's only two places $VERSION is looked at and then only to decide whether we should set it. I think it should be removed. I don't worry about backwards compat issues. I can't imagine code crazy enough to rely on it. If there is... well, they're crazy and can do something else.

Sun Aug 12 17:42:55 2007 mschwern [...] cpan.org - Correspondence added

Here's where this feature entered: http://public.activestate.com/cgi-bin/perlbrowse/b/lib/base.pm@3302 Whether to C<require> a base class package is determined by the absence of a global $VERSION in the base package. If $VERSION is not detected even after loading it, <base> will define $VERSION in the base package, setting it to the string C<-1, defined by base.pm>. Seeing as how that's no longer true, the whole symbol table is checked, this feature is vestigial and can be removed.

Sun Aug 12 17:46:55 2007 mschwern [...] cpan.org - Correspondence added

On Sun Aug 12 17:42:55 2007, MSCHWERN wrote: Show quoted text

> Seeing as how that's no longer true, the whole symbol table is checked, > this feature is vestigial and can be removed.

Sorry, I got confused by this code. unless (%{"$base\::"}) { require Carp; Carp::croak(<<ERROR); Base class package "$base" is empty. (Perhaps you need to 'use' the module which defines that package first.) ERROR } It does still use $VERSION to check if it loads, but the above suggests a fine alternative. Possibly looking for a defined entry in the symbol table, rather than just a symbol existing, to avoid the action-at-a-distance failure of 28799.

Sun Aug 12 18:38:19 2007 rjbs [...] cpan.org - Correspondence added

Subject:	Re: [rt.cpan.org #28579] stop setting $VERSION
Date:	Sun, 12 Aug 2007 18:37:34 -0400
To:	Michael G Schwern via RT <bug-base [...] rt.cpan.org>
From:	Ricardo SIGNES <rjbs [...] cpan.org>

* Michael G Schwern via RT <bug-base@rt.cpan.org> [2007-08-12T17:46:57] Show quoted text

> It does still use $VERSION to check if it loads, but the above suggests > a fine alternative. Possibly looking for a defined entry in the symbol > table, rather than just a symbol existing, to avoid the > action-at-a-distance failure of 28799.

I don't know if you care about this exception, but on perl 5.6, this will create a package: Foo->can('bar'); If $::{'Foo::'} did not exist before, it does now. So, if you've checked Foo in this way (I can imagine something like: if ($config{plugin} and $config{plugin}->can('handle_things')) { ... } and then later "require Bar" which does "use base 'Foo'", Foo will not be loaded. -- rjbs

Sat Mar 31 06:29:48 2012 RGARCIA [...] cpan.org - Correspondence added

Fixed in 2.18 (a long time ago)

Sat Mar 31 06:29:50 2012 RGARCIA [...] cpan.org - Status changed from 'open' to 'resolved'