Bug 28617 - misc/kohalib.pl no longer useful
Summary: misc/kohalib.pl no longer useful
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-06-23 09:45 UTC by Jonathan Druart
Modified: 2022-12-12 21:23 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
This enhancement removes the script misc/kohalib.pl. The purpose of this script was to load the relevant Koha lib for the different scripts (installation, cronjob, CLI, etc.). However, it is not used consistently and we prefer to rely on PERL5LIB. If upgrading ancient Koha systems from tarballs double-check that PERL5LIB is set in crontab.
Version(s) released in:
22.05.00


Attachments
Bug 28617: Remove kohalib.pl and rely on PERL5LIB (37.81 KB, patch)
2021-07-19 09:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 28617: Remove kohalib.pl and rely on PERL5LIB (37.78 KB, patch)
2021-11-22 09:55 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28617: (QA follow-up) Restore FindBin in two places (1.17 KB, patch)
2021-11-22 09:55 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 28617: Remove kohalib.pl and rely on PERL5LIB (37.81 KB, patch)
2021-11-29 10:32 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 28617: (QA follow-up) Restore FindBin in two places (1.19 KB, patch)
2021-11-29 10:32 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2021-06-23 09:45:44 UTC
This script is used by command line scripts to load the Koha lib.
Should not we rely on PERL5LIB instead?
Comment 1 Jonathan Druart 2021-06-23 09:50:28 UTC
This is not an enforced rule for all our CLI scripts, if it was really useful for someone we would have heard about them.
Comment 2 Martin Renvoize 2021-06-23 10:34:16 UTC
Must admit, I've never entirely understood what this script was all about and why we didn't set PERL5LIB somewhere instead.. I'm not sure recent scripts have included it and I've not come across issues using those (either as standard, dev or package installs).

Perhaps ask Galen if he knows of a reason to keep it?
Comment 3 Marcel de Rooy 2021-06-23 11:09:43 UTC
We should remove it and replace all occurrences like:
 eval { require "$FindBin::Bin/../kohalib.pl" };
Comment 4 Tomás Cohen Arazi 2021-06-23 11:44:11 UTC
I think the idea was to prevent some lib being replaced by a malicious one on a different path. Pretty much like the . fix Perl added.
Comment 5 David Cook 2021-06-23 23:54:11 UTC
(In reply to Jonathan Druart from comment #0)
> This script is used by command line scripts to load the Koha lib.
> Should not we rely on PERL5LIB instead?

+1
Comment 6 Galen Charlton 2021-07-16 20:04:01 UTC
If you're interested in the history, make yourself a hot beverage of your choice and read

http://koha.1045719.n5.nabble.com/RFC-installer-changes-td3065475.html

Time marches on, and one of the motivations for having kohalib.pl - making it possible to install Koha without setting a single environment variable - has been obviated by the vast improvements in the ease of installing Koha.

Consequently, I think kohalib.pl can go away.

As far as upgrade worries are concerned, a release note advising anybody who is still actively upgrading ancient Koha systems from tarballs that they should double-check that PERL5LIB is set in crontabs should suffice.
Comment 7 Jonathan Druart 2021-07-19 09:04:51 UTC
Created attachment 122935 [details] [review]
Bug 28617: Remove kohalib.pl and rely on PERL5LIB

The purpose of this script was to load the relevant Koha lib for the
different scripts (installation, cronjob, CLI, etc.)
However it is not used consistently and we prefer to rely on PERL5LIB.

From bug 28617 comment 6 from Galen:
"""
Time marches on, and one of the motivations for having kohalib.pl - making
it possible to install Koha without setting a single environment variable -
has been obviated by the vast improvements in the ease of installing Koha.

Consequently, I think kohalib.pl can go away.
"""

Test plan:
confirm that the changes make sense and that kohalib.pl can be removed
safely.
Comment 8 Jonathan Druart 2021-07-19 09:05:29 UTC
Thanks, Galen!
Comment 9 Marcel de Rooy 2021-11-22 09:55:52 UTC
Created attachment 127916 [details] [review]
Bug 28617: Remove kohalib.pl and rely on PERL5LIB

The purpose of this script was to load the relevant Koha lib for the
different scripts (installation, cronjob, CLI, etc.)
However it is not used consistently and we prefer to rely on PERL5LIB.

From bug 28617 comment 6 from Galen:
"""
Time marches on, and one of the motivations for having kohalib.pl - making
it possible to install Koha without setting a single environment variable -
has been obviated by the vast improvements in the ease of installing Koha.

Consequently, I think kohalib.pl can go away.
"""

Test plan:
confirm that the changes make sense and that kohalib.pl can be removed
safely.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 10 Marcel de Rooy 2021-11-22 09:55:56 UTC
Created attachment 127917 [details] [review]
Bug 28617: (QA follow-up) Restore FindBin in two places

Resolving:
    Name "FindBin::Bin" used only once: possible typo

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 11 Martin Renvoize 2021-11-29 10:32:47 UTC
Created attachment 128063 [details] [review]
Bug 28617: Remove kohalib.pl and rely on PERL5LIB

The purpose of this script was to load the relevant Koha lib for the
different scripts (installation, cronjob, CLI, etc.)
However it is not used consistently and we prefer to rely on PERL5LIB.

From bug 28617 comment 6 from Galen:
"""
Time marches on, and one of the motivations for having kohalib.pl - making
it possible to install Koha without setting a single environment variable -
has been obviated by the vast improvements in the ease of installing Koha.

Consequently, I think kohalib.pl can go away.
"""

Test plan:
confirm that the changes make sense and that kohalib.pl can be removed
safely.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 12 Martin Renvoize 2021-11-29 10:32:51 UTC
Created attachment 128064 [details] [review]
Bug 28617: (QA follow-up) Restore FindBin in two places

Resolving:
    Name "FindBin::Bin" used only once: possible typo

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 13 Martin Renvoize 2021-11-29 10:33:48 UTC
ALl works as expected, no additional cases found. QA scripts happy.

Passing QA
Comment 14 Fridolin Somers 2021-12-07 22:10:14 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄