Last Comment Bug 655477 - Require Perl 5.10.1
: Require Perl 5.10.1
Status: RESOLVED FIXED
:
Product: Bugzilla
Classification: Server Software
Component: Bugzilla-General (show other bugs)
: unspecified
: All All
: -- enhancement (vote)
: Bugzilla 5.0
Assigned To: Frédéric Buclin
: default-qa
Mentors:
Depends on: 668917
Blocks: 645427 645433 655912 662070 787529
  Show dependency treegraph
 
Reported: 2011-05-07 06:56 PDT by Frédéric Buclin
Modified: 2015-02-21 07:54 PST (History)
5 users (show)
LpSolit: approval+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
patch, v1 (12.02 KB, patch)
2011-08-08 03:52 PDT, Frédéric Buclin
mkanat: review+
Details | Diff | Splinter Review
patch, v1.1 (12.59 KB, patch)
2011-11-18 18:05 PST, Frédéric Buclin
LpSolit: review+
Details | Diff | Splinter Review

Description Frédéric Buclin 2011-05-07 06:56:16 PDT
In October 1999, Bugzilla 2.8 required Perl 5.4.
In August 2001, Bugzilla 2.16 required Perl 5.5 (bug 97721).
In October 2002, Bugzilla 2.18 required Perl 5.6 (bug 173495).
In March 2006, Bugzilla 3.0 required Perl 5.8 (bug 314470).
In August 2007, Bugzilla 3.2 required Perl 5.8.1 (bug 361149).
In 2012 or 2013, maybe Bugzilla 5 or 6 will require Perl 5.10.1?

There are some nice features in Perl 5.10 which would make our life easier to write code, see http://perldoc.perl.org/feature.html.

say "foo" instead of print "foo\n".
state $foo instead of having to use closures (my $foo; sub bar { $foo++ }).
given..when..default instead of if..elsif..elsif...else.
$foo = $bar // $baz instead of $foo = defined $bar ? $bar : $baz.
smart matching, see http://perldoc.perl.org/perlsyn.html#Smart-matching-in-detail

There are probably some other features which do not come to mind right now and which could be useful for Bugzilla. Maybe should we list them in this bug, and then we can decide if/when it worths the move to 5.10.1 (we should skip 5.10.0 as it had some major regressions).

I see nothing exciting in Perl 5.12 or Perl 5.14, so I doubt we would bump the min Perl version again in the near future.
Comment 1 Frédéric Buclin 2011-05-07 10:22:45 PDT
About //: we could also write $foo //= $bar, which is most of the time more accurate than $foo ||= $bar as we would correctly catch cases where $foo is already defined but set to 0 or "".

Also, there are more modules included in the core distribution of Perl 5.10.
Comment 2 Frédéric Buclin 2011-05-07 11:13:43 PDT
Perl 5.10.1 has ExtUtils::MakeMaker 6.55.

Also, 5.10.1 lets us call "use parent Foo" instead of "use base Foo", which seems the way to go since 5.10.1. I didn't check if it's faster when loading modules, though.
Comment 3 Max Kanat-Alexander 2011-05-08 20:51:11 PDT
I agree these are all great reasons to require 5.10.x.

Were there listed regressions in 5.10.0 that would actually affect us (beyond  throwing warnings in checksetup.pl)?

I agree that "use parent" is better if we can use it. Also, is autodie in 5.10? We should use that too as appropriate.
Comment 4 Frédéric Buclin 2011-05-09 04:08:38 PDT
(In reply to comment #3)
> Were there listed regressions in 5.10.0 that would actually affect us
> (beyond  throwing warnings in checksetup.pl)?

One bug which affects us directly in 5.10.0 is http://rt.perl.org/rt3/Public/Bug/Display.html?id=59998 which I reported to Perl developers:
"crypt() returns tainted data even when input strings are detainted"

This bug has been fixed in 5.10.1 (see also bug 455584 comment 1).


> Also, is autodie in 5.10?

Yes, it is, but only in 5.10.1, not in 5.10.0, see http://search.cpan.org/~dapm/perl-5.10.1/pod/perl5101delta.pod#New_Modules_and_Pragmata
Comment 5 Max Kanat-Alexander 2011-05-09 11:50:16 PDT
Okay, sweet. Then yes, I totally agree we should skip 5.10.0 and go to 5.10.1. We should do it for 5.0, agreed?
Comment 6 Frédéric Buclin 2011-05-09 12:00:01 PDT
Depends whether we will have a Bugzilla 4.4 first or not. If yes, then I'm fine requiring 5.10.1 in Bugzilla 5.0, else I fear some installations may not have Perl 5.10.1 or higher yet.
Comment 7 Max Kanat-Alexander 2011-05-09 12:06:30 PDT
Yeah, our only problem is seeing how many RHEL5 installs migrate to RHEL6. If there isn't a CentOS 6 available, then it doesn't seem likely that people will have migrated.
Comment 8 Frédéric Buclin 2011-05-09 17:49:59 PDT
See bugs depending on this one for other good reasons to require Perl 5.10.1.
Comment 9 Frédéric Buclin 2011-05-15 15:28:29 PDT
(Perl 5.14.0 has been released this week-end, marking the EOL of Perl 5.10.)
Comment 10 Frédéric Buclin 2011-05-15 15:36:04 PDT
(In reply to comment #7)
> If there isn't a CentOS 6 available

It seems it will be released later this month, per http://qaweb.dev.centos.org/qa/calendar/view/2011-5.
Comment 11 Frédéric Buclin 2011-08-08 03:52:20 PDT
Created attachment 551415 [details] [diff] [review]
patch, v1
Comment 12 Max Kanat-Alexander 2011-08-09 13:24:05 PDT
Comment on attachment 551415 [details] [diff] [review]
patch, v1

Review of attachment 551415 [details] [diff] [review]:
-----------------------------------------------------------------

This is so awesome! :-)

::: Bugzilla/DB.pm
@@ +1245,5 @@
>                         HandleError => \&_handle_error,
>                         TaintIn => 1,
> +                       # See https://rt.perl.org/rt3/Public/Bug/Display.html?id=30933
> +                       # for the reason to use NAME instead of NAME_lc (bug 253696).
> +                       FetchHashKeyName => 'NAME',

Oh man! Let's switch it to NAME_lc! :-) That's what we really want.

::: template/en/default/request/email.txt.tmpl
@@ +81,4 @@
>  [%- FILTER bullet = wrap(80) %]
>  
>  [% USE Bugzilla %]
> +[%-# .defined is necessary to avoid a taint issue, see bug 509794. %]

Oh lame, it still happens in 5.10.1? :-(
Comment 13 Max Kanat-Alexander 2011-08-09 13:24:38 PDT
I'd like to hold this off until landfill is ready to run trunk installs on 5.10.1 (or we move landfill to a RHEL6 machine).
Comment 14 Byron Jones ‹:glob› [PTO until 2016-10-10] 2011-08-09 21:29:04 PDT
(In reply to Max Kanat-Alexander from comment #12)
> > +                       # See https://rt.perl.org/rt3/Public/Bug/Display.html?id=30933
> > +                       # for the reason to use NAME instead of NAME_lc (bug 253696).
> > +                       FetchHashKeyName => 'NAME',
> 
> Oh man! Let's switch it to NAME_lc! :-) That's what we really want.

before we can do that, we need to ensure the dbi bug has been fixed.
Comment 15 Frédéric Buclin 2011-08-17 04:20:51 PDT
(In reply to Max Kanat-Alexander from comment #13)
> I'd like to hold this off until landfill is ready to run trunk installs on
> 5.10.1 (or we move landfill to a RHEL6 machine).

ETA?
Comment 16 Frédéric Buclin 2011-11-18 18:05:56 PST
Created attachment 575612 [details] [diff] [review]
patch, v1.1

Unbitrotten. I also removed our own say() in favor of the builtin one.
Comment 17 Frédéric Buclin 2012-03-29 14:10:39 PDT
I used \g in one of our regexps, but I realized that Perl 5.8.x doesn't support it:

"The \g and \k notations were introduced in Perl 5.10.0. Prior to that there were no named nor relative numbered capture groups."

This is a severe limitation, and this is another good reason to require Perl 5.10.1. I'm going to commit this patch once we branch for 4.4 (so Bugzilla 4.5 will require 5.10.1).
Comment 18 Jochen Wiedmann 2012-03-29 16:54:06 PDT
(In reply to Byron Jones ‹:glob› from comment #14)

> before we can do that, we need to ensure the dbi bug has been fixed.

DBI Bug? Any references? I used to be a DBI committer, perhaps I can reactivate
my status and help.

Jochen
Comment 19 Byron Jones ‹:glob› [PTO until 2016-10-10] 2012-03-29 19:22:07 PDT
(In reply to Jochen Wiedmann from comment #18)
> (In reply to Byron Jones ‹:glob› from comment #14)
> 
> > before we can do that, we need to ensure the dbi bug has been fixed.
> 
> DBI Bug? Any references?

https://rt.perl.org/rt3/Public/Bug/Display.html?id=30933
Comment 20 Frédéric Buclin 2012-08-30 15:22:46 PDT
The trunk is now open for 4.5/5.0.
Comment 21 Frédéric Buclin 2012-08-31 03:33:10 PDT
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/
modified checksetup.pl
modified Bugzilla/DB.pm
modified Bugzilla/Util.pm
modified Bugzilla/Install/CPAN.pm
modified Bugzilla/Install/Requirements.pm
modified docs/en/xml/installation.xml
modified docs/en/xml/modules.xml
modified t/001compile.t
modified template/en/default/email/flagmail.txt.tmpl
modified template/en/default/setup/strings.txt.pl
Committed revision 8374.
Comment 22 Frank Becker 2012-08-31 11:28:13 PDT
I get the following error:

Undefined subroutine &Bugzilla::Util::say called at Bugzilla/DB.pm line 702.
Undefined subroutine &Bugzilla::Util::say called at .../sanitycheck.cgi line 45.

after update to the actual trunk
Comment 23 Frédéric Buclin 2012-08-31 12:10:31 PDT
(In reply to Frank Becker from comment #22)
> Undefined subroutine &Bugzilla::Util::say called at .../sanitycheck.cgi line
> 45.

I can indeed reproduce with sanitycheck.cgi. This is weird, because I intentionally export say() from Bugzilla::Util, so I don't know why it cannot find it. I will add the :5.10 feature to all scripts, so that we don't have this problem again.
Comment 24 Frank Becker 2012-08-31 12:19:39 PDT
(In reply to Frédéric Buclin from comment #23)
> (In reply to Frank Becker from comment #22)
> > Undefined subroutine &Bugzilla::Util::say called at .../sanitycheck.cgi line
> > 45.
> 
> I can indeed reproduce with sanitycheck.cgi. This is weird, because I
> intentionally export say() from Bugzilla::Util, so I don't know why it
> cannot find it. I will add the :5.10 feature to all scripts, so that we
> don't have this problem again.

Yes I can find the export (line 19) but in version 1.126 the implementation was removed.
Comment 25 Frédéric Buclin 2015-02-21 07:54:22 PST
Added to relnotes for 5.0rc1.

Note You need to log in before you can comment on or make changes to this bug.