Bug 15182 - make test fails immediately on 3.22.00-beta due to missing Crypt::GCrypt which is optional
Summary: make test fails immediately on 3.22.00-beta due to missing Crypt::GCrypt whic...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Installation and upgrade (command-line installer) (show other bugs)
Version: unspecified
Hardware: PC Linux
: P5 - low blocker (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 15151
Blocks:
  Show dependency treegraph
 
Reported: 2015-11-12 17:09 UTC by Bob Ewart
Modified: 2017-06-14 22:08 UTC (History)
11 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
make test output (2.67 KB, text/plain)
2015-11-12 17:09 UTC, Bob Ewart
Details
Bug 15182: Continue to consider Crypt::GCrypt optional (1.51 KB, patch)
2015-11-13 14:39 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 15182: Continue to consider Crypt::GCrypt optional (1.65 KB, patch)
2015-11-13 14:47 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
Bug 15182 - [alternate] - Conditionally load Koha::NorwegianPatronDB (1.59 KB, patch)
2015-11-17 01:43 UTC, David Cook
Details | Diff | Splinter Review
[SIGNED-OFF]Bug 15182 - [alternate] - Conditionally load Koha::NorwegianPatronDB (1.70 KB, patch)
2015-11-17 03:37 UTC, Héctor Eduardo Castro Avalos
Details | Diff | Splinter Review
[PASSED QA] Bug 15182 - [alternate] - Conditionally load Koha::NorwegianPatronDB (1.76 KB, patch)
2015-11-17 11:51 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Bob Ewart 2015-11-12 17:09:17 UTC
Created attachment 44813 [details]
make test output

Running on openSUSE Leap 42.1

Run
perl Makefile.PL --prev-install-log /usr/share/koha/misc/koha-install-log
make
make test

make test fails on t/00-load.t  while trying to use Crypt::GCrypt in NorwegianPatronDB.pm 

See the attached file.

Crypt::GCrypt is not marked as required and in fact does not work with libgcrypt 1.6.1.  This was reported in 2014.

Deleting libgcrypt 1.6.1 would cause 794 other modules including systemd to be deleted.
Comment 1 Héctor Eduardo Castro Avalos 2015-11-12 19:00:05 UTC
This is launched becuase C4::Accounts depends from C4::Members and C4::Members depends on Koha::NorwegianPatronDB.

All modules required by "Norwegian national library card" are not required:

    'SOAP::Lite' => {
        'usage'    => 'Norwegian national library card',
        'required' => '0',
        'min_ver'  => '0.712',
    },
    'Crypt::GCrypt' => {
        'usage'    => 'Norwegian national library card',
        'required' => '0',
        'min_ver'  => '1.24',
    },
    'Convert::BaseN' => {
        'usage'    => 'Norwegian national library card',
        'required' => '0',
        'min_ver'  => '0.01',
    },
    'Digest::SHA' => {
        'usage'    => 'Norwegian national library card',
        'required' => '0',
        'min_ver'  => '5.61',
    },

In debian this is not a problem, since package in available sudo apt-get install libcrypt-gcrypt-perl.

I add here Tomás (RM)
Comment 2 Tomás Cohen Arazi 2015-11-13 02:44:27 UTC
Bob, I've tested this on Jessie and I cannot reproduce. So it seems your openSuSE box doesn't have all the dependencies installed in order to run make test.

Can you just install it?
Comment 3 Jonathan Druart 2015-11-13 14:27:29 UTC
Caused by commit f25fe6ddb4a340f12613784dc841ab5bfd672d6b
Date:   Fri Nov 6 11:21:56 2015 -0300
    Bug 15151: Avoid DB access to load C4::Members
Comment 4 Jonathan Druart 2015-11-13 14:39:09 UTC Comment hidden (obsolete)
Comment 5 Héctor Eduardo Castro Avalos 2015-11-13 14:47:20 UTC Comment hidden (obsolete)
Comment 6 Héctor Eduardo Castro Avalos 2015-11-13 15:05:35 UTC
Without this patch and Crypt::GCrypt, Patron module launches an error.
Comment 7 Bob Ewart 2015-11-13 15:58:39 UTC
In answer to Tomas

I installed it and opened the intranet login.  It immediately crashed because  NorwegianPatronDB couldn't find Crypt::GCrypt

The patch worked for me

Thanks
Comment 8 Katrin Fischer 2015-11-15 22:23:55 UTC
This patch breaks the staff start page for me?

Can't locate object method "load" via package "Koha::NorwegianPatronDB" at /home/katrin/kohaclone/C4/Members.pm line 46.
Compilation failed in require at /home/katrin/kohaclone/C4/Letters.pm line 27.
BEGIN failed--compilation aborted at /home/katrin/kohaclone/C4/Letters.pm line 27.
Compilation failed in require at /home/katrin/kohaclone/C4/Suggestions.pm line 29.
BEGIN failed--compilation aborted at /home/katrin/kohaclone/C4/Suggestions.pm line 29.
Compilation failed in require at /home/katrin/kohaclone/mainpage.pl line 29.
BEGIN failed--compilation aborted at /home/katrin/kohaclone/mainpage.pl line 29.

All works well without the patch.
Comment 9 David Cook 2015-11-17 01:14:14 UTC
Hi all:

I noticed that the author of Crypt::GCrypt is active on Github, so I reported the errors there: https://github.com/alexrj/Crypt-GCrypt/issues/2. Hopefully it's just a matter of him being unaware of the issue reported on CPAN back in 2014 and he'll move to fix the problem.

Tomas && Hector: the reason it works on Debian is that the Debian team have patched the original Crypt::GCrypt module. You can see that here: https://anonscm.debian.org/cgit/pkg-perl/packages/libcrypt-gcrypt-perl.git/log/

Bob: I use openSUSE 13.2, so I just ran into this problem myself when trying to load master. Hopefully alexrj will fix the problem upstream and then we can download a working module. I was unable to install Crypt::GCrypt from CPAN as it was failing too many tests.

It seems to me that without an updated Crypt::GCrypt or without this patch working, no one will be able to use Koha 3.22+ except on Debian-based systems...
Comment 10 David Cook 2015-11-17 01:18:33 UTC
Comment on attachment 44844 [details] [review]
[SIGNED-OFF]Bug 15182: Continue to consider Crypt::GCrypt optional

Review of attachment 44844 [details] [review]:
-----------------------------------------------------------------

::: C4/Members.pm
@@ +42,5 @@
>  use Koha::Database;
> +
> +use Module::Load::Conditional qw( can_load );
> +if ( can_load( modules => { 'Koha::NorwegianPatronDB' => undef } ) ) {
> +    load Koha::NorwegianPatronDB, qw( NLUpdateHashedPIN NLEncryptPIN NLSync );

Perhaps "load" should be replaced with "Module::Load::load".

I don't know why it's working for others, but I'm guessing that will fix it for her...
Comment 11 David Cook 2015-11-17 01:28:34 UTC
Module::Load and Module::Load::Conditional are both core modules, so I don't know why Katrin was getting that error... I'm guessing that "load" is exported in Module::Load::Conditional but that it doesn't get exported into Members.pm via Module::Load::Conditional.

But that doesn't really matter. As Chris Cormack pointed out in IRC, "can_load" already loads the module. There's no point trying to do it twice.

You also don't need to import NLUpdateHashedPIN, NLEncryptPIN, and NLSync into the main namespace, as they're being referred to via the Koha::NorwegianPatronDB namespace.

What you could do is this:

if ( ! can_load( modules => { 'Koha::NorwegianPatronDB' => undef } ) ) {
   warn "Unable to load Koha::NorwegianPatronDB";
}

instead of 

require Koha::NorwegianPatronDB;
Comment 12 David Cook 2015-11-17 01:43:42 UTC Comment hidden (obsolete)
Comment 13 David Cook 2015-11-17 01:45:56 UTC
Changing this back to "Needs Signoff".

I just realized I forgot a test plan...

_TEST PLAN_

After applying patch:

Debian Users:
1) Check to make sure libcrypt-gcrypt-perl is installed
2) visit mainpage.pl
3) Confirm that the page loads correctly

4) Uninstall libcrypt-gcrypt-perl
5) visit mainpage.pl
6) Confirm that the page loads correctly

7) Reinstall libcrypt-gcrypt-perl as you probably want to keep it

Non-Debian Users:
1) visit mainpage.pl
2) Confirm that the page loads correctly
3) Profit
Comment 14 Bob Ewart 2015-11-17 03:04:37 UTC
Crypt::GCrypt can be made to work on openSUSE.

Download the source for libgcrypt 1.5.4 from ftp://ftp.gnupg.org/gcrypt/libgcrypt/

It will install into /usr/local/lib64 where cpan doesn't find it.

cp /usr/local/lib64/libgcrypt.so.11 /usr/lib64

Then cpan install Crypt::GCrypt will work

It would be best if alexrj would update Crypt::GCrypt with the debian patch.  

Otherwise I've been trying to update INSTALL.opensuse which is sadly out of date (it's for openSUSE 11.3).  It's a moving target.
Comment 15 Héctor Eduardo Castro Avalos 2015-11-17 03:20:27 UTC
In debian Jessie if you uninstall $ dpkg -r libcrypt-gcrypt-perl

An error is launched:
Software error:

Can't locate Crypt/GCrypt.pm in @INC (you may need to install the Crypt::GCrypt module) (@INC contains: /var/koha/koha /var/koha/koha-perl/lib/perl5 /etc/perl /usr/local/lib/i386-linux-gnu/perl/5.20.2 /usr/local/share/perl/5.20.2 /usr/lib/i386-linux-gnu/perl5/5.20 /usr/share/perl5 /usr/lib/i386-linux-gnu/perl/5.20 /usr/share/perl/5.20 /usr/local/lib/site_perl .) at /var/koha/koha/Koha/NorwegianPatronDB.pm line 58.
BEGIN failed--compilation aborted at /var/koha/koha/Koha/NorwegianPatronDB.pm line 58.
Compilation failed in require at /var/koha/koha/C4/Members.pm line 43.
Compilation failed in require at /var/koha/koha/C4/Letters.pm line 27.
BEGIN failed--compilation aborted at /var/koha/koha/C4/Letters.pm line 27.
Compilation failed in require at /var/koha/koha/C4/Suggestions.pm line 29.
BEGIN failed--compilation aborted at /var/koha/koha/C4/Suggestions.pm line 29.
Compilation failed in require at /var/koha/koha/mainpage.pl line 29.
BEGIN failed--compilation aborted at /var/koha/koha/mainpage.pl line 29.
Comment 16 Héctor Eduardo Castro Avalos 2015-11-17 03:27:59 UTC
The patch does not apply, last patch needs to be obsolete.
Comment 17 Héctor Eduardo Castro Avalos 2015-11-17 03:37:51 UTC Comment hidden (obsolete)
Comment 18 David Cook 2015-11-17 03:59:04 UTC
(In reply to Bob Ewart from comment #14)
> Crypt::GCrypt can be made to work on openSUSE.
> 
> Download the source for libgcrypt 1.5.4 from
> ftp://ftp.gnupg.org/gcrypt/libgcrypt/
> 
> It will install into /usr/local/lib64 where cpan doesn't find it.
> 
> cp /usr/local/lib64/libgcrypt.so.11 /usr/lib64
> 
> Then cpan install Crypt::GCrypt will work
> 
> It would be best if alexrj would update Crypt::GCrypt with the debian patch.
> 
> 
> Otherwise I've been trying to update INSTALL.opensuse which is sadly out of
> date (it's for openSUSE 11.3).  It's a moving target.

Interesting... yeah I already have libgcrypt 1.6.1 installed. I don't know if I'd really want to have two installs alongside each other...

Good on you for looking at INSTALL.opensuse, although if you look at the listserv, there's talk about alternatives to INSTALL.debian, so you might want to follow what's being down with the INSTALL files.
Comment 19 David Cook 2015-11-17 04:00:01 UTC
(In reply to Héctor Eduardo Castro Avalos from comment #16)
> The patch does not apply, last patch needs to be obsolete.

You can use git bz interactively to only apply the patches you want to apply :)
Comment 20 Héctor Eduardo Castro Avalos 2015-11-17 04:04:07 UTC
(In reply to David Cook from comment #19)

> You can use git bz interactively to only apply the patches you want to apply
> :)

Thanks for the tip, David
Comment 21 Katrin Fischer 2015-11-17 11:51:48 UTC
Created attachment 44906 [details] [review]
[PASSED QA] Bug 15182 - [alternate] - Conditionally load Koha::NorwegianPatronDB

This patch loads Koha::NorwegianPatronDB if it can, or adds a warning
to the log if it can't load it.

Since the Koha::NorwegianPatronDB functionality in C4::Members is
wrapped in system preferences, the loading of the module is
irrelevant unless one actually wants to use the module and its
associated functionality.

NOTE:

This patch fixes a problem where we were getting errors saying
Crypt::GCrypt couldn't be loaded even though it's not a required
dependency.

This patch will likely only affect people not using Debian-based
systems where libcrypt-gcrypt-perl is available. The current
version of Crypt::GCrypt doesn't build so it's not an option
for most non-Debian users to install this not required
dependency in order to work around this issue...

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as advertised for Debian-based systems

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 22 Tomás Cohen Arazi 2015-11-17 12:56:28 UTC
Patch pushed to master.

Thanks David!
Comment 23 Frédéric Demians 2015-11-29 21:07:29 UTC
This patch has been pushed to 3.20.x, will be in 3.20.6.
Comment 24 David Cook 2015-11-29 22:16:16 UTC
Just took a look at https://github.com/alexrj and it seems that the author of Crypt::GCrypt is ignoring the issue I raised as there's no response but he's still pushing commits to other repositories...
Comment 25 Liz Rea 2016-01-06 21:04:14 UTC
Pushed to 3.18.13, and released.
Comment 26 Federico Leva 2016-02-01 18:30:27 UTC
The fix is good, with koha-3.22.01 in Fedora 23 I didn't install the non-required dependencies mentioned above and I confirm everything works. However the logs are a bit spammy, which is a bit confusing if loading NorwegianPatronDB isn't required:

 Members.pm: Unable to load Koha::NorwegianPatronDB at /usr/share/perl5/vendor_perl/C4/Members.pm line 46.: /usr/share/koha/intranet/cgi-bin/tools/manage-marc-import.pl, referer: http://46.101.193.160/cgi-bin/koha/tools/stage-marc-import.pl

There is a perl-Crypt-GCrypt-1.26-1 package in fedora 24 repos, 2 others work with cpanm and another is installed by default. HTH 
https://github.com/nemobis/beic-koha/commit/cae9809897377f36218d34c2595e7637e8a3bfd6