Bug #88458 for version: Put version.pm-specific code in its own file in the perl core

Fri Sep 06 01:18:12 2013 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Ticket created

Subject:

Put version.pm-specific code in its own file in the perl core

Suggestion: version.h (or .c?) in the core could contain: /* DO NOT EDIT this file here. This contains code for which the CPAN "version" distribution is upstream. */ #ifdef PERL_IN_UNIVERSAL_C ... #endif #ifdef PERL_IN_UTIL_C ... #endif And then util.c and universal.c could both contain: #include "version.h" Of course, there are endless variations to this (which file goes where, what includes what, etc.). But this could be a start. I could go ahead and do that if you could tell me exactly which functions need to be moved.

Mon Sep 09 20:54:13 2013 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Fri Sep 06 01:18:12 2013, SPROUT wrote: Show quoted text

> Suggestion: > > version.h (or .c?) in the core could contain: > > /* DO NOT EDIT this file here. This contains code for which the CPAN > "version" distribution is upstream. */ > > #ifdef PERL_IN_UNIVERSAL_C > ... > #endif > > #ifdef PERL_IN_UTIL_C > ... > #endif > > > And then util.c and universal.c could both contain: > > #include "version.h" > > > Of course, there are endless variations to this (which file goes > where, what includes what, etc.). But this could be a start. > > I could go ahead and do that if you could tell me exactly which > functions need to be moved.

Just to let you, so we don’t duplicate efforts, I’m starting to work on this. BTW, do you use git for the CPAN distribution?

Mon Sep 09 20:54:13 2013 The RT System itself - Status changed from 'new' to 'open'

Tue Sep 10 07:12:42 2013 john.peacock [...] havurah-software.org - Correspondence added

Subject:	Re: [rt.cpan.org #88458] Put version.pm-specific code in its own file in the perl core
Date:	Tue, 10 Sep 2013 07:12:19 -0400
To:	bug-version [...] rt.cpan.org
From:	John Peacock <john.peacock [...] havurah-software.org>

On 09/09/2013 08:54 PM, Father Chrysostomos via RT wrote: Show quoted text

> Just to let you, so we don’t duplicate efforts, I’m starting to work on this. > > BTW, do you use git for the CPAN distribution? >

ssh://hg@bitbucket.org/jpeacock/version/ All of the underlying version functions are in util.c (or vutil/vutil.c in the CPAN distro). All of the class/object methods are in universal.c (vutil/vxs.xs). I think that UNIVERSAL::VERSION is the only piece that "needs" to be in universal.c; the rest could be moved. John

Wed Sep 11 16:35:14 2013 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Tue Sep 10 07:12:42 2013, john.peacock@havurah-software.org wrote: Show quoted text

> On 09/09/2013 08:54 PM, Father Chrysostomos via RT wrote:

> > Just to let you, so we don’t duplicate efforts, I’m starting to work > > on this. > > > > BTW, do you use git for the CPAN distribution? > >

> > ssh://hg@bitbucket.org/jpeacock/version/

It won’t let me in without authentication. So I worked from the CPAN distribution. Show quoted text

> > All of the underlying version functions are in util.c (or > vutil/vutil.c > in the CPAN distro). All of the class/object methods are in > universal.c > (vutil/vxs.xs). I think that UNIVERSAL::VERSION is the only piece > that > "needs" to be in universal.c; the rest could be moved.

The attached file containing a patch series (which you should be able to feed straight to git am) moves the XS routines into a separate vxs.inc file. You will see that I have pushed to perl.git’s sprout/version branch blead’s equivalent of these patches; parts of util.c have been moved to vutil.c and parts of universal.c to vxs.inc.

Subject:

open_AiGkwWYI.txt

Message body is not shown because it is too large.

Wed Sep 11 20:46:54 2013 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Wed Sep 11 16:35:14 2013, SPROUT wrote: Show quoted text

> On Tue Sep 10 07:12:42 2013, john.peacock@havurah-software.org wrote:

> > On 09/09/2013 08:54 PM, Father Chrysostomos via RT wrote:

> > > Just to let you, so we don’t duplicate efforts, I’m starting to > > > work > > > on this. > > > > > > BTW, do you use git for the CPAN distribution? > > >

> > > > ssh://hg@bitbucket.org/jpeacock/version/

> > It won’t let me in without authentication. So I worked from the CPAN > distribution. >

> > > > All of the underlying version functions are in util.c (or > > vutil/vutil.c > > in the CPAN distro). All of the class/object methods are in > > universal.c > > (vutil/vxs.xs). I think that UNIVERSAL::VERSION is the only piece > > that > > "needs" to be in universal.c; the rest could be moved.

> > The attached file containing a patch series (which you should be able > to feed straight to git am) moves the XS routines into a separate > vxs.inc file. > > You will see that I have pushed to perl.git’s sprout/version branch > blead’s equivalent of these patches; parts of util.c have been moved > to vutil.c and parts of universal.c to vxs.inc.

I forgot to mention: I tested it with these perls: 5.8.8 5.10.1 5.12.4 5.14.4 5.18.1 v5.19.3-365-g7b6e807 I don’t have anything earlier installed. The patches include fixes for #88572 and #88495, too. I await your go-ahead before I push the perl.git branch to blead.

Thu Sep 12 16:00:22 2013 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Wed Sep 11 16:35:14 2013, SPROUT wrote: Show quoted text

> On Tue Sep 10 07:12:42 2013, john.peacock@havurah-software.org wrote:

> > On 09/09/2013 08:54 PM, Father Chrysostomos via RT wrote:

> > > Just to let you, so we don’t duplicate efforts, I’m starting to > > > work > > > on this. > > > > > > BTW, do you use git for the CPAN distribution? > > >

> > > > ssh://hg@bitbucket.org/jpeacock/version/

> > It won’t let me in without authentication. So I worked from the CPAN > distribution. >

> > > > All of the underlying version functions are in util.c (or > > vutil/vutil.c > > in the CPAN distro). All of the class/object methods are in > > universal.c > > (vutil/vxs.xs). I think that UNIVERSAL::VERSION is the only piece > > that > > "needs" to be in universal.c; the rest could be moved.

> > The attached file containing a patch series (which you should be able > to feed straight to git am) moves the XS routines into a separate > vxs.inc file. > > You will see that I have pushed to perl.git’s sprout/version branch > blead’s equivalent of these patches; parts of util.c have been moved > to vutil.c and parts of universal.c to vxs.inc.

That diff accidentally included changes to vutil/Makefile.PL, which is generated, in addition to the changes in the main Makefile.PL that generate it. Just omitting that hunk would work, but here is another patch sequence without that hunk, for your convenience.

Subject:

better patch.text

Message body is not shown because it is too large.

Fri Sep 20 07:30:24 2013 jpeacock [...] cpan.org - Correspondence added

On Thu Sep 12 16:00:22 2013, SPROUT wrote: Show quoted text

> That diff accidentally included changes to vutil/Makefile.PL, which is > generated, in addition to the changes in the main Makefile.PL that > generate it. > > Just omitting that hunk would work, but here is another patch sequence > without that hunk, for your convenience.

I'm getting test failures like this: t/01base.t ........ 1/? Undefined subroutine &version::vxs::new called at t/coretests.pm line 28. with the "better" patch. I've been swamped (Jewish high holiday season) and had no time to look at it; maybe this weekend. John

Fri Sep 20 15:55:44 2013 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Fri Sep 20 07:30:24 2013, JPEACOCK wrote: Show quoted text

> On Thu Sep 12 16:00:22 2013, SPROUT wrote:

> > That diff accidentally included changes to vutil/Makefile.PL, which > > is > > generated, in addition to the changes in the main Makefile.PL that > > generate it. > > > > Just omitting that hunk would work, but here is another patch > > sequence > > without that hunk, for your convenience.

> > I'm getting test failures like this: > > t/01base.t ........ 1/? Undefined subroutine &version::vxs::new called > at t/coretests.pm line 28. > > with the "better" patch. I've been swamped (Jewish high holiday > season) and had no time to look at it; maybe this weekend.

That’s very strange. I’m not seeing that at all. What perl version are you using? Maybe I accidentally omitted something from the patch.

Sat Sep 21 07:49:03 2013 jpeacock [...] cpan.org - Correspondence added

On Fri Sep 20 15:55:44 2013, SPROUT wrote: Show quoted text

> That’s very strange. I’m not seeing that at all. What perl version > are you using? Maybe I accidentally omitted something from the patch.

I don't know. I just shared the bitbucket repo with you so you can fork that and then confirm that you have all of the pieces, rather than trying to figure out the right patch. Just send me a pull request and I'll take care of it that way. Sorry about sending you the "private" URL the first time... ;-) John

Sat Sep 21 13:58:02 2013 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Sat Sep 21 07:49:03 2013, JPEACOCK wrote: Show quoted text

> On Fri Sep 20 15:55:44 2013, SPROUT wrote:

> > That’s very strange. I’m not seeing that at all. What perl version > > are you using? Maybe I accidentally omitted something from the > > patch.

> > I don't know. I just shared the bitbucket repo with you so you can > fork that and then confirm that you have all of the pieces, rather > than trying to figure out the right patch. Just send me a pull > request and I'll take care of it that way. Sorry about sending you > the "private" URL the first time... ;-)

I have limited web access, being behind a filtering proxy, and bitbucket is not in the list I can get do. I can put it a request to add that to the list of allowed domains, but it will be a few days at least. In the mean time, I have tried applying my ‘better patch’ to a freshly-unpacked tarball from CPAN, and all tests pass. So I can’t say what the problem is yet.

Sun Sep 22 22:01:53 2013 jpeacock [...] cpan.org - Correspondence added

On Sat Sep 21 13:58:02 2013, SPROUT wrote: Show quoted text

> In the mean time, I have tried applying my ‘better patch’ to a > freshly-unpacked tarball from CPAN, and all tests pass. So I can’t > say what the problem is yet.

I did the same and get exactly the same errors with a wide variety of Perls: 5.15.1, 5.19.0, 5.19.1, perl-blead all fail the same way; 5.10.1 fails with a lot of other errors as well. My guess is that the magic to replace the native version code is broken. I'll take a closer look at it tomorrow morning and see if I can figure out what the problem is... John

Sun Oct 13 21:57:17 2013 jpeacock [...] cpan.org - Correspondence added

On Sun Sep 22 22:01:53 2013, JPEACOCK wrote: Show quoted text

> 5.10.1 fails with a lot of other errors as well. My guess is that the > magic to replace the native version code is broken.

I'm wondering if the Perl's you were using for your tests already had a non-core version.pm installed, because I cannot get it to work, no matter what I do. In particular, it appears from my testing that the details[] array is not getting populated correctly, and hence the functions are not getting registered: $ perl -I blib/lib/ -I blib/arch/ -wle "use version::vxs; print version::vxs->new('1.2.3')" Can't locate object method "new" via package "version::vxs" at -e line 1. If I add the following patch: diff -r 35248c2474c9 vutil/vxs.xs --- a/vutil/vxs.xs Tue Oct 08 07:09:01 2013 -0400 +++ b/vutil/vxs.xs Sun Oct 13 21:51:23 2013 -0400 @@ -43,6 +43,7 @@ /* register the overloading (type 'A') magic */ PL_amagic_generation++; do { + fprintf(stderr,"registering %s\n", xsub->name); newXS(xsub->name, xsub->xsub, file); } while (++xsub < end); } I see the wrong array elements: registering UNIVERSAL::isa registering UNIVERSAL::can registering UNIVERSAL::DOES registering UNIVERSAL::VERSION registering version::() registering version::new registering version::parse registering version::("" registering version::stringify registering version::(0+ registering version::numify registering version::normal registering version::(cmp registering version::(<=> registering version::vcmp registering version::(bool registering version::boolean registering version::(nomethod registering version::noop registering version::is_alpha registering version::qv registering version::declare registering version::is_qv registering utf8::is_utf8 registering utf8::valid registering utf8::encode registering utf8::decode registering utf8::upgrade registering utf8::downgrade In fact, if I make the following change: diff -r 35248c2474c9 vutil/vxs.inc --- a/vutil/vxs.inc Tue Oct 08 07:09:01 2013 -0400 +++ b/vutil/vxs.inc Sun Oct 13 21:51:20 2013 -0400 @@ -4,7 +4,7 @@ #ifdef PERL_CORE # define VXS_CLASS "version" #else -# define VXS_CLASS "version::vxs" +# define VXS_CLASS "wrong::class" #endif #ifdef VXS_XSUB_DETAILS the output doesn't change at all (which is clearly wrong). I've applied the patch series to the "sprout" branch in the bitbucket repo, but I have no idea where to go from here.

Mon Oct 14 09:31:15 2013 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Sun Oct 13 21:57:17 2013, JPEACOCK wrote: Show quoted text

> On Sun Sep 22 22:01:53 2013, JPEACOCK wrote:

> > 5.10.1 fails with a lot of other errors as well. My guess is that > > the > > magic to replace the native version code is broken.

> > I'm wondering if the Perl's you were using for your tests already had > a non-core version.pm installed, because I cannot get it to work, no > matter what I do. > > In particular, it appears from my testing that the details[] array is > not getting populated correctly, and hence the functions are not > getting registered: > > $ perl -I blib/lib/ -I blib/arch/ -wle "use version::vxs; print > version::vxs->new('1.2.3')" > Can't locate object method "new" via package "version::vxs" at -e line > 1. > > If I add the following patch: > > diff -r 35248c2474c9 vutil/vxs.xs > --- a/vutil/vxs.xs Tue Oct 08 07:09:01 2013 -0400 > +++ b/vutil/vxs.xs Sun Oct 13 21:51:23 2013 -0400 > @@ -43,6 +43,7 @@ > /* register the overloading (type 'A') magic */ > PL_amagic_generation++; > do { > + fprintf(stderr,"registering %s\n", xsub->name); > newXS(xsub->name, xsub->xsub, file); > } while (++xsub < end); > } > > I see the wrong array elements: > > registering UNIVERSAL::isa > registering UNIVERSAL::can > registering UNIVERSAL::DOES > registering UNIVERSAL::VERSION > registering version::() > registering version::new > registering version::parse > registering version::("" > registering version::stringify > registering version::(0+ > registering version::numify > registering version::normal > registering version::(cmp > registering version::(<=> > registering version::vcmp > registering version::(bool > registering version::boolean > registering version::(nomethod > registering version::noop > registering version::is_alpha > registering version::qv > registering version::declare > registering version::is_qv > registering utf8::is_utf8 > registering utf8::valid > registering utf8::encode > registering utf8::decode > registering utf8::upgrade > registering utf8::downgrade > > In fact, if I make the following change: > > diff -r 35248c2474c9 vutil/vxs.inc > --- a/vutil/vxs.inc Tue Oct 08 07:09:01 2013 -0400 > +++ b/vutil/vxs.inc Sun Oct 13 21:51:20 2013 -0400 > @@ -4,7 +4,7 @@ > #ifdef PERL_CORE > # define VXS_CLASS "version" > #else > -# define VXS_CLASS "version::vxs" > +# define VXS_CLASS "wrong::class" > #endif > > #ifdef VXS_XSUB_DETAILS > > the output doesn't change at all (which is clearly wrong). > > I've applied the patch series to the "sprout" branch in the bitbucket > repo, but I have no idea where to go from here.

Sorry I’ve not been able to finish this up. Other things in real life have been getting in the way, and will continue for some time. However, I suspect that the ‘const struct xsub_details details[]’ in vxs.xs needs to be made static, and possibly renamed to vdetails to avoid conflicting with the one in the perl core. (And then that one should also be made static; it probably should have been to begin with.) Does that work?

Tue Oct 22 14:20:38 2013 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Mon Oct 14 09:31:15 2013, SPROUT wrote: Show quoted text

> On Sun Oct 13 21:57:17 2013, JPEACOCK wrote:

> > On Sun Sep 22 22:01:53 2013, JPEACOCK wrote:

> > > 5.10.1 fails with a lot of other errors as well. My guess is that > > > the > > > magic to replace the native version code is broken.

> > > > I'm wondering if the Perl's you were using for your tests already had > > a non-core version.pm installed, because I cannot get it to work, no > > matter what I do. > > > > In particular, it appears from my testing that the details[] array is > > not getting populated correctly, and hence the functions are not > > getting registered: > > > > $ perl -I blib/lib/ -I blib/arch/ -wle "use version::vxs; print > > version::vxs->new('1.2.3')" > > Can't locate object method "new" via package "version::vxs" at -e > > line > > 1. > > > > If I add the following patch: > > > > diff -r 35248c2474c9 vutil/vxs.xs > > --- a/vutil/vxs.xs Tue Oct 08 07:09:01 2013 -0400 > > +++ b/vutil/vxs.xs Sun Oct 13 21:51:23 2013 -0400 > > @@ -43,6 +43,7 @@ > > /* register the overloading (type 'A') magic */ > > PL_amagic_generation++; > > do { > > + fprintf(stderr,"registering %s\n", xsub->name); > > newXS(xsub->name, xsub->xsub, file); > > } while (++xsub < end); > > } > > > > I see the wrong array elements: > > > > registering UNIVERSAL::isa > > registering UNIVERSAL::can > > registering UNIVERSAL::DOES > > registering UNIVERSAL::VERSION > > registering version::() > > registering version::new > > registering version::parse > > registering version::("" > > registering version::stringify > > registering version::(0+ > > registering version::numify > > registering version::normal > > registering version::(cmp > > registering version::(<=> > > registering version::vcmp > > registering version::(bool > > registering version::boolean > > registering version::(nomethod > > registering version::noop > > registering version::is_alpha > > registering version::qv > > registering version::declare > > registering version::is_qv > > registering utf8::is_utf8 > > registering utf8::valid > > registering utf8::encode > > registering utf8::decode > > registering utf8::upgrade > > registering utf8::downgrade > > > > In fact, if I make the following change: > > > > diff -r 35248c2474c9 vutil/vxs.inc > > --- a/vutil/vxs.inc Tue Oct 08 07:09:01 2013 -0400 > > +++ b/vutil/vxs.inc Sun Oct 13 21:51:20 2013 -0400 > > @@ -4,7 +4,7 @@ > > #ifdef PERL_CORE > > # define VXS_CLASS "version" > > #else > > -# define VXS_CLASS "version::vxs" > > +# define VXS_CLASS "wrong::class" > > #endif > > > > #ifdef VXS_XSUB_DETAILS > > > > the output doesn't change at all (which is clearly wrong). > > > > I've applied the patch series to the "sprout" branch in the bitbucket > > repo, but I have no idea where to go from here.

> > Sorry I’ve not been able to finish this up. Other things in real life > have been getting in the way, and will continue for some time. > > However, I suspect that the ‘const struct xsub_details details[]’ in > vxs.xs needs to be made static, and possibly renamed to vdetails to > avoid conflicting with the one in the perl core. (And then that one > should also be made static; it probably should have been to begin > with.) Does that work?

Attached is a patch to make it static. Does it work for you? Sorry, no progress on the bitbucket front yet.

Subject:

make details static.txt

From 179a3d9c398fc0532800450a17689066ac387835 Mon Sep 17 00:00:00 2001 From: Father Chrysostomos <sprout@cpan.org> Date: Tue, 22 Oct 2013 11:17:24 -0700 Subject: [PATCH] vxs.xs: Make details static Otherwise it conflicts with the one perl defines in universal.c. diff --git a/vutil/vxs.xs b/vutil/vxs.xs index edd80ef..dc36d99 100644 --- a/vutil/vxs.xs +++ b/vutil/vxs.xs @@ -19,7 +19,7 @@ struct xsub_details { const char *proto; /* ignored */ }; -const struct xsub_details details[] = { +static const struct xsub_details details[] = { #define VXS_XSUB_DETAILS #include "vxs.inc" #undef VXS_XSUB_DETAILS

Tue Oct 22 21:46:34 2013 jpeacock [...] cpan.org - Correspondence added

On Tue Oct 22 14:20:38 2013, SPROUT wrote: Show quoted text

> Attached is a patch to make it static. Does it work for you?

I'm sorry, I thought I had updated the ticket. Either making details[] static or renaming it fixed that problem. The deeper problem I just identified is that the XS functions have to be renamed so they don't conflict with the core code. I'm seeing a regression of the perl #112478 (snprintf buffer overflow) fix, but we are getting closer. John

Thu Oct 24 20:58:43 2013 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Tue Oct 22 21:46:34 2013, JPEACOCK wrote: Show quoted text

> On Tue Oct 22 14:20:38 2013, SPROUT wrote:

> > Attached is a patch to make it static. Does it work for you?

> > I'm sorry, I thought I had updated the ticket. Either making > details[] static or renaming it fixed that problem. The deeper > problem I just identified is that the XS functions have to be renamed > so they don't conflict with the core code. I'm seeing a regression of > the perl #112478 (snprintf buffer overflow) fix, but we are getting > closer.

Here is a patch for that.

Subject:

Use VXS prefix.txt

From 7e237df84efb9949767ae4868098a9214f36bdeb Mon Sep 17 00:00:00 2001 From: Father Chrysostomos <sprout@cpan.org> Date: Thu, 24 Oct 2013 17:56:22 -0700 Subject: [PATCH] Use VXS_ prefix for XSUB bodies in CPAN version MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The names of the functions in core and in the CPAN version will con- flict otherwise. Since perl versions before 5.16.0 did not have XS_INTERNAL (which could solve this problem another way, making the functions static), it’s easier just to use different names. diff --git a/vutil/vxs.inc b/vutil/vxs.inc index 1615f69..2552ce8 100644 --- a/vutil/vxs.inc +++ b/vutil/vxs.inc @@ -3,48 +3,51 @@ #ifdef PERL_CORE # define VXS_CLASS "version" +# define VXSp(name) XS_##name #else # define VXS_CLASS "version::vxs" +# define VXSp(name) VXS_##name #endif +#define VXS(name) XS(VXSp(name)) #ifdef VXS_XSUB_DETAILS # ifdef PERL_CORE {"UNIVERSAL::VERSION", XS_UNIVERSAL_VERSION, NULL}, # else - {VXS_CLASS "::_VERSION", XS_UNIVERSAL_VERSION, NULL}, + {VXS_CLASS "::_VERSION", VXS_UNIVERSAL_VERSION, NULL}, # endif - {VXS_CLASS "::()", XS_version_noop, NULL}, - {VXS_CLASS "::new", XS_version_new, NULL}, - {VXS_CLASS "::parse", XS_version_new, NULL}, - {VXS_CLASS "::(\"\"", XS_version_stringify, NULL}, - {VXS_CLASS "::stringify", XS_version_stringify, NULL}, - {VXS_CLASS "::(0+", XS_version_numify, NULL}, - {VXS_CLASS "::numify", XS_version_numify, NULL}, - {VXS_CLASS "::normal", XS_version_normal, NULL}, - {VXS_CLASS "::(cmp", XS_version_vcmp, NULL}, - {VXS_CLASS "::(<=>", XS_version_vcmp, NULL}, + {VXS_CLASS "::()", VXSp(version_noop), NULL}, + {VXS_CLASS "::new", VXSp(version_new), NULL}, + {VXS_CLASS "::parse", VXSp(version_new), NULL}, + {VXS_CLASS "::(\"\"", VXSp(version_stringify), NULL}, + {VXS_CLASS "::stringify", VXSp(version_stringify), NULL}, + {VXS_CLASS "::(0+", VXSp(version_numify), NULL}, + {VXS_CLASS "::numify", VXSp(version_numify), NULL}, + {VXS_CLASS "::normal", VXSp(version_normal), NULL}, + {VXS_CLASS "::(cmp", VXSp(version_vcmp), NULL}, + {VXS_CLASS "::(<=>", VXSp(version_vcmp), NULL}, # ifdef PERL_CORE {VXS_CLASS "::vcmp", XS_version_vcmp, NULL}, # else - {VXS_CLASS "::VCMP", XS_version_vcmp, NULL}, + {VXS_CLASS "::VCMP", VXS_version_vcmp, NULL}, # endif - {VXS_CLASS "::(bool", XS_version_boolean, NULL}, - {VXS_CLASS "::boolean", XS_version_boolean, NULL}, - {VXS_CLASS "::(+", XS_version_noop, NULL}, - {VXS_CLASS "::(-", XS_version_noop, NULL}, - {VXS_CLASS "::(*", XS_version_noop, NULL}, - {VXS_CLASS "::(/", XS_version_noop, NULL}, - {VXS_CLASS "::(+=", XS_version_noop, NULL}, - {VXS_CLASS "::(-=", XS_version_noop, NULL}, - {VXS_CLASS "::(*=", XS_version_noop, NULL}, - {VXS_CLASS "::(/=", XS_version_noop, NULL}, - {VXS_CLASS "::(abs", XS_version_noop, NULL}, - {VXS_CLASS "::(nomethod", XS_version_noop, NULL}, - {VXS_CLASS "::noop", XS_version_noop, NULL}, - {VXS_CLASS "::is_alpha", XS_version_is_alpha, NULL}, - {VXS_CLASS "::qv", XS_version_qv, NULL}, - {VXS_CLASS "::declare", XS_version_qv, NULL}, - {VXS_CLASS "::is_qv", XS_version_is_qv, NULL}, + {VXS_CLASS "::(bool", VXSp(version_boolean), NULL}, + {VXS_CLASS "::boolean", VXSp(version_boolean), NULL}, + {VXS_CLASS "::(+", VXSp(version_noop), NULL}, + {VXS_CLASS "::(-", VXSp(version_noop), NULL}, + {VXS_CLASS "::(*", VXSp(version_noop), NULL}, + {VXS_CLASS "::(/", VXSp(version_noop), NULL}, + {VXS_CLASS "::(+=", VXSp(version_noop), NULL}, + {VXS_CLASS "::(-=", VXSp(version_noop), NULL}, + {VXS_CLASS "::(*=", VXSp(version_noop), NULL}, + {VXS_CLASS "::(/=", VXSp(version_noop), NULL}, + {VXS_CLASS "::(abs", VXSp(version_noop), NULL}, + {VXS_CLASS "::(nomethod", VXSp(version_noop), NULL}, + {VXS_CLASS "::noop", VXSp(version_noop), NULL}, + {VXS_CLASS "::is_alpha", VXSp(version_is_alpha), NULL}, + {VXS_CLASS "::qv", VXSp(version_qv), NULL}, + {VXS_CLASS "::declare", VXSp(version_qv), NULL}, + {VXS_CLASS "::is_qv", VXSp(version_is_qv), NULL}, #else #ifndef dVAR @@ -64,7 +67,7 @@ typedef char HVNAME; # define HEKf "s" #endif -XS(XS_UNIVERSAL_VERSION) +VXS(UNIVERSAL_VERSION) { dVAR; dXSARGS; @@ -165,7 +168,7 @@ XS(XS_UNIVERSAL_VERSION) XSRETURN(1); } -XS(XS_version_new) +VXS(version_new) { dVAR; dXSARGS; @@ -233,7 +236,7 @@ XS(XS_version_new) Perl_croak(aTHX_ varname " is not of type version"); \ } STMT_END -XS(XS_version_stringify) +VXS(version_stringify) { dVAR; dXSARGS; @@ -251,7 +254,7 @@ XS(XS_version_stringify) } } -XS(XS_version_numify) +VXS(version_numify) { dVAR; dXSARGS; @@ -267,7 +270,7 @@ XS(XS_version_numify) } } -XS(XS_version_normal) +VXS(version_normal) { dVAR; dXSARGS; @@ -285,7 +288,7 @@ XS(XS_version_normal) } } -XS(XS_version_vcmp) +VXS(version_vcmp) { dVAR; dXSARGS; @@ -325,7 +328,7 @@ XS(XS_version_vcmp) } } -XS(XS_version_boolean) +VXS(version_boolean) { dVAR; dXSARGS; @@ -348,7 +351,7 @@ XS(XS_version_boolean) } } -XS(XS_version_noop) +VXS(version_noop) { dVAR; dXSARGS; @@ -361,7 +364,7 @@ XS(XS_version_noop) XSRETURN_EMPTY; } -XS(XS_version_is_alpha) +VXS(version_is_alpha) { dVAR; dXSARGS; @@ -380,7 +383,7 @@ XS(XS_version_is_alpha) } } -XS(XS_version_qv) +VXS(version_qv) { dVAR; dXSARGS; @@ -434,7 +437,7 @@ XS(XS_version_qv) return; } -XS(XS_version_is_qv) +VXS(version_is_qv) { dVAR; dXSARGS;

Fri Oct 25 09:00:11 2013 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

Here is another patch. I just tried rebasing against blead and got test failures due to a thinko.

Subject:

Fix thinko.text

From 458d29e1ef8961d040ea40cec2c06dc6b0f80d9c Mon Sep 17 00:00:00 2001 From: Father Chrysostomos <sprout@cpan.org> Date: Fri, 25 Oct 2013 05:57:47 -0700 Subject: [PATCH] vxs.inc: Fix thinko This was causing test failures after rebasing against blead. diff --git a/vutil/vxs.inc b/vutil/vxs.inc index 2552ce8..e63173b 100644 --- a/vutil/vxs.inc +++ b/vutil/vxs.inc @@ -374,7 +374,7 @@ VXS(version_is_alpha) { SV *lobj; VTYPECHECK(lobj, ST(0), "lobj"); - if ( hv_exists(MUTABLE_HV(SvRV(lobj)), "alpha", 5 ) ) + if ( hv_exists(MUTABLE_HV(lobj), "alpha", 5 ) ) XSRETURN_YES; else XSRETURN_NO; @@ -447,7 +447,7 @@ VXS(version_is_qv) { SV *lobj; VTYPECHECK(lobj, ST(0), "lobj"); - if ( hv_exists(MUTABLE_HV(SvRV(lobj)), "qv", 2 ) ) + if ( hv_exists(MUTABLE_HV(lobj), "qv", 2 ) ) XSRETURN_YES; else XSRETURN_NO;

Wed Dec 25 22:06:23 2013 john.peacock [...] havurah-software.org - Correspondence added

Subject:	Re: [rt.cpan.org #88458] Put version.pm-specific code in its own file in the perl core
Date:	Wed, 25 Dec 2013 22:06:03 -0500
To:	bug-version [...] rt.cpan.org
From:	John Peacock <john.peacock [...] havurah-software.org>

On 10/25/2013 09:00 AM, Father Chrysostomos via RT wrote: Show quoted text

> Queue: version > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=88458 > > > Here is another patch. I just tried rebasing against blead and got test failures due to a thinko. >

Attached are two patches that apply to your branch that will bring bleadperl into sync with what will hopefully be released to CPAN soon. Note that vxs.inc is now vxs.in in the CPAN release, because I have to rename the functions for the CPAN release. This isn't final, but should give you a good idea of the direction I'm going in. I'm going to also post something to p5p, because I wound up making version::vpp a completely standalone module and I want that to be included in the core as well. I did that so I could deprecate the XS code for all Perl releases prior to 5.10.0. John

Message body is not shown because sender requested not to inline it.

Sun Dec 29 09:18:15 2013 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Wed Dec 25 22:06:23 2013, john.peacock@havurah-software.org wrote: Show quoted text

> On 10/25/2013 09:00 AM, Father Chrysostomos via RT wrote:

> > Queue: version > > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=88458 > > > > > Here is another patch. I just tried rebasing against blead and got > > test failures due to a thinko. > >

> > Attached are two patches that apply to your branch that will bring > bleadperl into sync with what will hopefully be released to CPAN soon. > Note that vxs.inc is now vxs.in in the CPAN release, because I have to > rename the functions for the CPAN release. This isn't final, but > should > give you a good idea of the direction I'm going in.

Thank you. I hope I am not too late, because I think this will be problematic: qx|perl -pe 's/XS_version/XS_version__xs/g; s/XS_UNIVERSAL/XS_UNIVERSAL_xs/' vutil/vxs.in > vutil/vxs.inc|; because Windows will need double quotes there. Also, that command will use the system perl, rather than the one running Makefile.PL. Alternatively, did you see the patch (attached to this ticket) that does it entirely with cpp macros? (It’s the one called ‘Use VXS prefix.txt’.) Anyway, since you say this isn’t final, should I wait, or should I go ahead and merge my branch to blead (with your patches)?

Sun Dec 29 12:11:44 2013 john.peacock [...] havurah-software.org - Correspondence added

Subject:	Re: [rt.cpan.org #88458] Put version.pm-specific code in its own file in the perl core
Date:	Sun, 29 Dec 2013 12:11:23 -0500
To:	bug-version [...] rt.cpan.org
From:	John Peacock <john.peacock [...] havurah-software.org>

On 12/29/2013 09:18 AM, Father Chrysostomos via RT wrote: Show quoted text

> Thank you. I hope I am not too late, because I think this will be problematic: > > qx|perl -pe 's/XS_version/XS_version__xs/g; > s/XS_UNIVERSAL/XS_UNIVERSAL_xs/' vutil/vxs.in > vutil/vxs.inc|; > > because Windows will need double quotes there. Also, that command will use the system perl, rather than the one running Makefile.PL. Alternatively, did you see the patch (attached to this ticket) that does it entirely with cpp macros? (It’s the one called ‘Use VXS prefix.txt’.)

I did not see that patch. I'll try that now. Show quoted text

> > Anyway, since you say this isn’t final, should I wait, or should I go ahead and merge my branch to blead (with your patches)? >

You should wait. I'm seeing some test failures when running tests in core Perl. Specifically, the code in version.pm itself that decides whether to use vpp or vxs needs to go away completely when integrated into blead. version::vxs does not exist in core (and version::vpp is there but unused). Thanks for your patience... John

Sun Dec 29 12:33:22 2013 jpeacock [...] cpan.org - Correspondence added

Two more patches to apply to your branch. The only test failures I am seeing are in porting/* and hence are something that have to be resolved when you merge to blead. I think this is everything apart from some discontinuties in the errors that version::vpp returns for some edge cases (which I am happy to document and ignore).

Subject:

0018-Apply-patch-from-Sprout-to-make-vxs.inc-better.patch

From dd17f8472bb45e438fb453aaca1e60741838e624 Mon Sep 17 00:00:00 2001 From: John Peacock <jpeacock@cpan.org> Date: Sun, 29 Dec 2013 12:26:30 -0500 Subject: [PATCH 18/18] Apply patch from Sprout to make vxs.inc better --- cpan/version/lib/version.pm | 4 +-- vxs.inc | 83 +++++++++++++++++++++++---------------------- 2 files changed, 45 insertions(+), 42 deletions(-) diff --git a/cpan/version/lib/version.pm b/cpan/version/lib/version.pm index e6620c2..b21f1bd 100644 --- a/cpan/version/lib/version.pm +++ b/cpan/version/lib/version.pm @@ -4,13 +4,13 @@ package version; use 5.005_05; use strict; -use vars qw(@ISA $VERSION $CLASS $STRICT $LAX *declare *qv); +use vars qw(@ISA $VERSION $CLASS *declare *qv); $VERSION = 0.9905; $CLASS = 'version'; # avoid using Exporter -use version::regex; +require version::regex; *version::is_lax = \&version::regex::is_lax; *version::is_strict = \&version::regex::is_strict; diff --git a/vxs.inc b/vxs.inc index 25918e6..cb894f2 100644 --- a/vxs.inc +++ b/vxs.inc @@ -3,48 +3,51 @@ #ifdef PERL_CORE # define VXS_CLASS "version" +# define VXSp(name) XS_##name #else # define VXS_CLASS "version::vxs" +# define VXSp(name) VXS_##name #endif +#define VXS(name) XS(VXSp(name)) #ifdef VXS_XSUB_DETAILS # ifdef PERL_CORE {"UNIVERSAL::VERSION", XS_UNIVERSAL_VERSION, NULL}, # else - {VXS_CLASS "::_VERSION", XS_UNIVERSAL_VERSION, NULL}, + {VXS_CLASS "::_VERSION", VXS_UNIVERSAL_VERSION, NULL}, # endif - {VXS_CLASS "::()", XS_version_noop, NULL}, - {VXS_CLASS "::new", XS_version_new, NULL}, - {VXS_CLASS "::parse", XS_version_new, NULL}, - {VXS_CLASS "::(\"\"", XS_version_stringify, NULL}, - {VXS_CLASS "::stringify", XS_version_stringify, NULL}, - {VXS_CLASS "::(0+", XS_version_numify, NULL}, - {VXS_CLASS "::numify", XS_version_numify, NULL}, - {VXS_CLASS "::normal", XS_version_normal, NULL}, - {VXS_CLASS "::(cmp", XS_version_vcmp, NULL}, - {VXS_CLASS "::(<=>", XS_version_vcmp, NULL}, + {VXS_CLASS "::()", VXSp(version_noop), NULL}, + {VXS_CLASS "::new", VXSp(version_new), NULL}, + {VXS_CLASS "::parse", VXSp(version_new), NULL}, + {VXS_CLASS "::(\"\"", VXSp(version_stringify), NULL}, + {VXS_CLASS "::stringify", VXSp(version_stringify), NULL}, + {VXS_CLASS "::(0+", VXSp(version_numify), NULL}, + {VXS_CLASS "::numify", VXSp(version_numify), NULL}, + {VXS_CLASS "::normal", VXSp(version_normal), NULL}, + {VXS_CLASS "::(cmp", VXSp(version_vcmp), NULL}, + {VXS_CLASS "::(<=>", VXSp(version_vcmp), NULL}, # ifdef PERL_CORE {VXS_CLASS "::vcmp", XS_version_vcmp, NULL}, # else - {VXS_CLASS "::VCMP", XS_version_vcmp, NULL}, + {VXS_CLASS "::VCMP", VXS_version_vcmp, NULL}, # endif - {VXS_CLASS "::(bool", XS_version_boolean, NULL}, - {VXS_CLASS "::boolean", XS_version_boolean, NULL}, - {VXS_CLASS "::(+", XS_version_noop, NULL}, - {VXS_CLASS "::(-", XS_version_noop, NULL}, - {VXS_CLASS "::(*", XS_version_noop, NULL}, - {VXS_CLASS "::(/", XS_version_noop, NULL}, - {VXS_CLASS "::(+=", XS_version_noop, NULL}, - {VXS_CLASS "::(-=", XS_version_noop, NULL}, - {VXS_CLASS "::(*=", XS_version_noop, NULL}, - {VXS_CLASS "::(/=", XS_version_noop, NULL}, - {VXS_CLASS "::(abs", XS_version_noop, NULL}, - {VXS_CLASS "::(nomethod", XS_version_noop, NULL}, - {VXS_CLASS "::noop", XS_version_noop, NULL}, - {VXS_CLASS "::is_alpha", XS_version_is_alpha, NULL}, - {VXS_CLASS "::qv", XS_version_qv, NULL}, - {VXS_CLASS "::declare", XS_version_qv, NULL}, - {VXS_CLASS "::is_qv", XS_version_is_qv, NULL}, + {VXS_CLASS "::(bool", VXSp(version_boolean), NULL}, + {VXS_CLASS "::boolean", VXSp(version_boolean), NULL}, + {VXS_CLASS "::(+", VXSp(version_noop), NULL}, + {VXS_CLASS "::(-", VXSp(version_noop), NULL}, + {VXS_CLASS "::(*", VXSp(version_noop), NULL}, + {VXS_CLASS "::(/", VXSp(version_noop), NULL}, + {VXS_CLASS "::(+=", VXSp(version_noop), NULL}, + {VXS_CLASS "::(-=", VXSp(version_noop), NULL}, + {VXS_CLASS "::(*=", VXSp(version_noop), NULL}, + {VXS_CLASS "::(/=", VXSp(version_noop), NULL}, + {VXS_CLASS "::(abs", VXSp(version_noop), NULL}, + {VXS_CLASS "::(nomethod", VXSp(version_noop), NULL}, + {VXS_CLASS "::noop", VXSp(version_noop), NULL}, + {VXS_CLASS "::is_alpha", VXSp(version_is_alpha), NULL}, + {VXS_CLASS "::qv", VXSp(version_qv), NULL}, + {VXS_CLASS "::declare", VXSp(version_qv), NULL}, + {VXS_CLASS "::is_qv", VXSp(version_is_qv), NULL}, #else #ifndef dVAR @@ -64,7 +67,7 @@ typedef char HVNAME; # define HEKf "s" #endif -XS(XS_UNIVERSAL_VERSION) +VXS(UNIVERSAL_VERSION) { dVAR; dXSARGS; @@ -165,7 +168,7 @@ XS(XS_UNIVERSAL_VERSION) XSRETURN(1); } -XS(XS_version_new) +VXS(version_new) { dVAR; dXSARGS; @@ -233,7 +236,7 @@ XS(XS_version_new) Perl_croak(aTHX_ varname " is not of type version"); \ } STMT_END -XS(XS_version_stringify) +VXS(version_stringify) { dVAR; dXSARGS; @@ -251,7 +254,7 @@ XS(XS_version_stringify) } } -XS(XS_version_numify) +VXS(version_numify) { dVAR; dXSARGS; @@ -267,7 +270,7 @@ XS(XS_version_numify) } } -XS(XS_version_normal) +VXS(version_normal) { dVAR; dXSARGS; @@ -285,7 +288,7 @@ XS(XS_version_normal) } } -XS(XS_version_vcmp) +VXS(version_vcmp) { dVAR; dXSARGS; @@ -325,7 +328,7 @@ XS(XS_version_vcmp) } } -XS(XS_version_boolean) +VXS(version_boolean) { dVAR; dXSARGS; @@ -348,7 +351,7 @@ XS(XS_version_boolean) } } -XS(XS_version_noop) +VXS(version_noop) { dVAR; dXSARGS; @@ -361,7 +364,7 @@ XS(XS_version_noop) XSRETURN_EMPTY; } -XS(XS_version_is_alpha) +VXS(version_is_alpha) { dVAR; dXSARGS; @@ -380,7 +383,7 @@ XS(XS_version_is_alpha) } } -XS(XS_version_qv) +VXS(version_qv) { dVAR; dXSARGS; @@ -434,7 +437,7 @@ XS(XS_version_qv) return; } -XS(XS_version_is_qv) +VXS(version_is_qv) { dVAR; dXSARGS; -- 1.8.3.2

Sun Dec 29 12:35:24 2013 jpeacock [...] cpan.org - Correspondence added

Missed this patch

Subject:

0017-Integrate-CPAN-release-of-version.pm-0.9905.patch

From a517ef51bd19bd5e6e33ce3e70f03bd8b349eed1 Mon Sep 17 00:00:00 2001 From: John Peacock <jpeacock@cpan.org> Date: Sun, 29 Dec 2013 11:49:43 -0500 Subject: [PATCH 17/18] Integrate CPAN release of version.pm 0.9905 When adding the CPAN-distributed files for version.pm, it is necessary to delete an entire block out of lib/version.pm, since that code is only necessary with the CPAN release. Within core Perl, there is no version::vxs implementation class any more. --- MANIFEST | 6 +++++- cpan/version/lib/version.pm | 46 ++++----------------------------------------- 2 files changed, 9 insertions(+), 43 deletions(-) diff --git a/MANIFEST b/MANIFEST index 4806631..a5e53de 100644 --- a/MANIFEST +++ b/MANIFEST @@ -2741,7 +2741,10 @@ cpan/Unicode-Normalize/t/tie.t Unicode::Normalize cpan/version/lib/version/Internals.pod Description of the internals of version objects cpan/version/lib/version.pm Support for version objects cpan/version/lib/version.pod Documentation of the version module -cpan/version/t/01base.t Tests for version objects` +cpan/version/lib/version/regex.pm Regex for matching version objects +cpan/version/lib/version/vpp.pm Pure Perl implementation of XS code +cpan/version/t/00impl-pp.t Tests for version objects +cpan/version/t/01base.t Tests for version objects cpan/version/t/02derived.t Tests for version objects cpan/version/t/03require.t Tests for version objects cpan/version/t/04strict_lax.t Tests for version objects @@ -2749,6 +2752,7 @@ cpan/version/t/05sigdie.t Tests for version objects cpan/version/t/06noop.t Tests for version objects cpan/version/t/07locale.t Tests for version objects cpan/version/t/08_corelist.t Tests for version objects +cpan/version/t/09_list_util.t Tests for version objects cpan/version/t/coretests.pm Tests for version objects cpan/Win32API-File/buffers.h Win32API::File extension cpan/Win32API-File/cFile.h Win32API::File extension diff --git a/cpan/version/lib/version.pm b/cpan/version/lib/version.pm index 7e548a8..e6620c2 100644 --- a/cpan/version/lib/version.pm +++ b/cpan/version/lib/version.pm @@ -9,48 +9,10 @@ use vars qw(@ISA $VERSION $CLASS $STRICT $LAX *declare *qv); $VERSION = 0.9905; $CLASS = 'version'; -{ - local $SIG{'__DIE__'}; - eval "use version::vxs $VERSION"; - if ( $@ ) { # don't have the XS version installed - eval "use version::vpp $VERSION"; # don't tempt fate - die "$@" if ( $@ ); - push @ISA, "version::vpp"; - local $^W; - *version::qv = \&version::vpp::qv; - *version::declare = \&version::vpp::declare; - *version::_VERSION = \&version::vpp::_VERSION; - *version::vcmp = \&version::vpp::vcmp; - *version::new = \&version::vpp::new; - if ($] >= 5.009000) { - no strict 'refs'; - *version::stringify = \&version::vpp::stringify; - *{'version::(""'} = \&version::vpp::stringify; - *{'version::(<=>'} = \&version::vpp::vcmp; - *version::parse = \&version::vpp::parse; - } - *version::is_strict = \&version::vpp::is_strict; - *version::is_lax = \&version::vpp::is_lax; - } - else { # use XS module - push @ISA, "version::vxs"; - local $^W; - *version::declare = \&version::vxs::declare; - *version::qv = \&version::vxs::qv; - *version::_VERSION = \&version::vxs::_VERSION; - *version::vcmp = \&version::vxs::VCMP; - *version::new = \&version::vxs::new; - if ($] >= 5.009000) { - no strict 'refs'; - *version::stringify = \&version::vxs::stringify; - *{'version::(""'} = \&version::vxs::stringify; - *{'version::(<=>'} = \&version::vxs::VCMP; - *version::parse = \&version::vxs::parse; - } - *version::is_strict = \&version::vxs::is_strict; - *version::is_lax = \&version::vxs::is_lax; - } -} +# avoid using Exporter +use version::regex; +*version::is_lax = \&version::regex::is_lax; +*version::is_strict = \&version::regex::is_strict; sub import { no strict 'refs'; -- 1.8.3.2

Sun Dec 29 13:00:11 2013 john.peacock [...] havurah-software.org - Correspondence added

Subject:	Re: [rt.cpan.org #88458] Put version.pm-specific code in its own file in the perl core
Date:	Sun, 29 Dec 2013 12:59:59 -0500
To:	bug-version [...] rt.cpan.org
From:	John Peacock <john.peacock [...] havurah-software.org>

On 12/29/2013 12:11 PM, John Peacock via RT wrote: Show quoted text

> You should wait. I'm seeing some test failures when running tests in > core Perl. Specifically, the code in version.pm itself that decides > whether to use vpp or vxs needs to go away completely when integrated > into blead. version::vxs does not exist in core (and version::vpp is > there but unused).

Seeing different core test failures, now related to version::_VERSION not existing.... grrr John

Sun Dec 29 13:50:03 2013 john.peacock [...] havurah-software.org - Correspondence added

Subject:	Re: [rt.cpan.org #88458] Put version.pm-specific code in its own file in the perl core
Date:	Sun, 29 Dec 2013 13:49:53 -0500
To:	bug-version [...] rt.cpan.org
From:	John Peacock <john.peacock [...] havurah-software.org>

On 12/29/2013 01:00 PM, John Peacock via RT wrote: Show quoted text

> Seeing different core test failures, now related to version::_VERSION > not existing....

What made this so maddening to debug is that changing vxs.inc does not correctly rebuild Perl. Both universal.c and util.c had to be touch'd before any changes to vxs.inc were compiled. One more patch, making sure that version::_VERSION is always exported attached. John

Message body is not shown because sender requested not to inline it.

Sun Dec 29 14:46:15 2013 john.peacock [...] havurah-software.org - Correspondence added

Subject:	Re: [rt.cpan.org #88458] Put version.pm-specific code in its own file in the perl core
Date:	Sun, 29 Dec 2013 14:46:05 -0500
To:	bug-version [...] rt.cpan.org
From:	John Peacock <john.peacock [...] havurah-software.org>

On 12/29/2013 01:50 PM, John Peacock via RT wrote: Show quoted text

> Queue: version > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=88458 > > > On 12/29/2013 01:00 PM, John Peacock via RT wrote:

>> Seeing different core test failures, now related to version::_VERSION >> not existing....

> > What made this so maddening to debug is that changing vxs.inc does not > correctly rebuild Perl. Both universal.c and util.c had to be touch'd > before any changes to vxs.inc were compiled. > > One more patch, making sure that version::_VERSION is always exported > attached. > > John >

Turns out we lost the protection from crazy people who try and pass in arrayref's instead of scalars to version->new(). Go figure. Attached is an additional patch to handle that case. John

Message body is not shown because sender requested not to inline it.

Thu Jan 02 09:25:20 2014 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Sun Dec 29 14:46:15 2013, john.peacock@havurah-software.org wrote: Show quoted text

> On 12/29/2013 01:50 PM, John Peacock via RT wrote:

> > Queue: version > > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=88458 > > > > > On 12/29/2013 01:00 PM, John Peacock via RT wrote:

> >> Seeing different core test failures, now related to version::_VERSION > >> not existing....

> > > > What made this so maddening to debug is that changing vxs.inc does not > > correctly rebuild Perl. Both universal.c and util.c had to be touch'd > > before any changes to vxs.inc were compiled. > > > > One more patch, making sure that version::_VERSION is always exported > > attached. > > > > John > >

> > Turns out we lost the protection from crazy people who try and pass in > arrayref's instead of scalars to version->new(). Go figure. > > Attached is an additional patch to handle that case.

Thank you. I have updated perl.git’s sprout/version branch. I await a green light from you before merging it to blead.

Fri Jan 03 14:58:49 2014 john.peacock [...] havurah-software.org - Correspondence added

Subject:	Re: [rt.cpan.org #88458] Put version.pm-specific code in its own file in the perl core
Date:	Fri, 03 Jan 2014 14:58:35 -0500
To:	bug-version [...] rt.cpan.org, undisclosed-recipients:;
From:	John Peacock <john.peacock [...] havurah-software.org>

On 01/02/2014 09:25 AM, Father Chrysostomos via RT wrote: Show quoted text

> > Thank you. I have updated perl.git’s sprout/version branch. I await a green light from you before merging it to blead.

I think that if you have verified my assertion that all tests pass, then you can go ahead and merge it to blead. I haven't had time to run regression tests on all of my edge-case Perl's, and there are probably going to be Makefile.PL and test changes to get it working all the way back to 5.005_06 (pure Perl only), but the XS bits appear to be much improved. I can always submit patches for those bits when I do the official CPAN release. John

Sat Jan 04 08:24:53 2014 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Fri Jan 03 14:58:49 2014, john.peacock@havurah-software.org wrote: Show quoted text

> On 01/02/2014 09:25 AM, Father Chrysostomos via RT wrote:

> > > > Thank you. I have updated perl.git’s sprout/version branch. I await > > a green light from you before merging it to blead.

> > I think that if you have verified my assertion that all tests pass, > then > you can go ahead and merge it to blead. I haven't had time to run > regression tests on all of my edge-case Perl's, and there are probably > going to be Makefile.PL and test changes to get it working all the way > back to 5.005_06 (pure Perl only), but the XS bits appear to be much > improved. I can always submit patches for those bits when I do the > official CPAN release.

Thank you. I have merged it as a76c354d5b.

Sat Jan 04 13:12:13 2014 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Fri Jan 03 14:58:49 2014, john.peacock@havurah-software.org wrote: Show quoted text

> On 01/02/2014 09:25 AM, Father Chrysostomos via RT wrote:

> > > > Thank you. I have updated perl.git’s sprout/version branch. I await > > a green light from you before merging it to blead.

> > I think that if you have verified my assertion that all tests pass, > then > you can go ahead and merge it to blead. I haven't had time to run > regression tests on all of my edge-case Perl's, and there are probably > going to be Makefile.PL and test changes to get it working all the way > back to 5.005_06 (pure Perl only), but the XS bits appear to be much > improved. I can always submit patches for those bits when I do the > official CPAN release.

Wait! One more thing: My changes broke C89 builds. Karl Williamson fixed it in 39ced5adb3 by patching vxs.inc directly in the perl core. I have attached it here. Please include this, so that things stay in synch. I am about to apply the patch you just sent to p5p. Hopefully these are the last two things....

Subject:

Move code to after declarations.text

From 39ced5adb3b09f93e0263d0f679de4cb6d7c2dc9 Mon Sep 17 00:00:00 2001 From: Karl Williamson <public@khwilliamson.com> Date: Sat, 4 Jan 2014 10:43:22 -0700 Subject: [PATCH] vxs.inc: Move code to after declarations This macro, added in e1c774b6, is actual code, and needs to be after the declarations, so that C89 compilers compile it. diff --git a/vxs.inc b/vxs.inc index e297c38..2e4f409 100644 --- a/vxs.inc +++ b/vxs.inc @@ -171,12 +171,13 @@ VXS(version_new) { dVAR; dXSARGS; - PERL_UNUSED_VAR(cv); SV *vs = items ? ST(1) : &PL_sv_undef; SV *rv; const char * classname = ""; STRLEN len; U32 flags = 0; + PERL_UNUSED_VAR(cv); + SP -= items; if (items > 3 || items == 0)

Sat Jan 04 15:57:23 2014 BULKDD [...] cpan.org - Correspondence added

On Wed Dec 25 22:06:23 2013, john.peacock@havurah-software.org wrote: Show quoted text

> I'm going to also post something to p5p, because I wound up making > version::vpp a completely standalone module and I want that to be > included in the core as well. I did that so I could deprecate the XS > code for all Perl releases prior to 5.10.0. > > John

Why is the XS code being deprecated for older Perls?

Sat Jan 04 16:02:14 2014 john.peacock [...] havurah-software.org - Correspondence added

Subject:	Re: [rt.cpan.org #88458] Put version.pm-specific code in its own file in the perl core
Date:	Sat, 04 Jan 2014 16:02:01 -0500
To:	bug-version [...] rt.cpan.org, undisclosed-recipients:;
From:	John Peacock <john.peacock [...] havurah-software.org>

On 01/04/2014 03:57 PM, Daniel Dragan via RT wrote: Show quoted text

> Why is the XS code being deprecated for older Perls? >

Because supporting all of those older Perl's has made the XS code hard to maintain. Now I don't need to have a crazy #ifdef maze (and custom ppperl.h code as well). The pure Perl code is feature-equivalent, albeit somewhat slower. John

Sat Jan 04 18:00:08 2014 BULKDD [...] cpan.org - Correspondence added

On Tue Oct 22 14:20:38 2013, SPROUT wrote: Show quoted text

> On Mon Oct 14 09:31:15 2013, SPROUT wrote:

> > Sorry I’ve not been able to finish this up. Other things in real life > > have been getting in the way, and will continue for some time. > > > > However, I suspect that the ‘const struct xsub_details details[]’ in > > vxs.xs needs to be made static, and possibly renamed to vdetails to > > avoid conflicting with the one in the perl core. (And then that one > > should also be made static; it probably should have been to begin > > with.) Does that work?

> > Attached is a patch to make it static. Does it work for you? > > Sorry, no progress on the bitbucket front yet.

Why does the version::vxs class exist? and why are there glob aliasing in version.pm, the first instance of it I tracked down to https://bitbucket.org/jpeacock/version/commits/32b1c7454fd278b68d969d66c52847b16ca9bb04#chg-lib/version.pm but there is no comment supporting why @ISA wont work. About https://rt.cpan.org/Ticket/Display.html?id=14417 , yes, Window's DLL loader matches by dll file name, directory excluded, to last DLL loaded with same name, but only if just a file name is passed to LoadLibrary or you are importing through import table (an absolute path in import table importing is never done), Perl's Dynaloader uses an absolute path, atleast since 5.6.2 ------------------------------------ 3555: hModule = LoadLibraryExA(PerlDir_mapA(filename), NULL, LOAD_WITH_ALTERED_SEARCH_PATH); ------------------------------------ + filename 0x0140f814 "C:\p56\5.6.2\lib\MSWin32-x86\auto\Socket\Socket.dll" const char * ------------------------------------ the other version.dll on windows is "C:\WINDOWS\system32\version.dll" which contains an API for querying and manipulating version metadata from PE files. Why doesn't the XS version just newXS() under version:: directly? Next, in https://bitbucket.org/jpeacock/version/commits/e42f0b6eee814521c70790d54006e6b30104bccd AKA https://rt.cpan.org/Ticket/Attachment/1261703/668027/open_AiGkwWYI.txt FC create VTYPECHECK macro, but that created a multiple eval problem due to ST() (arg val to VTYPECHECK) macro being recalculated over and over between function calls instead of saving the SV* to a auto. I'll probably add more problem as I continue to understand what happened to version in blead since it wasn't just splitting core version out into a easily replaceable file but other refactoring too all mixed together.

Sat Jan 04 18:26:40 2014 john.peacock [...] havurah-software.org - Correspondence added

Subject:	Re: [rt.cpan.org #88458] Put version.pm-specific code in its own file in the perl core
Date:	Sat, 04 Jan 2014 18:26:29 -0500
To:	bug-version [...] rt.cpan.org
From:	John Peacock <john.peacock [...] havurah-software.org>

On 01/04/2014 06:00 PM, Daniel Dragan via RT wrote: Show quoted text

> > Why does the version::vxs class exist? and why are there glob > aliasing in version.pm,

The version::vxs class exists as the implementation class for version on CPAN only; under Perl, the layers of macros that Sprout created cause that to vanish and there is only version:: class. It exists as an independent class on CPAN because it has to be able to be pluggable in the case where the pure Perl version::vpp replaces it. If you look at the way that the CPAN version.pm works, you will see that there is a clear parallel for each of the implementation classes. It also exists as an independent class because the CPAN release has to replace the core code with functions of different names with different implementations, but intercept the calls by replacing the global symbols. Show quoted text

> the first instance of it I tracked down to > https://bitbucket.org/jpeacock/version/commits/32b1c7454fd278b68d969d66c52847b16ca9bb04#chg-lib/version.pm > but there is no comment supporting why @ISA wont work. About > https://rt.cpan.org/Ticket/Display.html?id=14417 , yes, Window's DLL > loader matches by dll file name, directory excluded, to last DLL > loaded with same name, but only if just a file name is passed to > LoadLibrary or you are importing through import table (an absolute > path in import table importing is never done), Perl's Dynaloader uses > an absolute path, atleast since 5.6.2

I'm not following any of this, however. Is there a problem on Windows and if so, can you open a different ticket with details? Show quoted text

> I'll probably add more problem as I continue to understand what > happened to version in blead since it wasn't just splitting core > version out into a easily replaceable file but other refactoring too > all mixed together.

Actually, there was remarkably little refactoring involved. All of the code in vutil.c used to be copied virtually verbatim to util.c. All of the code in vxs.inc started life as dynamically generated code from vxs.xs that was plopped into universal.c. Over time that generated code drifted as changes were made in core, which is why Sprout undertook the effort to split the code out. Now there is a clear "upstream" and a way for me to easily port changes made in the core back to the CPAN release. I can start removing the maze of #ifdef's in vutil.h, since I no longer need to support any Perl prior to v5.10.0. The XS code in vxs.inc that supports the version:: class itself can be rewritten as proper XS code, instead of the horrible mixture of generated and manually tweaked code. John

Sat Jan 04 19:44:00 2014 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Sat Jan 04 18:26:40 2014, john.peacock@havurah-software.org wrote: Show quoted text

> I can start removing the maze of #ifdef's in vutil.h, since I no > longer need to support any Perl prior to v5.10.0. The XS code in > vxs.inc that supports the version:: class itself can be rewritten as > proper XS code, instead of the horrible mixture of generated and > manually tweaked code.

I am not sure about that last bit. What universal.c #includes needs to be written in C, not XS. Were you thinking of some other way of bootstrapping it?

Sat Jan 04 19:56:27 2014 john.peacock [...] havurah-software.org - Correspondence added

Subject:	Re: [rt.cpan.org #88458] Put version.pm-specific code in its own file in the perl core
Date:	Sat, 04 Jan 2014 19:56:18 -0500
To:	bug-version [...] rt.cpan.org, undisclosed-recipients:;
From:	John Peacock <john.peacock [...] havurah-software.org>

On 01/04/2014 07:44 PM, Father Chrysostomos via RT wrote: Show quoted text

> I am not sure about that last bit. What universal.c #includes needs to > be written in C, not XS. Were you thinking of some other way of > bootstrapping it?

No, I meant rewritten as proper C, not as XS => C.... John

Sat Jan 04 20:53:05 2014 BULKDD [...] cpan.org - Correspondence added

On Sat Jan 04 18:26:40 2014, john.peacock@havurah-software.org wrote: Show quoted text

> On 01/04/2014 06:00 PM, Daniel Dragan via RT wrote:

> > > > Why does the version::vxs class exist? and why are there glob > > aliasing in version.pm,

> > The version::vxs class exists as the implementation class for version > on > CPAN only; under Perl, the layers of macros that Sprout created cause > that to vanish and there is only version:: class. It exists as an > independent class on CPAN because it has to be able to be pluggable in > the case where the pure Perl version::vpp replaces it.

So version::vxs is glob aliased to version::vpp if XS code couldn't be loaded? I didn't notice any code that did that. I know version:: is aliased to version::vpp or version::vxs, but don't see version::vpp or version::vxs being aliased to each other. Show quoted text

> If you look at > the way that the CPAN version.pm works, you will see that there is a > clear parallel for each of the implementation classes. It also exists > as an independent class because the CPAN release has to replace the > core > code with functions of different names with different implementations, > but intercept the calls by replacing the global symbols. >

You didn't answer why not overwrite the version:: directly from vpp.pm or vxs.inc? Its an extra stash HV and extra GVs for little gain of an API that isn't public. IIRC you mentioned somewhere you want to ship VPP as a separate CPAN distro. Show quoted text

> > the first instance of it I tracked down to > > https://bitbucket.org/jpeacock/version/commits/32b1c7454fd278b68d969d66c52847b16ca9bb04#chg- > > lib/version.pm > > but there is no comment supporting why @ISA wont work. About > > https://rt.cpan.org/Ticket/Display.html?id=14417 , yes, Window's DLL > > loader matches by dll file name, directory excluded, to last DLL > > loaded with same name, but only if just a file name is passed to > > LoadLibrary or you are importing through import table (an absolute > > path in import table importing is never done), Perl's Dynaloader uses > > an absolute path, atleast since 5.6.2

> > I'm not following any of this, however. Is there a problem on Windows > and if so, can you open a different ticket with details?

There is no problem, just bad design, there just isn't a reason for version::vxs class to exist, "MODULE = version::vxs PACKAGE = version" would have fixed the problem. A different solution, I haven't tested (I'd try to reproduce the problem described in the ticket first), less efficient, is to require() Win32:: inside version.pm before calling XSLoader or DynaLoader so MS's version.dll is loaded first and becomes the primary version.dll seen when an import of "version.dll" needs to be resolved after version's XS DLL loaded. For details, see http://msdn.microsoft.com/en-us/library/windows/desktop/ms684179%28v=vs.85%29.aspx and start reading at Parameters -> lpFileName [in] -> Paragraph 4 ("If the string specifies a fully qualified path,"). The complaint in #14417 is the following scenario. foo.dll is a non-Perl dll, which import table links to string "version.dll". foo.dll wont load if version.dll the XS dll is first dll with a file name of "version.dll" loaded into the process because version.dll the XS dll doesn't export the same functions as MS's version.dll which are (dumped from WinXP SP3) ---------------------------- 1 0 00001A40 GetFileVersionInfoA 2 1 000019EF GetFileVersionInfoSizeA 3 2 0000138C GetFileVersionInfoSizeW 4 3 0000166F GetFileVersionInfoW 5 4 000024CD VerFindFileA 6 5 000033A8 VerFindFileW 7 6 000026F7 VerInstallFileA 8 7 00003756 VerInstallFileW 9 8 VerLanguageNameA (forwarded to KERNEL32.VerLanguageNam eA) 10 9 VerLanguageNameW (forwarded to KERNEL32.VerLanguageNam eW) 11 A 000018AA VerQueryValueA 12 B 00002958 VerQueryValueIndexA 13 C 0000297F VerQueryValueIndexW 14 D 00001805 VerQueryValueW ---------------------------- none of the above will be found in the import table of version.dll the XS dll so foo.dll won't load. 3rd solution, much more complicated solution is for version.dll the XS dll to have Win32 specific code to export all those MS subs, with the body of each function being a stub that calls a C non-const static function pointer, which was filled in DllMain (called unsafe by MS) or in BOOT:. The function pointers were GetProcAddr'ed from a "%windir%\System32\version.dll" (an absolute path) so they are from the correct, MS, version.dll. When a non-Perl DLL imports "version.dll", they get func * to XS version.dll, whose function calls the MS version.dll. I'm not sure if version.dll got additional functions between Win2K, WinXP, and Win7. The choice what to do for unimplented MS version.dll funcs can be SEGV with NULL, or C std lib process exit, or rewriting the in-memory copy of the export table of XS version.dll. Show quoted text

>

> > I'll probably add more problem as I continue to understand what > > happened to version in blead since it wasn't just splitting core > > version out into a easily replaceable file but other refactoring too > > all mixed together.

> > Actually, there was remarkably little refactoring involved.

Nearly every line in xsub version_new was changed between pre-split blead version and the final merge blead. --------------------------------------------------------------- @@ -118,55 +167,75 @@ XS(XS_UNIVERSAL_VERSION) XSRETURN(1); } -XS(XS_version_new) +VXS(version_new) { dVAR; dXSARGS; - if (items > 3 || items < 1) - croak_xs_usage(cv, "class, version"); + PERL_UNUSED_VAR(cv); + SV *vs = items ? ST(1) : &PL_sv_undef; + SV *rv; + const char * classname = ""; + STRLEN len; + U32 flags = 0; SP -= items; - { - SV *vs = ST(1); - SV *rv; - STRLEN len; - const char *classname; - U32 flags; - - /* Just in case this is something like a tied hash */ - SvGETMAGIC(vs); - - if ( sv_isobject(ST(0)) ) { /* get the class if called as an object method */ - const HV * stash = SvSTASH(SvRV(ST(0))); - classname = HvNAME(stash); - len = HvNAMELEN(stash); - flags = HvNAMEUTF8(stash) ? SVf_UTF8 : 0; - } - else { - classname = SvPV(ST(0), len); - flags = SvUTF8(ST(0)); - } - if ( items == 1 || ! SvOK(vs) ) { /* no param or explicit undef */ - /* create empty object */ - vs = sv_newmortal(); - sv_setpvs(vs, "0"); - } - else if ( items == 3 ) { - vs = sv_newmortal(); - Perl_sv_setpvf(aTHX_ vs,"v%s",SvPV_nolen_const(ST(2))); - } + if (items > 3 || items == 0) + Perl_croak(aTHX_ "Usage: version::new(class, version)"); - rv = new_version(vs); - if ( strnNE(classname,"version", len) ) /* inherited new() */ - sv_bless(rv, gv_stashpvn(classname, len, GV_ADD | flags)); + /* Just in case this is something like a tied hash */ + SvGETMAGIC(vs); - mPUSHs(rv); - PUTBACK; - return; + if ( items == 1 || ! SvOK(vs) ) { /* no param or explicit undef */ + /* create empty object */ + vs = sv_newmortal(); + sv_setpvs(vs,"undef"); + } + else if (items == 3 ) { + vs = sv_newmortal(); +#if PERL_VERSION == 5 + sv_setpvf(vs,"v%s",SvPV_nolen_const(ST(2))); +#else + Perl_sv_setpvf(aTHX_ vs,"v%s",SvPV_nolen_const(ST(2))); +#endif + } + if ( sv_isobject(ST(0)) ) { + /* get the class if called as an object method */ + const HV * stash = SvSTASH(SvRV(ST(0))); + classname = HvNAME_get(stash); + len = HvNAMELEN_get(stash); +#ifdef HvNAMEUTF8 + flags = HvNAMEUTF8(stash) ? SVf_UTF8 : 0; +#endif + } + else { + classname = SvPV(ST(0), len); + flags = SvUTF8(ST(0)); } + + rv = NEW_VERSION(vs); + if ( len != sizeof(VXS_CLASS)-1 + || strcmp(classname,VXS_CLASS) != 0 ) /* inherited new() */ +#if PERL_VERSION == 5 + sv_bless(rv, gv_stashpv((char *)classname, GV_ADD)); +#else + sv_bless(rv, gv_stashpvn(classname, len, GV_ADD | flags)); +#endif + + mPUSHs(rv); + PUTBACK; + return; } ---------------------------------------------------------------- Show quoted text

> All of > the > code in vutil.c used to be copied virtually verbatim to util.c. All > of > the code in vxs.inc started life as dynamically generated code from > vxs.xs that was plopped into universal.c. Over time that generated > code > drifted as changes were made in core, which is why Sprout undertook > the > effort to split the code out. Now there is a clear "upstream" and a > way > for me to easily port changes made in the core back to the CPAN > release. >

This should have been done sooner or a no edit policy to util.c since I've gotten commits into blead (and I don't have a commit bit) for version:: in util.c with no objections rather than it being copy paste from CPAN version::. Good it is finally done. Show quoted text

> I can start removing the maze of #ifdef's in vutil.h, since I no > longer need to support any Perl prior to v5.10.0.

I personally don't find much difficulty supporting 5.6 and 5.8 with XS. If ppport.h doesn't have it, I cpan grep for the equivalent or copy it out of p5p core from when it was first introduced into blead. Show quoted text

> The XS code in > vxs.inc that supports the version:: class itself can be rewritten as > proper XS code, instead of the horrible mixture of generated and > manually tweaked code.

If that is done, miniperl during the build process won't have any version:: to use since xsubpp won't work until it is Makefile.PL'ed and make all'ed by miniperl. hand written XS is more efficient than what xsubpp produces anyway. Next issue, https://bitbucket.org/jpeacock/version/src/9f49d95d787c38ad5b6fcdab71f1ce7c95a1b3d3/vutil/vxs.inc?at=default#cl-158 Why does var ret exist? added in https://bitbucket.org/jpeacock/version/commits/2a7768a85f8bd94e5a0648db79ac086fd51e4541 seems to be obsolete in https://bitbucket.org/jpeacock/version/commits/d5c1282dfe131324776ad64f8a3fa2696d997dd1

Sat Jan 04 21:15:19 2014 BULKDD [...] cpan.org - Cc BULKDD added

Sat Jan 04 22:17:44 2014 john.peacock [...] havurah-software.org - Correspondence added

Subject:	Re: [rt.cpan.org #88458] Put version.pm-specific code in its own file in the perl core
Date:	Sat, 04 Jan 2014 22:17:33 -0500
To:	bug-version [...] rt.cpan.org, undisclosed-recipients:;
From:	John Peacock <john.peacock [...] havurah-software.org>

On 01/04/2014 08:53 PM, Daniel Dragan via RT wrote: Show quoted text

> So version::vxs is glob aliased to version::vpp if XS code couldn't > be loaded? I didn't notice any code that did that. I know version:: > is aliased to version::vpp or version::vxs, but don't see > version::vpp or version::vxs being aliased to each other.

I think we are talking past each other here. version:vxs and version:vpp provide the same interface and version:: methods are aliased to the appropriate implementation class at load time. It was much simpler to maintain that parallel construction for consistency then it was to have version:vxs export methods directly into the version:: namespace. Show quoted text

> You didn't answer why not overwrite the version:: directly from > vpp.pm or vxs.inc? Its an extra stash HV and extra GVs for little > gain of an API that isn't public. IIRC you mentioned somewhere you > want to ship VPP as a separate CPAN distro.

No, I'm just saying version:vpp is now installed even if the version:vxs code is installed too. It won't ever be an independent module. Show quoted text

>> >> Actually, there was remarkably little refactoring involved.

I meant from the point of view of the CPAN release, which has always been considered the "upstream". What was in the core had drifted away from the CPAN release and this effort was to make that not happen any longer. Any improvements that need to be made will happen in the CPAN release first, then go into core (in the ideal case). Show quoted text

> Why does var ret exist? > > added in > https://bitbucket.org/jpeacock/version/commits/2a7768a85f8bd94e5a0648db79ac086fd51e4541 > seems to be obsolete in > https://bitbucket.org/jpeacock/version/commits/d5c1282dfe131324776ad64f8a3fa2696d997dd1 >

That is an actual bug; there was an extensive discussion about what UNIVERSAL::VERSION should return and several patches went back and forth (I won't find the thread, since the discussion was concluded successfully ). It appears that the patch to remove "ret" wasn't applied correctly to the CPAN code, so it snuck back in during the reconciliation. I am going to open a new bug ticket on CPAN's RT to fix it there. John

Sat Jan 04 22:18:42 2014 BULKDD [...] cpan.org - Correspondence added

About ------------------------------------------- SV * Perl_new_version2(pTHX_ SV *ver); SV * Perl_upg_version2(pTHX_ SV *sv, bool qv); SV * Perl_vstringify2(pTHX_ SV *vs); SV * Perl_vverify2(pTHX_ SV *vs); SV * Perl_vnumify2(pTHX_ SV *vs); SV * Perl_vnormal2(pTHX_ SV *vs); SV * Perl_vstringify2(pTHX_ SV *vs); int Perl_vcmp2(pTHX_ SV *lsv, SV *rsv); const char * Perl_prescan_version2(pTHX_ const char *s, bool strict, const char** errstr, bool *sqv, int *ssaw_decimal, int *swidth, bool *salpha); ------------------------------------------- In the current setup, if version is built as core, it winds up in libperl.so/perl5**.dll, with non-"2" functions being exposed to XS code, if version is built as CPAN, the "2" functions show up, and all XS code still calls the "old" version functions. Considered a vtable (on Win32) or extern'ing them and LD_PRELOAD hooking on *nix so if you upgrade your version:: from cpan, XSUBs also call the new version:: in the process? What about a lazy XS loader that requires the full version binary at the first time it is used ( Win32:: used to be compiled core, like universal::, it was later moved to CPAN, with a lazy loader at http://perl5.git.perl.org/perl.git/blob/HEAD:/ext/Win32CORE/Win32CORE.c )?

Sat Jan 04 23:41:13 2014 BULKDD [...] cpan.org - Correspondence added

On Sat Jan 04 18:00:08 2014, BULKDD wrote: Show quoted text

> Next, in > https://bitbucket.org/jpeacock/version/commits/e42f0b6eee814521c70790d54006e6b30104bccd > AKA > https://rt.cpan.org/Ticket/Attachment/1261703/668027/open_AiGkwWYI.txt > FC create VTYPECHECK macro, but that created a multiple eval problem > due to ST() (arg val to VTYPECHECK) macro being recalculated over and > over between function calls instead of saving the SV* to a auto.

Related problem, https://bitbucket.org/jpeacock/version/src/9f49d95d787c38ad5b6fcdab71f1ce7c95a1b3d3/vutil/vutil.h?at=default#cl-86 "#define ISA_CLASS_OBJ(v,c) (sv_isobject(v) && sv_derived_from(v,c))" does an unnecessary strlen, use sv_derived_from_pvn. https://bitbucket.org/jpeacock/version/src/9f49d95d787c38ad5b6fcdab71f1ce7c95a1b3d3/vutil/vutil.c?at=default#cl-487 keys "width" and "original" are redundantly fetched twice from the old version hv/sv in new_version. https://bitbucket.org/jpeacock/version/src/9f49d95d787c38ad5b6fcdab71f1ce7c95a1b3d3/vutil/vutil.c?at=default#cl-503 "const I32 rev = SvIV(*av_fetch(sav, key, FALSE));" av_fetch is called 3 times in this macro. https://bitbucket.org/jpeacock/version/src/9f49d95d787c38ad5b6fcdab71f1ce7c95a1b3d3/vutil/vutil.c?at=default#cl-515 ----------------------------------------------- char * const version = savepvn( (const char*)mg->mg_ptr, len); sv_setpvn(rv,version,len); /* this is for consistency with the pure Perl class */ if ( isDIGIT(*version) ) sv_insert(rv, 0, 0, "v", 1); Safefree(version); ---------------------------------- Why was savepvn called? remove savepvn or use sv_usepvn_flags.

Sun Jan 05 10:31:57 2014 john.peacock [...] havurah-software.org - Correspondence added

Subject:	Re: [rt.cpan.org #88458] Put version.pm-specific code in its own file in the perl core
Date:	Sun, 05 Jan 2014 10:31:47 -0500
To:	bug-version [...] rt.cpan.org, undisclosed-recipients:;
From:	John Peacock <john.peacock [...] havurah-software.org>

On 01/04/2014 11:41 PM, Daniel Dragan via RT wrote: Show quoted text

> Related problem, > https://bitbucket.org/jpeacock/version/src/9f49d95d787c38ad5b6fcdab71f1ce7c95a1b3d3/vutil/vutil.h?at=default#cl-86 > "#define ISA_CLASS_OBJ(v,c) (sv_isobject(v) && sv_derived_from(v,c))" > does an unnecessary strlen, use sv_derived_from_pvn.

That function is not documented in perlapi, so I was unaware of its existence. DONE. Show quoted text

> > https://bitbucket.org/jpeacock/version/src/9f49d95d787c38ad5b6fcdab71f1ce7c95a1b3d3/vutil/vutil.c?at=default#cl-487 > keys "width" and "original" are redundantly fetched twice from the > old version hv/sv in new_version.

In both cases, the first call is hv_exists and the second is hv_fetch. Are you suggesting I go straight to fetch (into a temporary), then add to the new version? Show quoted text

> > https://bitbucket.org/jpeacock/version/src/9f49d95d787c38ad5b6fcdab71f1ce7c95a1b3d3/vutil/vutil.c?at=default#cl-503 > >"const I32 rev = SvIV(*av_fetch(sav, key, FALSE));" av_fetch is called 3

times in this macro. And? I don't see anything in perlapi about copying an av so I have to iterate over every element and push it to the new copy. Show quoted text

> > https://bitbucket.org/jpeacock/version/src/9f49d95d787c38ad5b6fcdab71f1ce7c95a1b3d3/vutil/vutil.c?at=default#cl-515 > > ----------------------------------------------- char * const version > = savepvn( (const char*)mg->mg_ptr, len); sv_setpvn(rv,version,len); > /* this is for consistency with the pure Perl class */ if ( > isDIGIT(*version) ) sv_insert(rv, 0, 0, "v", 1); Safefree(version); > ---------------------------------- Why was savepvn called? remove > savepvn or use sv_usepvn_flags. >

I'm not clear what you are advocating here. I need to be able to reference the original v-string twice: once to copy it into rv and once to potentially insert a leading 'v' (for consistency in the later parse). Is sv_usepvn_flags() that much faster for this usecase (only bare v-strings)? John

Sun Jan 05 11:13:19 2014 john.peacock [...] havurah-software.org - Correspondence added

Subject:	Re: [rt.cpan.org #88458] Put version.pm-specific code in its own file in the perl core
Date:	Sun, 05 Jan 2014 11:13:10 -0500
To:	bug-version [...] rt.cpan.org, undisclosed-recipients:;
From:	John Peacock <john.peacock [...] havurah-software.org>

On 01/04/2014 10:18 PM, Daniel Dragan via RT wrote: Show quoted text

> libperl.so/perl5**.dll, with non-"2" functions being exposed to XS > code, if version is built as CPAN, the "2" functions show up, and all > XS code still calls the "old" version functions.

Not all new code calls the old functions. The macros in vutil.h: Show quoted text

> # define SCAN_VERSION(a,b,c) Perl_scan_version2(aTHX_ a,b,c) > # define NEW_VERSION(a) Perl_new_version2(aTHX_ a) > # define UPG_VERSION(a,b) Perl_upg_version2(aTHX_ a, b) > # define VSTRINGIFY(a) Perl_vstringify2(aTHX_ a) > # define VVERIFY(a) Perl_vverify2(aTHX_ a) > # define VNUMIFY(a) Perl_vnumify2(aTHX_ a) > # define VNORMAL(a) Perl_vnormal2(aTHX_ a) > # define VCMP(a,b) Perl_vcmp2(aTHX_ a,b) > # define PRESCAN_VERSION(a,b,c,d,e,f,g) Perl_prescan_version2(aTHX_ a,b,c,d,e,f,g)

ensure that the replacement functions are called. NOTE that not all replacement functions are required to be called, however. The only functions that MUST be replaced in the XS code are: NEW_VERSION UPG_VERSION and the C code from vutil.c additionally calls the replacements: UPG_VERSION PRESCAN_VERSION SCAN_VERSION All of the other replacement methods have not changed (yet) and so the core code can be called freely and is completely equivalent to the CPAN release. For clarity's sake, I supposed could go through and make all of the calls in vutil.c use the macros so that the "*2" functions would always be called by the CPAN code. That wasn't practical until the codebases were unified. John

Sun Jan 05 11:18:28 2014 john.peacock [...] havurah-software.org - Correspondence added

Subject:	Re: [rt.cpan.org #88458] Put version.pm-specific code in its own file in the perl core
Date:	Sun, 05 Jan 2014 11:18:19 -0500
To:	bug-version [...] rt.cpan.org, undisclosed-recipients:;
From:	John Peacock <john.peacock [...] havurah-software.org>

On 01/05/2014 11:13 AM, John Peacock via RT wrote: Show quoted text

> ensure that the replacement functions are called. NOTE that not all > replacement functions are required to be called, however. The only > functions that MUST be replaced in the XS code are: > > NEW_VERSION > UPG_VERSION

Ignore that; all the XS methods call the macro forms for all functions. That should teach me not to answer from memory instead of reading the code. John

Sun Jan 05 14:24:57 2014 $_ = 'spro^^*%*^6ut# [...] &$%*c>#!^!#&!pan.org'; y/a-z.@//cd; print - Correspondence added

On Sun Jan 05 10:31:57 2014, john.peacock@havurah-software.org wrote: Show quoted text

> On 01/04/2014 11:41 PM, Daniel Dragan via RT wrote:

> > > > https://bitbucket.org/jpeacock/version/src/9f49d95d787c38ad5b6fcdab71f1ce7c95a1b3d3/vutil/vutil.c?at=default#cl- > > 503 > > > > "const I32 rev = SvIV(*av_fetch(sav, key, FALSE));" av_fetch is > > called 3

> times in this macro. > > And? I don't see anything in perlapi about copying an av so I have to > iterate over every element and push it to the new copy.

SvIV(foo()) calls foo() three times. SvIVx(foo()) calls it once. Show quoted text

>

> > > > https://bitbucket.org/jpeacock/version/src/9f49d95d787c38ad5b6fcdab71f1ce7c95a1b3d3/vutil/vutil.c?at=default#cl- > > 515 > > > > ----------------------------------------------- char * const version > > = savepvn( (const char*)mg->mg_ptr, len); sv_setpvn(rv,version,len); > > /* this is for consistency with the pure Perl class */ if ( > > isDIGIT(*version) ) sv_insert(rv, 0, 0, "v", 1); Safefree(version); > > ---------------------------------- Why was savepvn called? remove > > savepvn or use sv_usepvn_flags. > >

> > I'm not clear what you are advocating here. I need to be able to > reference the original v-string twice: once to copy it into rv and > once > to potentially insert a leading 'v' (for consistency in the later > parse). Is sv_usepvn_flags() that much faster for this usecase (only > bare v-strings)?

The string buffer gets copied twice the way the code is currently written (once by savepvn, once by sv_setpvn). Either passing mg->mg_ptr directly to sv_setpvn (avoiding savepvn) or using sv_usepvn instead of sv_setpvn + Safefree would cause it to be copied just once.

Mon Jan 06 02:19:37 2014 BULKDD [...] cpan.org - Correspondence added

On Sun Jan 05 11:13:19 2014, john.peacock@havurah-software.org wrote: Show quoted text

> On 01/04/2014 10:18 PM, Daniel Dragan via RT wrote:

> > libperl.so/perl5**.dll, with non-"2" functions being exposed to XS > > code, if version is built as CPAN, the "2" functions show up, and all > > XS code still calls the "old" version functions.

> > Not all new code calls the old functions. The macros in vutil.h:

I wasn't clear. If lets say Socket, the XS module, uses "new_version", it will always link and use to the libperl.so/perl5**.dll version of new_version. If you load a newer CPAN version:: into the process, the CV version::new() and CV version::vxs::new() goes to the new version:: code and to the new C func Perl_new_version2, while the Socket's "new_version" still goes to the old built in core version:: (AKA C func Perl_new_version) in libperl.so. My proposal is, Socket's "new_version" being forwarded to Perl_new_version2 when a new CPAN version:: is loaded into the process. Socket is NOT recompiled in this scheme. This scheme probably requires support in p5p's interp for core'd version:: to have a vtable that CPAN version:: can replace at runtime (use/require time).

Mon Jan 13 06:48:48 2014 jpeacock [...] cpan.org - Correspondence added

Fixed in 0.9906

Mon Jan 13 06:48:49 2014 jpeacock [...] cpan.org - Status changed from 'open' to 'resolved'

Mon Jan 13 06:48:49 2014 jpeacock [...] cpan.org - Fixed in 0.9906 added