require vs. use: Perl compilation errors in Storage modules

Bug #1835620 reported by Jason Stephenson
This bug affects 1 person
Affects Status Importance Assigned to Milestone

Bug Description

Evergreen Version: master (all current releases affected)
OpenSRF Version: irrelevant
PostgreSQL Version: irrelevant
Linux Distribution: All currently supported distributions, plus Debian Buster
Perl Versions: 5.20.2 (Debian Jessie), 5.22.1 (Ubuntu Xenial), 5.24.1 (Debian Stretch), 5.26.1 (Ubuntu Xenial), 5.28.1 (Debian Buster)

While testing Ben's branches for bug 1835458, I decided to check all of the Perl modules for errors. I ran the following commands to do this:

find /usr/local/share/perl/5.28.1/OpenSRF -name *.pm -exec perl -c {} \; >& opensrf.log
find /usr/local/share/perl/5.28.1/OpenILS -name *.pm -exec perl -c {} \; >& openils.log

Ignoring all of the "subroutine redefined" warnings caused by our circular references in both command outputs, the second command turned up the following errors:

Can't locate object method "has_a" via package "actor::user" (perhaps you forgot to load "actor::user"?) at /usr/local/share/perl/5.28.1/OpenILS/Application/Storage/ line 433.
Compilation failed in require at /usr/share/perl/5.28/ line 135.
 ...propagated at /usr/share/perl/5.28/ line 157.
BEGIN failed--compilation aborted at /usr/local/share/perl/5.28.1/OpenILS/Application/Storage/CDBI/ line 6.
Compilation failed in require at /usr/local/share/perl/5.28.1/OpenILS/Application/Storage/ line 9.
BEGIN failed--compilation aborted at /usr/local/share/perl/5.28.1/OpenILS/Application/Storage/ line 9.
Can't locate object method "register_method" via package "OpenILS::Application::Storage" at /usr/local/share/perl/5.28.1/OpenILS/Application/Storage/Driver/Pg/ line 19.
Compilation failed in require at /usr/local/share/perl/5.28.1/OpenILS/Application/Storage/Driver/ line 15.
BEGIN failed--compilation aborted at /usr/local/share/perl/5.28.1/OpenILS/Application/Storage/Driver/ line 15.

I then ran the same two commands, modulo the Perl version, on all of the currently supported distributions: Ubuntu Xenial and Bionic, Debian Jessie and Stretch. They all turned up more or less the same errors.

These errors appear to happen mainly with perl -c. It seems like they should, but do not, prevent the service frome starting.

Without getting into a lot of detail, it looks like we should "require" instead of "use" certain modules used by the two modules that have errors. I have already made a branch that does this, but I want to do more testing and to add Perl tests for the module before sharing it.

description: updated
Revision history for this message
Jason Stephenson (jstephenson) wrote :

Branch is working/user/dyrcona/lp1835620-require-not-use:;a=shortlog;h=refs/heads/user/dyrcona/lp1835620-require-not-use

It switches the submodules from being used to being required, adds comments in the code to give a partial explanation why, and updates the storage driver tests to test all the submodules.

All Perl tests pass, and I ran a good number of storage methods and everything seems to work.

We should probably add some storage live tests at some point.

Changed in evergreen:
assignee: Jason Stephenson (jstephenson) → nobody
milestone: none → 3.4-beta1
tags: added: pullrequest
Galen Charlton (gmc)
Changed in evergreen:
importance: Undecided → Low
status: New → Won't Fix
status: Won't Fix → Confirmed
milestone: 3.4-beta1 → 3.3.3
no longer affects: evergreen/3.3
Changed in evergreen:
assignee: nobody → Galen Charlton (gmc)
Revision history for this message
Galen Charlton (gmc) wrote :

Works for me. Sign-off branch is working/user/gmcharlt/lp1835620_signoff.

tags: added: signedoff
Changed in evergreen:
milestone: 3.3.3 → 3.3.4
Changed in evergreen:
milestone: 3.3.4 → 3.3.5
Revision history for this message
Jason Boyer (jboyer) wrote :

Looks good to me too, pushed to master..rel_3_1

Thanks Jason and Galen!

Changed in evergreen:
assignee: Galen Charlton (gmc) → nobody
status: Confirmed → Fix Committed
Changed in evergreen:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers