CPAN->deb related fixes

[aranc23]

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

[jordansissel]

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]

@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.

[wbraswell]

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]

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

[NicholasBHubbard]

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.

[jordansissel]

@NicholasBHubbard @wbraswell thanks for the swift review <3

[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?

[aranc23]

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.

[jordansissel]

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 <#1947 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABAF2TMABBC77GRGEJVXF3WF7TSNANCNFSM6AAAAAARQMXGTA>. You are receiving this because you were mentioned.Message ID: ***@***.***>

[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:

fpm/lib/fpm/package/deb.rb

Lines 675 to 760 in 5b104bc

	def converted_from(origin)
	self.dependencies = self.dependencies.collect do |dep|
	fix_dependency(dep)
	end.flatten
	self.provides = self.provides.collect do |provides|
	fix_provides(provides)
	end.flatten
	if origin == FPM::Package::CPAN
	# The fpm cpan code presents dependencies and provides fields as perl(ModuleName)
	# so we'll need to convert them to something debian supports.
	# Replace perl(ModuleName) > 1.0 with Debian-style perl-ModuleName (> 1.0)
	perldepfix = lambda do |dep|
	m = dep.match(/perl\((?<name>[A-Za-z0-9_:]+)\)\s*(?<op>.*$)/)
	if m.nil?
	# 'dep' syntax didn't look like 'perl(Name) > 1.0'
	dep
	else
	# Also replace '::' in the perl module name with '-'
	modulename = m["name"].gsub("::", "-")
	# Fix any upper-casing or other naming concerns Debian has about packages
	name = "#{attributes[:cpan_package_name_prefix]}-#{modulename}"
	if m["op"].empty?
	name
	else
	# 'dep' syntax was like this (version constraint): perl(Module) > 1.0
	"#{name} (#{m["op"]})"
	end
	end
	end
	rejects = [ "perl(vars)", "perl(warnings)", "perl(strict)", "perl(Config)" ]
	self.dependencies = self.dependencies.reject do |dep|
	# Reject non-module Perl dependencies like 'vars' and 'warnings'
	rejects.include?(dep)
	end.collect(&perldepfix).collect(&method(:fix_dependency))
	# Also fix the Provides field 'perl(ModuleName) = version' to be 'perl-modulename (= version)'
	self.provides = self.provides.collect(&perldepfix).collect(&method(:fix_provides))
	end # if origin == FPM::Packagin::CPAN
	if origin == FPM::Package::Deb
	changelog_path = staging_path("usr/share/doc/#{name}/changelog.Debian.gz")
	if File.exists?(changelog_path)
	logger.debug("Found a deb changelog file, using it.", :path => changelog_path)
	attributes[:deb_changelog] = build_path("deb_changelog")
	File.open(attributes[:deb_changelog], "w") do |deb_changelog|
	Zlib::GzipReader.open(changelog_path) do |gz|
	IO::copy_stream(gz, deb_changelog)
	end
	end
	File.unlink(changelog_path)
	end
	end
	if origin == FPM::Package::Deb
	changelog_path = staging_path("usr/share/doc/#{name}/changelog.gz")
	if File.exists?(changelog_path)
	logger.debug("Found an upstream changelog file, using it.", :path => changelog_path)
	attributes[:deb_upstream_changelog] = build_path("deb_upstream_changelog")
	File.open(attributes[:deb_upstream_changelog], "w") do |deb_upstream_changelog|
	Zlib::GzipReader.open(changelog_path) do |gz|
	IO::copy_stream(gz, deb_upstream_changelog)
	end
	end
	File.unlink(changelog_path)
	end
	end
	if origin == FPM::Package::Gem
	# fpm's gem input will have provides as "rubygem-name = version"
	# and we need to convert this to Debian-style "rubygem-name (= version)"
	self.provides = self.provides.collect do |provides|
	m = /^(#{attributes[:gem_package_name_prefix]})-([^\s]+)\s*=\s*(.*)$/.match(provides)
	if m
	"#{m[1]}-#{m[2]} (= #{m[3]})"
	else
	provides
	end
	end
	end
	end # def converted_from

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]

@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)

[bouda1]

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.

[wbraswell]

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

[NicholasBHubbard]

looks good to me

[omercier]

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