Bug 33018 - debian package tidy-up
Summary: debian package tidy-up
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Packaging (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Mason James
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-02-21 02:48 UTC by Mason James
Modified: 2024-11-01 00:43 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 33018: debian package tidy-up (1.30 KB, patch)
2023-02-21 02:51 UTC, Mason James
Details | Diff | Splinter Review
Bug 33018: debian package tidy-up (1.40 KB, patch)
2023-03-29 12:23 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 33018: debian package tidy-up (2) (739 bytes, patch)
2023-04-27 09:57 UTC, Mason James
Details | Diff | Splinter Review
Bug 33018: debian package tidy-up (1.45 KB, patch)
2024-11-01 00:43 UTC, David Cook
Details | Diff | Splinter Review
Bug 33018: debian package tidy-up (2) (793 bytes, patch)
2024-11-01 00:43 UTC, David Cook
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mason James 2023-02-21 02:48:59 UTC
this bug aims to track a tidy-up of debian package files
Comment 1 Mason James 2023-02-21 02:51:51 UTC Comment hidden (obsolete)
Comment 2 Magnus Enger 2023-03-29 12:21:59 UTC
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"?
Comment 3 Magnus Enger 2023-03-29 12:23:31 UTC
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.
Comment 4 Magnus Enger 2023-03-29 12:24:15 UTC
Please reset to NSO if thorough testing is considered necessary.
Comment 5 David Cook 2023-04-26 03:39:13 UTC
(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.
Comment 6 Mason James 2023-04-27 09:57:13 UTC
Created attachment 150288 [details] [review]
Bug 33018: debian package tidy-up (2)

remove unrequired module
Comment 7 Mason James 2023-04-27 10:06:30 UTC
(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
Comment 8 Magnus Enger 2023-04-27 11:07:46 UTC
Thanks Mason, sounds good!
Comment 9 Jonathan Druart 2023-05-03 09:52:58 UTC
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)
Comment 10 Jonathan Druart 2023-05-16 10:22:03 UTC
Mason, ping?
Comment 11 Mason James 2023-07-16 09:36:49 UTC
(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
Comment 12 Katrin Fischer 2023-07-29 21:55:34 UTC
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)',
Comment 13 Marcel de Rooy 2023-08-25 09:03:41 UTC
(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 :)
Comment 14 Pedro Amorim 2023-12-22 10:52:11 UTC
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.
Comment 15 Martin Renvoize (ashimema) 2024-06-04 15:53:31 UTC
Are we still keen to do this Mason?
Comment 16 David Cook 2024-10-31 23:44:48 UTC
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"
Comment 17 David Cook 2024-10-31 23:57:25 UTC
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.
Comment 18 David Cook 2024-11-01 00:06:36 UTC
(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.
Comment 19 David Cook 2024-11-01 00:42:01 UTC
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.
Comment 20 David Cook 2024-11-01 00:43:02 UTC
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>
Comment 21 David Cook 2024-11-01 00:43:05 UTC
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>