Build breaks due to recent changes in Koha::Logger. It fails because of absence of Log4Perl.conf
Steps to reproduce in KTD without packaging in the middle: $ rm /etc/koha/sites/kohadev/log4perl.conf $ kshell k$ prove t/00-load.t t/00-load.t .. 6/? # Failed test 'use C4::Breeding;' # at t/00-load.t line 46. # Tried to use 'C4::Breeding'. # Error: Cannot open /etc/koha/sites/kohadev/log4perl.conf (No such file or directory) at /usr/share/perl5/Log/Log4perl/Config/BaseConfigurator.pm line 51. # Compilation failed in require at /kohadevbox/koha/C4/Breeding.pm line 29. # BEGIN failed--compilation aborted at /kohadevbox/koha/C4/Breeding.pm line 29. # Compilation failed in require at t/00-load.t line 46. # BEGIN failed--compilation aborted at t/00-load.t line 46. Bailout called. Further testing stopped: ***** PROBLEMS LOADING FILE 'C4::Breeding' FAILED--Further testing stopped: ***** PROBLEMS LOADING FILE 'C4::Breeding' kohadev-koha@8c9d066462c7:/kohadevbox/koha$
Created attachment 105013 [details] [review] Bug 25527: Initialize the logger when required In an OO package, the logger initialization should happen in the constructor. This is not an OO package and the initialization is happening on loading it. This is a wrong behaviour and certainly breaks in environments where initialization cannot happen (package building, for example). There could be several options to solve this, as it is used in a single sub on this package, I opted for initializing on that sub. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 105014 [details] [review] Bug 25527: Initialize the logger in the constructor in ExternalContent This patch makes Koha::Logger initialization happen in the ->new method for the OverDrive handler. In the case of RecordedBooks, it doesn't look like it is used so I just removed it. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
I'm currently rebuilding my dev env to see if I broke anything. But it looks correct and should be tested :-D
Created attachment 105017 [details] [review] Bug 25527: Add logger to Koha::ExternalContent This patch makes Koha::Logger initialization happen in the ->new method for the Koha::ExternalContent-derived classes. In the case of RecordedBooks, it doesn't look like it is used at all. In the case of OverDrive, it will now use the Koha::ExternalContent exported logger accessor. I added tests for this addition to Koha::ExternalContent to the OverDrive tests. I also removed references to Test::DBIx::Class as it is not used at all. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha_ExternalContent_OverDrive.t => SUCCESS: Tests pass! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 105018 [details] [review] Bug 25527: Initialize the logger when required In an OO package, the logger initialization should happen in the constructor. This is not an OO package and the initialization is happening on loading it. This is a wrong behaviour and certainly breaks in environments where initialization cannot happen (package building, for example). There could be several options to solve this, as it is used in a single sub on this package, I opted for initializing on that sub. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Created attachment 105019 [details] [review] Bug 25527: Add logger to Koha::ExternalContent This patch makes Koha::Logger initialization happen in the ->new method for the Koha::ExternalContent-derived classes. In the case of RecordedBooks, it doesn't look like it is used at all. In the case of OverDrive, it will now use the Koha::ExternalContent exported logger accessor. I added tests for this addition to Koha::ExternalContent to the OverDrive tests. I also removed references to Test::DBIx::Class as it is not used at all. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha_ExternalContent_OverDrive.t => SUCCESS: Tests pass! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
It works! additional test: My package build fails but at a much later step and doesn't seems at all related to this.
> Bug 25527: Initialize the logger when required I should note that this patch revert a pref optimization (bug 14597 comment 17). But I don't think it's valid enough to block this.
Created attachment 105020 [details] [review] Bug 25527: Initialize the logger when required In an OO package, the logger initialization should happen in the constructor. This is not an OO package and the initialization is happening on loading it. This is a wrong behaviour and certainly breaks in environments where initialization cannot happen (package building, for example). There could be several options to solve this, as it is used in a single sub on this package, I opted for initializing on that sub. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 105021 [details] [review] Bug 25527: Add logger to Koha::ExternalContent This patch makes Koha::Logger initialization happen in the ->new method for the Koha::ExternalContent-derived classes. In the case of RecordedBooks, it doesn't look like it is used at all. In the case of OverDrive, it will now use the Koha::ExternalContent exported logger accessor. I added tests for this addition to Koha::ExternalContent to the OverDrive tests. I also removed references to Test::DBIx::Class as it is not used at all. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/Koha_ExternalContent_OverDrive.t => SUCCESS: Tests pass! Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Nice work everyone! Pushed to master for 20.05
missing dependency - not backported to 19.11.x