Bug 20104 - Update minimum version of Perl to 5.20 because of caller()
Summary: Update minimum version of Perl to 5.20 because of caller()
Status: CLOSED INVALID
Alias: None
Product: Koha
Classification: Unclassified
Component: Installation and upgrade (web-based installer) (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial
Assignee: Mark Tompsett
QA Contact: Testopia
URL:
Keywords:
Depends on: 12932
Blocks: 20594
  Show dependency treegraph
 
Reported: 2018-01-30 19:46 UTC by Mark Tompsett
Modified: 2020-11-12 14:59 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: String patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 20104: Update minimum version of Perl to 5.20 (2.07 KB, patch)
2018-01-30 19:58 UTC, Mark Tompsett
Details | Diff | Splinter Review
Bug 20104: Update minimum version of Perl to 5.20 (2.13 KB, patch)
2018-04-06 22:26 UTC, Roch D'Amour
Details | Diff | Splinter Review
Bug 20104: Update minimum version of Perl to 5.20 (2.20 KB, patch)
2018-04-07 10:29 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Tompsett 2018-01-30 19:46:41 UTC
The C4::Log::cronlogaction uses caller, which did not exist prior to 5.20. And since Xenial, Debian 8, and Debian 9 all have versions at least 5.20 (the current version of perl is 5.26), it makes sense to update the version.
Comment 1 Mark Tompsett 2018-01-30 19:58:28 UTC Comment hidden (obsolete)
Comment 2 Roch D'Amour 2018-04-06 22:26:20 UTC Comment hidden (obsolete)
Comment 3 Katrin Fischer 2018-04-07 10:29:26 UTC
Created attachment 73829 [details] [review]
Bug 20104: Update minimum version of Perl to 5.20

This patch not only updates the version check, but repurposes
the perlversion parameter, so as to not have to retranslate
templates again when the version number changes after this.

The changes are very self-evident.

Signed-off-by: Roch D'Amour <roch.damour@inlibro.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 4 Katrin Fischer 2018-04-07 10:29:58 UTC
Will also require a change to the release notes (scripts) I think as the minimal requirements are part of the text.
Comment 5 Jonathan Druart 2018-04-11 19:54:32 UTC
Pushed to master for 18.05, thanks to everybody involved!
Comment 6 David Cook 2018-04-17 01:34:53 UTC
(In reply to M. Tompsett from comment #0)
> The C4::Log::cronlogaction uses caller, which did not exist prior to 5.20.

That is false. You can find "caller" in RHEL 6.9 which is still using Perl 5.10.

And I just used it in Perl 5.18.
Comment 7 David Cook 2018-04-17 01:36:09 UTC
I just bumped into this change preventing me from working on Koha master on OpenSUSE 42.3, which was released 2017-07-26. It uses Perl 5.18.2, which has worked fine with Koha. 

Requiring 5.20 seems like an awfully big decision, which seems to have had no discussion in the community. I'm going to send out a listserv email to hear what others have to say.
Comment 8 David Cook 2018-04-17 02:36:10 UTC
As Mason pointed out to me on IRC, the perldoc site search results seems broken for older versions of Perl.

As you can see, caller has been around a very long time: https://perldoc.perl.org/5.8.8/functions/caller.html
Comment 9 Mark Tompsett 2018-04-17 05:29:57 UTC
Comment on attachment 73829 [details] [review]
Bug 20104: Update minimum version of Perl to 5.20

Review of attachment 73829 [details] [review]:
-----------------------------------------------------------------

::: installer/install.pl
@@ +93,4 @@
>      $template->param( 'checkmodule' => 1 )
>        ; # we start with the assumption that there are no problems and set this to 0 if there are
>  
> +    unless ( $] >= 5.020000 ) {    # Bug 20104

Then change this back to 5.010000 -- clearly the perldoc site needs improvement. I was just trying to make sure the version of perl needed was accurately reflected.

@@ +93,5 @@
>      $template->param( 'checkmodule' => 1 )
>        ; # we start with the assumption that there are no problems and set this to 0 if there are
>  
> +    unless ( $] >= 5.020000 ) {    # Bug 20104
> +        $template->param( problems => 1, perlversion => '5.20', checkmodule => 0 );

And this to 5.10

::: koha-tmpl/intranet-tmpl/prog/en/modules/installer/step1.tt
@@ +70,4 @@
>                          <p>I encountered some problems.</p>
>                          <ul>
>                              [% IF ( perlversion ) %]
> +                                <li>Your Perl version is out of date. Please upgrade to a newer version of Perl (at least version [% perlversion %]).</li>

Because this refactor is worth keeping, I believe.
Comment 10 Mark Tompsett 2018-04-17 13:33:32 UTC
(In reply to M. Tompsett from comment #9)
> Because this refactor is worth keeping, I believe.

I have put the reversion of Perl version in bug 20594. Thank you for correcting me, David.
Comment 11 Jonathan Druart 2018-04-19 15:42:56 UTC
Reverted.