Description
Jared Camins-Esakov
2013-03-21 02:48:36 UTC
Created attachment 16570 [details] [review] Bug 9883 - Koha::Plugins::Base should not require koha-conf to load Test Plan: 1) Run t/db_dependent/Plugins.t 2) Apply patch 3) Re-run t/db_dependent/Plugins.t Created attachment 16597 [details] [review] Bug 9883 - Koha::Plugins::Base should not require koha-conf to load Test Plan: 1) Run t/db_dependent/Plugins.t 2) Apply patch 3) Re-run t/db_dependent/Plugins.t Signed-off-by: Magnus Enger <magnus@enger.priv.no> The tests pass both before and after the patch. I can see no ill effects on the functionality (tried uploading and accessing the Kitchen Sink plugin). Some notes about my testing: After applying the patch the tests still failed, after talking to Kyle I realized I should make sure that my conf is current. So I am rerunning Makefile.pl doing an upgrade. 3 new dependencies are listed: - Archive::Zip - could be installed with apt-get libarchive-zip-perl - Archive::Extract - no package? - Module::Bundled::Files - no package? Module::Bundled::Files seems to make the test fail, but according to the output of koha_perl_deps.pl -m it should not be required. Archive::Extract is not listed by the script at all, but only by Makefile.PL. It turns out I misunderstood the error message. The problem is not the modification to @INC (though that should definitely be removed if it is unnecessary) but rather the inclusion of C4::Auth. C4::Auth should be loaded with require, at runtime. Created attachment 16640 [details] [review] [PASSED QA] Bug 9883 - Koha::Plugins::Base should not require koha-conf to load Test Plan: 1) Run t/db_dependent/Plugins.t 2) Apply patch 3) Re-run t/db_dependent/Plugins.t Signed-off-by: Magnus Enger <magnus@enger.priv.no> The tests pass both before and after the patch. I can see no ill effects on the functionality (tried uploading and accessing the Kitchen Sink plugin). Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> After installing libmodule-bundled-files-perl from the debian.koha-community.org I ran through Makefile.PL updating my conf file successfully. Also all tests and QA script pass now. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Sorry, I should have been more clear in my last update. I would *also* like a follow-up adjusting the use C4::Auth to a require. The patch currently on the bug will go in as-is, but rather than adding yet another bug to Bugzilla, I figure it makes more sense to change the title and consolidate. Created attachment 16710 [details] [review] Bug 9883 - Koha::Plugins::* should not require koha-conf to load - Followup - Load C4::Auth at run time (In reply to comment #6) > Sorry, I should have been more clear in my last update. I would *also* like > a follow-up adjusting the use C4::Auth to a require. The patch currently on > the bug will go in as-is, but rather than adding yet another bug to > Bugzilla, I figure it makes more sense to change the title and consolidate. I'm having a bit of difficulty following your line of though. Is this followup what you were looking for? Created attachment 16727 [details] [review] Bug 9883 - Add missing parameter to koha-conf.xml causing Plugins.t to fail Anything in a BEGIN block gets run at compile time. I was thinking of using "require C4::Auth" right before C4::Auth routines are required. (In reply to comment #10) > Anything in a BEGIN block gets run at compile time. I was thinking of using > "require C4::Auth" right before C4::Auth routines are required. Thanks! I'll replace that followup with a more sensible one. Created attachment 16777 [details] [review] Bug 9883 - Koha::Plugins::* should not require koha-conf to load - Followup - Load C4::Auth at run time Created attachment 16782 [details] [review] Bug 9883 - Create plugins dir in Makefile Created attachment 16786 [details] [review] Bug 9883 - Koha::Plugins::Base should not require koha-conf to load Test Plan: 1) Run t/db_dependent/Plugins.t 2) Apply patch 3) Re-run t/db_dependent/Plugins.t Signed-off-by: Magnus Enger <magnus@enger.priv.no> The tests pass both before and after the patch. I can see no ill effects on the functionality (tried uploading and accessing the Kitchen Sink plugin). Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> After installing libmodule-bundled-files-perl from the debian.koha-community.org I ran through Makefile.PL updating my conf file successfully. Also all tests and QA script pass now. Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 16787 [details] [review] [PASSED QA] Bug 9883 - Add missing parameter to koha-conf.xml causing Plugins.t to fail Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Created attachment 16788 [details] [review] [PASSED QA] Bug 9883 - Koha::Plugins::* should not require koha-conf to load - Followup - Load C4::Auth at run time Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Unset KOHA_CONF variable and made sure t/00_load.t passed Also ran Makefile.PL doing an update on my existing installation and checked koha-conf.xml: <pluginsdir>/home/katrin/koha-dev/var/lib/plugins</pluginsdir> <enable_plugins>0</enable_plugins> This patch has been pushed to master. |