[Bug 10639] New: Circular dependencies cause inventory.pl to fail when returning items
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10639 Bug ID: 10639 Summary: Circular dependencies cause inventory.pl to fail when returning items Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: major Priority: P5 - low Component: Circulation Assignee: koha-bugs@lists.koha-community.org Reporter: dcook@prosentient.com.au QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com, kyle.m.hall@gmail.com Currently, when you're running an inventory, inventory.pl will die if any of the items need to be returned (i.e. Koha says they're checked out, but they were found on the shelf, so we do an "AddReturn"). After chatting to Chris Cormack and Jared Camins-Esakov on IRC, it appears the cause is due to "circular dependencies". That is, C4::Circulation::AddReturn is failing, because Perl thinks certain subs used in AddReturn from C4::Items actually belong to C4::Circulation (or rather it's just plain confused about where they're supposed to come from). This is because C4::Items and C4::Circulation depend on each other. C4::Circulation directly loads C4::Items, while C4::Items loads C4::Search which loads C4::Reserves which loads C4::Circulation. So...when inventory.pl loads C4::Items and C4::Circulation...we run into problems calling a sub from C4::Items within C4::Circulation, because there are conflicts/collisions in the namespace for subs from C4::Items (i.e. they've been loaded twice, so Perl has a hissy fit). Chris mentioned that the model for new Koha modules will be to have no circular dependencies, but for now...I'm thinking that we fully qualify calls to imported subs... -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10639 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution|--- |INVALID --- Comment #1 from David Cook <dcook@prosentient.com.au> --- Nevermind...I thought that I had tested this in the master branch, but apparently I had not. Apparently, I had actually just eyeballed the master code that I expected to be different (in Circulation.pm) and seeing it was the same...decided that was enough. In fact, Jared Camins-Esakov removed this particular circular dependency back in 3.08.01 with Bug 7847: OPAC search dies with Plackthis (commit c97e0c76739b0624359feb47ee0573ebc46b8409). In order to do so, he removed the "use C4::Search" declaration at the top of C4::Items and instead used "require C4::Search" in the subs that needed it, and then used fully qualified sub names (since they are required when using "require" rather than "use"). Yay, Jared! -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10639 --- Comment #2 from David Cook <dcook@prosentient.com.au> --- That said, I think this is something that developers should keep in mind going forward... Chris Cormack said that the ideal method should be as follows: 1) Export subs using the EXPORT_OK array 2) Import subs using "use C4::Module qw (sub1 sub2)" Jared Camins-Esakov also mentioned: 1) In addition to the above, ff there is going to be any confusion, fully qualify the name (e.g. Add the C4::Module:: notation to the front of your imported sub when you're calling it). 2) "A require and fully-qualified call will at least make it easy to recognize problems." My two cents: I wonder if we should decide on the best method and put it in the Coding Guidelines? Should we always fully qualify subroutine names? -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org