fpm icon indicating copy to clipboard operation
fpm copied to clipboard

CPAN->deb related fixes

Open aranc23 opened this issue 2 years ago • 15 comments

I found a few issues when building deb packages from CPAN modules:

The names of automatic dependencies created from the CPAN modules didn't match the standard (and what I observed to be the case on Ubuntu 20.04) (https://www.debian.org/doc/packaging-manuals/perl-policy/ch-module_packages.html)

The naming of deb packages from CPAN modules couldn't be set correctly using the --cpan-package-name-prefix option because the code assumed there is a dash between the prefix and the package/module name. It also can't be set correctly without adding a --cpan-package-name-postfix option. (ie: Config::Validator, rpm: perl-Config-Validator, deb: libconfig-validator-perl)

Automatic dependency creation for CPAN modules in deb packages needed the ability to filter out some modules that are provided by other packages or base perl package so I added --cpan-package-reject-from-depends

aranc23 avatar Oct 27 '22 19:10 aranc23

I like with the idea of this change -- that is, to emit Debian packages that more closely match what Debian calls their own Perl packages. It makes sense!

I found a few things which I would considering a breaking change, and I'm not sure what the best next step is. I'd like to find a path forward that reduces surprises to folks who are upgrading and receive this change.

Here's some differences I noted. In my tests, I was not using any new flags.

When packaging Regexp::Common, the provides are:

  • On main branch: Provides: perl-regexp-common (= 2017060201), perl-regexp-common-entry (= 2017060201)
  • On this PR: Provides: libregexp-common-perl (= 2017060201), libregexp-common-entry-perl (= 2017060201)

fpm -f -s cpan -t deb Regexp::Common

When packaging HTTP::Request, the depends are:

  • Depends: perl-carp, perl-clone (>= 0.46), perl-compress-raw-bzip2, perl-compress-raw-zlib (>= 2.062), perl-encode (>= 3.01), perl-encode-locale (>= 1), perl-exporter (>= 5.57), perl-file-spec, perl-http-date (>= 6), perl-io-compress-bzip2 (>= 2.021), perl-io-compress-deflate, perl-io-compress-gzip, perl-io-html, perl-io-uncompress-inflate, perl-io-uncompress-rawinflate, perl-lwp-mediatypes (>= 6), perl-mime-base64 (>= 2.1), perl-mime-quotedprint, perl-uri (>= 1.10), perl-parent, perl (>= 5.8.1)
  • On this PR: Depends: libcarp-perl, libclone-perl (>= 0.46), libcompress-raw-bzip2-perl, libcompress-raw-zlib-perl (>= 2.062), libencode-perl (>= 3.01), libencode-locale-perl (>= 1), libexporter-perl (>= 5.57), libfile-spec-perl, libhttp-date-perl (>= 6), libio-compress-bzip2-perl (>= 2.021), libio-compress-deflate-perl, libio-compress-gzip-perl, libio-html-perl, libio-uncompress-inflate-perl, libio-uncompress-rawinflate-perl, liblwp-mediatypes-perl (>= 6), libmime-base64-perl (>= 2.1), libmime-quotedprint-perl, liburi-perl (>= 1.10), libparent-perl, perl (>= 5.8.1)

fpm --verbose -f -s cpan -t deb --no-cpan-test HTTP::Request

The impact is that someone upgrading (from the current version of FPM to something with this PR included) would get very different debian package results which are likely to cause disruptions.

This change impacts the provides and depends fields (of Debian packages), but the original package name is the same in both main and this PR -- perl-http-message_6.44_all.deb perl-regexp-common_2017060201_all.deb, for example.

jordansissel avatar Oct 29 '22 02:10 jordansissel

@wbraswell @NicholasBHubbard Y'all use the cpan feature a bit, I think? Can y'all take a look at this and let us know your thoughts? I'm mostly interested in the behavior changes and less worried about the code itself.

jordansissel avatar Oct 29 '22 02:10 jordansissel

Please give @NicholasBHubbard and a little time to carefully review this PR, to make sure it's not something we're already working on fixing ourselves.

wbraswell avatar Oct 29 '22 03:10 wbraswell

@jordansissel Okay we're good with accepting this PR, thanks!

wbraswell avatar Oct 29 '22 23:10 wbraswell

This PR looks good to me, and I agree that it is probably a good idea for CPAN deb packages to be named using Debian conventions. A test for this functionality should probably be added to either deb_spec.rb or cpan_spec.rb.

NicholasBHubbard avatar Oct 29 '22 23:10 NicholasBHubbard

@NicholasBHubbard @wbraswell thanks for the swift review <3

jordansissel avatar Oct 30 '22 05:10 jordansissel

Summarizing my above comments, this change feels pretty significant and could cause problems for users who are expecting the previous behavior after they upgrade, or more specifically, aren't expecting to be surprised ;)

Thoughts:

  • Make the package names follow Debian style, not just the Provides/Depends fields. That is, this PR still produces packages named 'perl-whatever' instead of 'libwhatever-perl'
  • Have a documented way to restore the old behavior (packages and dependencies named perl-whatever)
  • Consider making this new behavior the default, and I'll take care of trying to announce the behavior change somewhat loudly in the changelog and other comms channels.

@aranc23 what do you think?

jordansissel avatar Oct 30 '22 05:10 jordansissel

Make the package names follow Debian style, not just the Provides/Depends fields. That is, this PR still produces packages named 'perl-whatever' instead of 'libwhatever-perl':

Since the package name is set in cpan.rb, I couldn't figure out how to tell what the target was in order to implement different package names depending on the output format. Is that context available in the source module?

Have a documented way to restore the old behavior (packages and dependencies named perl-whatever):

I think that would be straightforward.

aranc23 avatar Oct 31 '22 15:10 aranc23

I’ll see if I can help come up with a little demo to show how to do this

On Mon, Oct 31, 2022, at 8:54 AM, Aran wrote:

Make the package names follow Debian style, not just the Provides/Depends fields. That is, this PR still produces packages named 'perl-whatever' instead of 'libwhatever-perl':

Since the package name is set in cpan.rb, I couldn't figure out how to tell what the target was in order to implement different package names depending on the output format. Is that context available in the source module?

Have a documented way to restore the old behavior (packages and dependencies named perl-whatever): I think that would be straightforward.

— Reply to this email directly, view it on GitHub https://github.com/jordansissel/fpm/pull/1947#issuecomment-1297303340, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABAF2TMABBC77GRGEJVXF3WF7TSNANCNFSM6AAAAAARQMXGTA. You are receiving this because you were mentioned.Message ID: @.***>

jordansissel avatar Nov 03 '22 05:11 jordansissel

Here's some details about the internals and what methods are called when fpm converts a package. Inside fpm, package conversion will pass through a converted_from method on the class that is the output type, in this case "deb". That is, something like fpm -s cpan -t deb Regexp::Common will do, roughly:

  • In FPM::Package::CPAN, call input("Regexp::Common")
  • Hand off some metadata to a new object, FPM::Package::Deb
  • Call FPM::Package::Deb's converted_from(FPM::Package::CPAN) so that the new deb package instance can apply any cpan-specific conversions.

Here's a link to the Deb converted_from code: https://github.com/jordansissel/fpm/blob/5b104bccf04a1f4c0d79a72ebf10f68db2046225/lib/fpm/package/deb.rb#L675-L760

In this scenario, we can add additional handling for the CPAN case to change the package name itself.

diff --git a/lib/fpm/package/deb.rb b/lib/fpm/package/deb.rb
index 77b69e1..34f7fb8 100644
--- a/lib/fpm/package/deb.rb
+++ b/lib/fpm/package/deb.rb
@@ -681,6 +681,18 @@ class FPM::Package::Deb < FPM::Package
     end.flatten
 
     if origin == FPM::Package::CPAN
+
+      # By default, we'd prefer to name Debian-targeted Perl packages using the
+      # same naming scheme that Debian itself uses, which is usually something
+      # like "lib<module-name-hyphenated>-perl", such as libregexp-common-perl
+      #
+      # Therefore, if the --cpan-package-name-prefix flag is not set, we should 
+      # make our package name use this same scheme.
+      if !attributes[:cpan_package_name_prefix_given?]
+        logger.info("Changing package name to match Debian's typical libmodule-name-perl style")
+        self.name = "lib#{self.name.sub(/^perl-/, "")}-perl"
+      end
+
       # The fpm cpan code presents dependencies and provides fields as perl(ModuleName)
       # so we'll need to convert them to something debian supports.

Using the above patch, I can try it with:

% fpm -s cpan -t deb Regexp::Common
Created package {:path=>"libregexp-common-perl_2017060201_all.deb"}

# I don't have `dpkg` on my current machine, so I'll unpack the Debian control file another way to show the Package, Depends, and Provides fields:
% ar p libregexp-common-perl_2017060201_all.deb control.tar.gz | tar -zxO ./control | egrep '^(Provides|Depends|Package):'
Package: libregexp-common-perl
Depends: perl (>= 5.01)
Provides: libregexp-common-perl (= 2017060201), libregexp-common-entry-perl (= 2017060201)

Thoughts? Does this look right? :)

jordansissel avatar Nov 14 '22 04:11 jordansissel

@aranc23 I made a small change in the command-line docs generator that now caused a merge conflict on your docs/cli-reference.rst changes.

I regenerate the docs before each release, so it's safe to remove the changes to the flag documentation (cli-reference.rst, etc)

jordansissel avatar Nov 14 '22 07:11 jordansissel

I see this PR today and still not merged whereas you look all agree about it. Is there something wrong? I'm very excited to use it with my debian box. Thank you.

bouda1 avatar Jun 14 '24 09:06 bouda1

@jordansissel & @NicholasBHubbard : are we all in agreement that this PR can be merged now?

wbraswell avatar Jun 14 '24 16:06 wbraswell

looks good to me

NicholasBHubbard avatar Jun 16 '24 02:06 NicholasBHubbard

Hi folks, No pressure but we'd love to see this enhancement come to production :smiling_face_with_tear: Take care!

omercier avatar Jul 11 '24 09:07 omercier