[patch] constant.pm: remove 5.6 checks, small test adjustment for 5.8

p5pRT commented 12 years ago

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

Searchable as RT114770$

p5pRT commented 12 years ago

From @maddingue

This is a bug report for perl from saper@cpan.org\, generated with the help of perlbug 1.39 running under perl 5.16.1.

Hello\,

Attached are two patches for constant.pm: * constant.pm-1.24-remove-5.6-checks.diff Given constant.pm is no longer compatible with Perl 5.6 and previous\, the parts of the code that check for these versions can be removed.

* constant.pm-1.24-utf8-test.diff Needed so the tests pass on Perl 5.8.0 to 5.8.3

Regards\,

Sébastien Aperghis-Tramoni

file: constant.pm-1.24-remove-5.6-checks.diff

Inline Patch

```diff --- blead/dist/constant/lib/constant.pm 2012-09-04 11:15:26.000000000 +0200 +++ blead/dist/constant/lib/constant.pm 2012-09-05 21:25:14.000000000 +0200 @@ -1,5 +1,5 @@ package constant; -use 5.005; +use 5.008; use strict; use warnings::register; @@ -17,10 +17,9 @@ my %forbidden = (%keywords, %forced_into_main); -my $str_end = $] >= 5.006 ? "\\z" : "\\Z"; -my $normal_constant_name = qr/^_?[^\W_0-9]\w*$str_end/; -my $tolerable = qr/^[A-Za-z_]\w*$str_end/; -my $boolean = qr/^[01]?$str_end/; +my $normal_constant_name = qr/^_?[^\W_0-9]\w*\z/; +my $tolerable = qr/^[A-Za-z_]\w*\z/; +my $boolean = qr/^[01]?\z/; BEGIN { # We'd like to do use constant _CAN_PCS => $] > 5.009002 --- blead/dist/constant/t/constant.t 2009-12-28 13:04:37.000000000 +0100 +++ blead/dist/constant/t/constant.t 2012-09-04 11:17:30.000000000 +0200 @@ -167,7 +167,6 @@ @warnings = (); eval q{ no warnings; - #local $^W if $] < 5.006; use warnings 'constant'; use constant 'BEGIN' => 1 ; use constant 'INIT' => 1 ; ```

file: constant.pm-1.24-utf8-test.diff

Inline Patch

```diff --- blead/dist/constant/t/utf8.t 2011-04-18 10:16:25.000000000 +0200 +++ blead/dist/constant/t/utf8.t 2012-09-03 17:04:36.000000000 +0200 @@ -1,9 +1,15 @@ #!./perl -T +use Test::More; +BEGIN { + plan skip_all => "irrelevant on pre-5.8.4" if $] < 5.008004 +} + # Tests for constant.pm that require the utf8 pragma use utf8; -use Test::More tests => 2; + +plan tests => 2; use constant π => 4 * atan2 1, 1; --- ```

Flags: category=library severity=low module=constant

Site configuration information for perl 5.16.1:

Configured by maddingue at Tue Aug 14 00:18:54 CEST 2012.

Summary of my perl5 (revision 5 version 16 subversion 1) configuration:
Platform: osname=darwin\, osvers=11.4.0\, archname=darwin-2level uname='darwin landroval.local 11.4.0 darwin kernel version 11.4.0: mon apr 9 19:32:15 pdt 2012; root:xnu-1699.26.8~1release_x86_64 x86_64 ' config_args='-des -Dprefix=/opt/perl/5.16.1' hint=recommended\, useposix=true\, d_sigaction=define useithreads=undef\, usemultiplicity=undef useperlio=define\, d_sfio=undef\, uselargefiles=define\, usesocks=undef use64bitint=define\, use64bitall=define\, uselongdouble=undef usemymalloc=n\, bincompat5005=undef Compiler: cc='cc'\, ccflags ='-fno-common -DPERL_DARWIN -fno-strict-aliasing -pipe -fstack-protector -I/opt/local/include'\, optimize='-O3'\, cppflags='-fno-common -DPERL_DARWIN -fno-strict-aliasing -pipe -fstack-protector -I/opt/local/include' ccversion=''\, gccversion='4.2.1 Compatible Apple Clang 3.1 (tags/Apple/clang-318.0.45)'\, gccosandvers='' intsize=4\, longsize=8\, ptrsize=8\, doublesize=8\, byteorder=12345678 d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=16 ivtype='long'\, ivsize=8\, nvtype='double'\, nvsize=8\, Off_t='off_t'\, lseeksize=8 alignbytes=8\, prototype=define Linker and Libraries: ld='env MACOSX_DEPLOYMENT_TARGET=10.3 cc'\, ldflags =' -fstack-protector -L/usr/local/lib -L/opt/local/lib' libpth=/usr/local/lib /opt/local/lib /usr/lib libs=-lgdbm -ldbm -ldl -lm -lutil -lc perllibs=-ldl -lm -lutil -lc libc=\, so=dylib\, useshrplib=false\, libperl=libperl.a gnulibc_version='' Dynamic Linking: dlsrc=dl_dlopen.xs\, dlext=bundle\, d_dlsymun=undef\, ccdlflags=' ' cccdlflags=' '\, lddlflags=' -bundle -undefined dynamic_lookup -L/usr/local/lib -L/opt/local/lib -fstack-protector'

Locally applied patches:

@INC for perl 5.16.1: /opt/perl/5.16.1/lib/site_perl/5.16.1/darwin-2level /opt/perl/5.16.1/lib/site_perl/5.16.1 /opt/perl/5.16.1/lib/5.16.1/darwin-2level /opt/perl/5.16.1/lib/5.16.1 .

Environment for perl 5.16.1: DYLD_LIBRARY_PATH (unset) HOME=/Users/maddingue LANG=fr_FR.UTF-8 LANGUAGE (unset) LD_LIBRARY_PATH (unset) LOGDIR (unset) PATH=/opt/perl/current/bin:/opt/local/bin:/opt/local/sbin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/X11/bin PERL_BADLANG (unset) SHELL=/bin/bash

p5pRT commented 12 years ago

From @rurban

On Wed\, Sep 5\, 2012 at 3:13 PM\, Sebastien Aperghis-Tramoni \perlbug\-followup@perl\.org wrote:

# New Ticket Created by Sebastien Aperghis-Tramoni # Please include the string: [perl #114770] # in the subject line of all future correspondence about this issue. # \<URL: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=114770 >

This is a bug report for perl from saper@cpan.org\, generated with the help of perlbug 1.39 running under perl 5.16.1.

Hello\,

Attached are two patches for constant.pm: * constant.pm-1.24-remove-5.6-checks.diff Given constant.pm is no longer compatible with Perl 5.6 and previous\, the parts of the code that check for these versions can be removed.

* constant.pm-1.24-utf8-test.diff Needed so the tests pass on Perl 5.8.0 to 5.8.3

Veto.

constant does not work on 5.6 anymore because the former change was wrong. Rather fix the one-line mistake.

I'll come up with a patch\, but I'm lying sick in bed for a few days.

Why are you guys are so destructive all the time? -- Reini Urban http://cpanel.net/ http://www.perl-compiler.org/

p5pRT commented 12 years ago

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

p5pRT commented 12 years ago

From @iabyn

On Wed\, Sep 05\, 2012 at 07:40:39PM -0500\, Reini Urban wrote:

Why are you guys are so destructive all the time?

Why are you an obnoxious arsehole all the time?

-- In England there is a special word which means the last sunshine of the summer. That word is "spring".

p5pRT commented 12 years ago

From @nwc10

I'm going to beat Ricardo to this:

On Thu\, Sep 06\, 2012 at 11:53:13AM +0100\, Dave Mitchell wrote:

On Wed\, Sep 05\, 2012 at 07:40:39PM -0500\, Reini Urban wrote:

Why are you guys are so destructive all the time?

Why are you an obnoxious arsehole all the time?

That sort of language is not acceptable on this list from *anyone*. You\, me\, Ricardo (or Reini). That sort of personal attack is also out of place.

Whatever you feel. However true it is (or might seem to be).

(And to be clear here\, I don't agree with your statement)

Nicholas Clark

p5pRT commented 12 years ago

From @ap

* Reini Urban \rurban@x\-ray\.at [2012-09-06 02:45]:

Why are you guys are so destructive all the time?

People work on what they care about. So do you. Most people consider Perl 5.6 old and smelly. Not so you. They have their reasons\, and you have yours\, both of which have some rational (or at least -seeming) basis.

If you are the only one who cares about something\, it will fall on you to constantly keep it alive. That’s just how it is.

That’s not malice. That is just the consequence of people working on what they care about\, not what you care about – just like you work on what you care about and not on what random other people care about.

But if you are willing to take on the work for what you care about and it does not hinder people from working on what they care\, I think you will find they will not actively fight you. They just want to avoid what from their perspective seems like a waste of effort. For that reason they will sometimes argue\, but I think you will find that the reason is not unreasoned hatred of 5.6 but simply wasted effort aversion. So if you can convince them that letting you keep 5.6 alive will not force them to go to extra effort\, because you are willing to foot the bill\, they will concede and cooperate.

That is what I have seen happen to you so far\, at least.

People aren’t destructive or against you\, you are simply the only one who cares about certain things. That sucks\, because you are left to watch out for the needs of those things alone\, but there’s no helping that when you’re on your own. Throwing around accusations will achieve little else than making an already lonely job harder on yourself.

Sorry. :-/

Regards\, -- Aristotle Pagaltzis // \<http://plasmasturm.org/>

p5pRT commented 12 years ago

From @renormalist

Aristotle Pagaltzis \pagaltzis@gmx\.de writes:

* Reini Urban \rurban@x\-ray\.at [2012-09-06 02:45]:

Why are you guys are so destructive all the time? People aren’t destructive or against you\, you are simply the only one who cares about certain things.

Just to put in a statement from the side: Besides the style of discussion *I* care for the speed meditations that Reini regularly says.

It's very fair to negotiate communication style but don't forget the content over it.

Speed matters\, that's a fact. Not allowing discussion before Reini comes up with the silver bullet isn't fair either.

Kind regards\, Steffen -- Steffen Schwigon \ss5@renormalist\.net Perl benchmarks \<http://perlformance.net> Dresden Perl Mongers \<http://dresden-pm.org/>

p5pRT commented 12 years ago

From @maddingue

Steffen Schwigon wrote via RT:

Aristotle Pagaltzis \pagaltzis@gmx\.de writes:

* Reini Urban \rurban@x\-ray\.at [2012-09-06 02:45]:

Why are you guys are so destructive all the time? People aren’t destructive or against you\, you are simply the only one who cares about certain things.

Just to put in a statement from the side: Besides the style of discussion *I* care for the speed meditations that Reini regularly says.

It's very fair to negotiate communication style but don't forget the content over it.

Speed matters\, that's a fact. Not allowing discussion before Reini comes up with the silver bullet isn't fair either.

In this very case\, and unless I misunderstood\, it was not about speed but about keeping or not constant.pm compatible with Perl 5.6. Personally\, I am all in favor of not breaking backcompat as long as it doesn't require unreasonable efforts. Most of the work that the Perl 5 Porters did on constant.pm in the past year has been to correctly handle Unicode names\, which AFAICT really requires Perl 5.8. And in any case\, the previous versions are available on the CPAN.

-- Sébastien Aperghis-Tramoni

Close the world\, txEn eht nepO.

p5pRT commented 12 years ago

From @doy

On Tue\, Sep 11\, 2012 at 10:00:19AM +0100\, Steffen Schwigon wrote:

Aristotle Pagaltzis \pagaltzis@gmx\.de writes:

* Reini Urban \rurban@x\-ray\.at [2012-09-06 02:45]:

Why are you guys are so destructive all the time? People aren’t destructive or against you\, you are simply the only one who cares about certain things.

Just to put in a statement from the side: Besides the style of discussion *I* care for the speed meditations that Reini regularly says.

It's very fair to negotiate communication style but don't forget the content over it.

Speed matters\, that's a fact. Not allowing discussion before Reini comes up with the silver bullet isn't fair either.

I fail to see where anyone here is not allowing discussion. In fact\, we seem to have discussions about this pretty regularly(:

-doy

p5pRT commented 12 years ago

From @nwc10

On Tue\, Sep 11\, 2012 at 09:27:27AM -0500\, Jesse Luehrs wrote:

On Tue\, Sep 11\, 2012 at 10:00:19AM +0100\, Steffen Schwigon wrote:

Speed matters\, that's a fact. Not allowing discussion before Reini comes up with the silver bullet isn't fair either.

There is no silver bullet.

I think that it's accurate to say that everyone agrees that Perl 5 (as is) is too dynamic to optimise well.

Reini's broad thrust seems to be to take the approach of toning down the dynamism in order to apply conventional (static language) optimisation techniques.

That will get some speedup\, but without (at least) prototyping it\, it's not clear how much. Or how invasive the necessary language changes would be.

But it's certainly not a silver bullet. The silver bullet would be a way to speed up all (or most) existing code on CPAN\, unchanged.

I fail to see where anyone here is not allowing discussion. In fact\, we seem to have discussions about this pretty regularly(:

Agree. We seem to have a lot of discussions about this and quite a few things pretty regularly. If there seems to be any concern about "not allowing discussion"\, it's more that nearly all of these discussions are mere talking shops\, and go nowhere. It seems that many people have opinions\, but very few actually have anything more substantive to contribute. This becomes immensely frustrating to the people who could actually do something more substantive\, but seem to find all their time and enthusiasm consumed by dealing with e-mail.

It's not "no discussion". It's "please\, more code\, less code-free discussion"

Very much a reaction to "meetings\, the practical alternative to work".

Nicholas Clark

p5pRT commented 11 years ago

From @maddingue

Hello all\,

On Sep 5. 2012 at 22:13\, Sebastien Aperghis-Tramoni wrote via RT:

Attached are two patches for constant.pm: * constant.pm-1.24-remove-5.6-checks.diff Given constant.pm is no longer compatible with Perl 5.6 and previous\, the parts of the code that check for these versions can be removed.

* constant.pm-1.24-utf8-test.diff Needed so the tests pass on Perl 5.8.0 to 5.8.3

May I respectfully ask the status about these patches?

Thanks

-- Sébastien Aperghis-Tramoni

Close the world\, txEn eht nepO.

p5pRT commented 11 years ago

From @khwilliamson

On Wed Dec 05 14:37:29 2012\, Maddingue_ wrote:

Hello all\,

On Sep 5. 2012 at 22:13\, Sebastien Aperghis-Tramoni wrote via RT:

Attached are two patches for constant.pm: * constant.pm-1.24-remove-5.6-checks.diff Given constant.pm is no longer compatible with Perl 5.6 and previous\, the parts of the code that check for these versions can be removed.

* constant.pm-1.24-utf8-test.diff Needed so the tests pass on Perl 5.8.0 to 5.8.3

May I respectfully ask the status about these patches?

Thanks

I just looked them over\, and the thread for this ticket. I applied the 2nd patch as commit 7d7d93ad5efb66d5764017a7a62c90f3b5353986 The first patch is too controversial for me to feel comfortable applying\, but this 2nd should get your tests to pass\, I think -- Karl Williamson

p5pRT commented 11 years ago

From @maddingue

Karl Williamson wrote via RT:

I just looked them over\, and the thread for this ticket. I applied the 2nd patch as commit 7d7d93ad5efb66d5764017a7a62c90f3b5353986 The first patch is too controversial for me to feel comfortable applying\, but this 2nd should get your tests to pass\, I think

Thank you.

-- Sébastien Aperghis-Tramoni

Close the world\, txEn eht nepO.

p5pRT commented 11 years ago

From @jkeenan

On Wed Dec 05 16:22:03 2012\, khw wrote:

On Wed Dec 05 14:37:29 2012\, Maddingue_ wrote:

Hello all\,

On Sep 5. 2012 at 22:13\, Sebastien Aperghis-Tramoni wrote via RT:

I just looked them over\, and the thread for this ticket. I applied the 2nd patch as commit 7d7d93ad5efb66d5764017a7a62c90f3b5353986 The first patch is too controversial for me to feel comfortable applying\, but this 2nd should get your tests to pass\, I think

If I read Sebastien's original post correctly\, this is the not-yet-applied portion of the patch:

#####

Inline Patch

```diff --- blead/dist/constant/lib/constant.pm 2012-09-04 11:15:26.000000000 +0200 +++ blead/dist/constant/lib/constant.pm 2012-09-05 21:25:14.000000000 +0200 @@ -1,5 +1,5 @@ ```

package constant; -use 5.005; +use 5.008; use strict; use warnings::register;

@@ -17\,10 +17\,9 @@

my %forbidden = (%keywords\, %forced_into_main);

-my $str_end = $] >= 5.006 ? "\\z" : "\\Z"; -my $normal_constant_name = qr/^_?[^\W_0-9]\w*$str_end/; -my $tolerable = qr/^[A-Za-z_]\w*$str_end/; -my $boolean = qr/^[01]?$str_end/; +my $normal_constant_name = qr/^_?[^\W_0-9]\w*\z/; +my $tolerable = qr/^[A-Za-z_]\w*\z/; +my $boolean = qr/^[01]?\z/;

BEGIN { # We'd like to do use constant _CAN_PCS => $] > 5.009002

Inline Patch

```diff --- blead/dist/constant/t/constant.t 2009-12-28 13:04:37.000000000 +0100 +++ blead/dist/constant/t/constant.t 2012-09-04 11:17:30.000000000 +0200 @@ -167,7 +167,6 @@ ```

@warnings = (); eval q{ no warnings; - #local $^W if $] \< 5.006; use warnings 'constant'; use constant 'BEGIN' => 1 ; use constant 'INIT' => 1 ;

##### Can we have some temperate discussion of whether we want to go forward with this or not?

Thank you very much. Jim Keenan

p5pRT commented 11 years ago

From @bulk88

On Mon Jan 14 16:24:44 2013\, jkeenan wrote:

Can we have some temperate discussion of whether we want to go forward with this or not?

Thank you very much. Jim Keenan

Probably not :-(

What is the technical question regarding the changes? I ask\, what used to be a valid name\, what is the proposed valid name?

To support old Perls\, I personally use BEGIN block constants and constant folding. No runtime impact then. I wouldn't say 5.6 is impossible old\, someone asked for 5.6 support on a CPAN module a few weeks ago and 5.6 support was added easily to that module.

-- bulk88 ~ bulk88 at hotmail.com

p5pRT commented 11 years ago

From @iabyn

On Mon\, Jan 14\, 2013 at 04:24:45PM -0800\, James E Keenan via RT wrote:

Can we have some temperate discussion of whether we want to go forward with this or not?

My understanding is that previous change(s) to constant.pm broke backwards compatibility with 5.6.x\, and this change formalises it by removing the now "useless" remaining 5.6 backcompat code.

The other alternative is to restore 5.6.x compatibility. However\, since no-one has come forward with such a fix\, I suggest we go ahead and apply the change. If at some point someone wants comes up with a fix\, they can always re-add the backcomp code too.

Bear in mind that we're talking about support for a 10-year old version of perl that already comes with its own copy of constant.pm\, and there are older versions of constant.pm in CPAN if people really need it.

-- Art is anything that has a label (especially if the label is "untitled 1")

p5pRT commented 11 years ago

From @maddingue

Dave Mitchell wrote:

On Mon\, Jan 14\, 2013 at 04:24:45PM -0800\, James E Keenan via RT wrote:

Can we have some temperate discussion of whether we want to go forward with this or not?

My understanding is that previous change(s) to constant.pm broke backwards compatibility with 5.6.x\, and this change formalises it by removing the now "useless" remaining 5.6 backcompat code.

Yes\, it is exactly this. Knowing that porters' time is limited\, and given that all the recent changes to constant.pm were to improve its Unicode support\, I thought it was better to remove what was left of 5.6-isms.

The other alternative is to restore 5.6.x compatibility. However\, since no-one has come forward with such a fix\, I suggest we go ahead and apply the change. If at some point someone wants comes up with a fix\, they can always re-add the backcomp code too.

I remember trying to fix the module for 5.6\, but was unable to do so\, for the reasons previously exposed.

Bear in mind that we're talking about support for a 10-year old version of perl that already comes with its own copy of constant.pm\, and there are older versions of constant.pm in CPAN if people really need it.

And the need to upgrade from their perfectly working constant.pm included with their distribution is quite low.

-- Sébastien Aperghis-Tramoni

Close the world\, txEn eht nepO.

p5pRT commented 11 years ago

From @maddingue

Dave Mitchell wrote:

On Mon\, Jan 14\, 2013 at 04:24:45PM -0800\, James E Keenan via RT wrote:

Can we have some temperate discussion of whether we want to go forward with this or not?

My understanding is that previous change(s) to constant.pm broke backwards compatibility with 5.6.x\, and this change formalises it by removing the now "useless" remaining 5.6 backcompat code.

Yes\, it is exactly this. Knowing that porters' time is limited\, and given that all the recent changes to constant.pm were to improve its Unicode support\, I thought it was better to remove what was left of 5.6-isms.

The other alternative is to restore 5.6.x compatibility. However\, since no-one has come forward with such a fix\, I suggest we go ahead and apply the change. If at some point someone wants comes up with a fix\, they can always re-add the backcomp code too.

I remember trying to fix the module for 5.6\, but was unable to do so\, for the reasons previously exposed.

Bear in mind that we're talking about support for a 10-year old version of perl that already comes with its own copy of constant.pm\, and there are older versions of constant.pm in CPAN if people really need it.

And the need to upgrade from their perfectly working constant.pm included with their distribution is quite low.

-- Sébastien Aperghis-Tramoni

Close the world\, txEn eht nepO.

p5pRT commented 11 years ago

From @jkeenan

On Fri Jan 18 12:11:03 2013\, Maddingue_ wrote:

Dave Mitchell wrote:

On Mon\, Jan 14\, 2013 at 04:24:45PM -0800\, James E Keenan via RT wrote:

Can we have some temperate discussion of whether we want to go forward with this or not?

My understanding is that previous change(s) to constant.pm broke backwards compatibility with 5.6.x\, and this change formalises it by removing the now "useless" remaining 5.6 backcompat code.

Yes\, it is exactly this. Knowing that porters' time is limited\, and given that all the recent changes to constant.pm were to improve its Unicode support\, I thought it was better to remove what was left of 5.6-isms.

The other alternative is to restore 5.6.x compatibility. However\, since no-one has come forward with such a fix\, I suggest we go ahead and apply the change. If at some point someone wants comes up with a fix\, they can always re-add the backcomp code too.

I remember trying to fix the module for 5.6\, but was unable to do so\, for the reasons previously exposed.

Bear in mind that we're talking about support for a 10-year old version of perl that already comes with its own copy of constant.pm\, and there are older versions of constant.pm in CPAN if people really need it.

And the need to upgrade from their perfectly working constant.pm included with their distribution is quite low.

I extracted the remaining patch submission from Maddingue's post\, bumped the version number and applied all as commit eb10a876.

Closing ticket.

Thank you very much. Jim Keenan

p5pRT commented 11 years ago

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

Perl / perl5