Bug 33018

It looks like these modules are still in use in Koha? 

$ grep -rn "LWP::Protocol" *
cpanfile:62:requires 'LWP::Protocol::https', '5.836';
debian/list-deps:30:    'LWP::Protocol::https' => 'liblwp-protocol-https-perl | libwww-perl (<<6.02), libio-socket-ssl-perl',

$ grep -rn "IO::Socket::IP" *
debian/list-deps:32:    'IO::Socket::IP'       => 'perl-modules (>= 5.20.0) | libio-socket-ip-perl',
misc/bin/connexion_import_daemon.pl:91:# use IO::Socket::IP;

$ grep -rn "CPAN::Meta" *
cpanfile:15:requires 'CPAN::Meta', '2.150006';
debian/list-deps:33:    'CPAN::Meta'           => 'libcpan-meta-perl (>= 2.150010) | perl-modules-5.28 | perl-modules-5.30 | perl-modules (>= 5.20.2)',

So the point is that we had to provide them in the Koha Debian repo, but now we don't have to do that anymore, and we can remove them from "special treatment"?

Created attachment 148910 [details] [review]
Bug 33018: debian package tidy-up

this patch removes unneeded package dependencies

to test:
 - apply patch
 - build and install koha-common package, note success

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Not tested, I trust Mason to the right thing.

Please reset to NSO if thorough testing is considered necessary.

(In reply to Magnus Enger from comment #2)
> So the point is that we had to provide them in the Koha Debian repo, but now
> we don't have to do that anymore, and we can remove them from "special
> treatment"?

I think this question should be answered before this issue is progressed.

Created attachment 150288 [details] [review]
Bug 33018: debian package tidy-up (2)

remove unrequired module

(In reply to Magnus Enger from comment #2)
> It looks like these modules are still in use in Koha? 
> 
> $ grep -rn "LWP::Protocol" *
> cpanfile:62:requires 'LWP::Protocol::https', '5.836';
> debian/list-deps:30:    'LWP::Protocol::https' =>
> 'liblwp-protocol-https-perl | libwww-perl (<<6.02), libio-socket-ssl-perl',

this pkg is required, but no longer needs to be explicitly listed in deb/list-deps

> 
> $ grep -rn "IO::Socket::IP" *
> debian/list-deps:32:    'IO::Socket::IP'       => 'perl-modules (>= 5.20.0)
> | libio-socket-ip-perl',
> misc/bin/connexion_import_daemon.pl:91:# use IO::Socket::IP;

this pkg is not required, as its now provided by perl-modules-* pkg

> 
> $ grep -rn "CPAN::Meta" *
> cpanfile:15:requires 'CPAN::Meta', '2.150006';
> debian/list-deps:33:    'CPAN::Meta'           => 'libcpan-meta-perl (>=
> 2.150010) | perl-modules-5.28 | perl-modules-5.30 | perl-modules (>=
> 5.20.2)',

this pkg is not required, as its now provided by perl-modules-* pkg

Thanks Mason, sounds good!

I would not remove CPAN::Meta from the cpanfile: 

  commit a34d086a24b1e4519f8b31331d4c42b87b8e4a83
  Bug 19735: Move Perl deps definitions into a cpanfile

    Added dependencies:
     - Module::CPANfile
     - CPAN::Meta (dependency of Module::CPANfile, but we need a more recent
       version than the one Module::CPANfile requires)

Mason, ping?

(In reply to Jonathan Druart from comment #9)
> I would not remove CPAN::Meta from the cpanfile: 

hi Jonathan, sorry for the late reply

sounds good to me, i'll obsolete the patch

I am not sure how to QA this one - can someone else take over please?

I notice there is still this line in the patch:
-    'CPAN::Meta'           => 'libcpan-meta-perl (>= 2.150010) | perl-modules-5.28 | perl-modules-5.30 | perl-modules (>= 5.20.2)',

(In reply to Magnus Enger from comment #3)
> Created attachment 148910 [details] [review] [review]
> Bug 33018: debian package tidy-up
> 
> this patch removes unneeded package dependencies
> 
> to test:
>  - apply patch
>  - build and install koha-common package, note success
> 
> Signed-off-by: Magnus Enger <magnus@libriotech.no>
> Not tested, I trust Mason to the right thing.

Sorry, but I am moving back status. Signoff means test. QA is reading too :)

Hi, I'd like to help here but I don't think I'm qualified to.
What is list-deps used for exactly, why did it need the now removed entries, why are they now being removed and how does list-deps relate to cpanfile.
Ignore the questions above if they're irrelevant in moving this bug.

Are we still keen to do this Mason?

Personally, I still don't get why we use debian/update-control, debian/list-deps, and debian/control.in and don't just manage debian/control by hand, but c'est la vie.

I've come up with a little test plan. You'll probably find "bash debian/update-control" takes a longish time to run. 

Test plan:
0. Apply the patch
1. apt-get update
2. apt-get install apt-file
3. apt-file update
4. vi debian/control
5. Find "Package: koha-common" and change "yaz" to "yazzz"
4. bash debian/update-control
5. Review the changes to debian/control using "git diff"

When reviewing the change...

1. When reviewing, there should be no new reference to libio-socket-ip-perl, since we don't have that in the cpanfile and IO::Socket::IP is already part of perl-base. So that's good. 

2. With this patch, libcpan-meta-perl now appears in debian/control without any reference to version number of "perl-modules", and this is potentially a problem... because CPAN::Meta 2.150010 is part of the perl core (via perl-modules-5.XX) since Perl 5.14 (AKA Debian 7 Wheezy). 

So... if we accidentally installed libcpan-meta-perl it's probably not a big deal, because it's the same version as the core version. One of them would appear first when searching @INC and it shouldn't really matter. But... we really shouldn't be installing a package (that we host on debian.koha-community.org) if we don't need it. 

3. "liblwp-protocol-https-perl" appears instead of "liblwp-protocol-https-perl | libwww-perl (<<6.02), libio-socket-ssl-perl," 

liblwp-protocol-https-perl is in buster, bullseye, bookworm, etc and we don't have it in the debian.koha-community.org repo. I think this is fine.

--

So overall... I think the IO::Socket::IP and LWP::Protocol::https changes are good, but not the the CPAN::Meta one.

(In reply to David Cook from comment #17)
> So overall... I think the IO::Socket::IP and LWP::Protocol::https changes
> are good, but not the the CPAN::Meta one.

That is to say, at the moment, we don't install libcpan-meta-perl in koha-testing-docker or prod. If we pushed this change, installing Koha would now install libcpan-meta-perl.

(In reply to Jonathan Druart from comment #9)
> I would not remove CPAN::Meta from the cpanfile: 
> 
>   commit a34d086a24b1e4519f8b31331d4c42b87b8e4a83
>   Bug 19735: Move Perl deps definitions into a cpanfile
> 
>     Added dependencies:
>      - Module::CPANfile
>      - CPAN::Meta (dependency of Module::CPANfile, but we need a more recent
>        version than the one Module::CPANfile requires)

Ok let's consider versions... 

Our oldest supported OS is... maybe Debian 11 (Bullseye) which uses Perl 5.32 or Ubuntu 20.04 which uses Perl 5.30.0. 

- Currently, we're requiring 2.150010
- CPAN::Meta in 5.14.0 is 2.110440
- CPAN::Meta in 5.26.0 is 2.150010

So we should be good from Perl 5.26.0 onwards.

Restored Mason's cpanfile patch and re-ran the test plan...

libcpan-meta-perl line is removed and liblwp-protocl-https-perl is included without version numbers.

Looks good to me. Going to mark as signed off. 

I suppose additional testing would involve building the package and making sure things using CPAN::Meta and LWP https URLs still works, but I'd be very confident that they would. The only potential problem would be with Koha running on very old OSes.

Created attachment 173848 [details] [review]
Bug 33018: debian package tidy-up

this patch removes unneeded package dependencies

to test:
 - apply patch
 - build and install koha-common package, note success

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Not tested, I trust Mason to the right thing.
Signed-off-by: David Cook <dcook@prosentient.com.au>

Created attachment 173849 [details] [review]
Bug 33018: debian package tidy-up (2)

remove unrequired module

	modified:   cpanfile

Signed-off-by: David Cook <dcook@prosentient.com.au>