Skip to content

PR-1947 finalizations#2128

Open
NicholasBHubbard wants to merge 3 commits intojordansissel:mainfrom
NicholasBHubbard:pr-1947-finalizations
Open

PR-1947 finalizations#2128
NicholasBHubbard wants to merge 3 commits intojordansissel:mainfrom
NicholasBHubbard:pr-1947-finalizations

Conversation

@NicholasBHubbard
Copy link
Contributor

@NicholasBHubbard NicholasBHubbard commented Nov 24, 2025

This PR finalizes the already merged PR #1947. @jordansissel, in this comment on PR #1947, said he would merge the PR, but before the changes make it into an fpm release he would like to rename the new flag from --cpan-package-reject-from-depends to --cpan-disable-dependency, and tests should be added for the new features. Commit 56450ce renames the flag, and commit 706552a adds the new tests. Note that I added the tests to deb_spec.rb (instead of cpan_spec.rb) since the new code from PR #1947 primarily lives in deb.rb and not cpan.rb.

While writing the new tests I found a bug in the code, which commit f22581a fixes. The problem is that user-specified disabled dependencies are not actually excluded if they happen to have a version requirement. This is due to the strings naming found dependencies being compared against the user-specified ignored dependencies, where only the former contains the version requirement. I fixed this by stripping off a potential version specification in the found dependency before doing the comparison.

Here is a reproduction of the bug. The first command sequence shows that the user-specified Digest::base ignored dependency is not ignored in fpm currently, while the second command sequence is shows that this PR fixes the issue.

### current fpm
$ fpm --cpan-package-reject-from-depends Digest::base --debug-workspace -s cpan -t deb Digest::MD5
Blanking 'provides' field 'perl(Digest::MD5) = 2.59' because it's invalid {:level=>:warn}
Debian tools (dpkg/apt) don't do well with packages that use capital letters in the name. In some cases it will automatically downcase them, in others it will not. It is confusing. Best to not use any capital letters at all. I have downcased the package name for you just to be safe. {:oldname=>"perl-Digest-MD5", :fixedname=>"perl-digest-md5", :level=>:warn}
Downcasing dependency 'libDigest-base-perl' because deb packages  don't work so good with uppercase names {:level=>:warn}
Downcasing dependency 'libXSLoader-perl' because deb packages  don't work so good with uppercase names {:level=>:warn}
Created package {:path=>"libdigest-md5-perl_2.59_amd64.deb"}
plugin directory {:plugin=>"cpan", :pathtype=>:staging_path, :path=>"/tmp/package-cpan-staging-09a6e30a79a7dfd67e5d58a0aa4c16f5ffaa5147d5c40063d987a869f0f0"}
plugin directory {:plugin=>"cpan", :pathtype=>:build_path, :path=>"/tmp/package-cpan-build-89b3145f4a63de73d44b79ec28ee3a63db52974dba68dd5f0eb9c688e4b0"}
plugin directory {:plugin=>"deb", :pathtype=>:staging_path, :path=>"/tmp/package-cpan-staging-09a6e30a79a7dfd67e5d58a0aa4c16f5ffaa5147d5c40063d987a869f0f0"}
plugin directory {:plugin=>"deb", :pathtype=>:build_path, :path=>"/tmp/package-deb-build-c8a6a0a9ed9e17eb8fe43012011a6715c23e5ba3f6287800342f9c6cd5fa"}
$ cd /tmp/package-deb-build-c8a6a0a9ed9e17eb8fe43012011a6715c23e5ba3f6287800342f9c6cd5fa
$ tar -xf control.tar.gz
$ grep ^Depends control
Depends: libdigest-base-perl (>= 1.00), libxsloader-perl, perl (>= 5.006)
### fpm from this PR
$ fpm --cpan-disable-dependency Digest::base --debug-workspace -s cpan -t deb Digest::MD5
Blanking 'provides' field 'perl(Digest::MD5) = 2.59' because it's invalid {:level=>:warn}
Debian tools (dpkg/apt) don't do well with packages that use capital letters in the name. In some cases it will automatically downcase them, in others it will not. It is confusing. Best to not use any capital letters at all. I have downcased the package name for you just to be safe. {:oldname=>"perl-Digest-MD5", :fixedname=>"perl-digest-md5", :level=>:warn}
Downcasing dependency 'libXSLoader-perl' because deb packages  don't work so good with uppercase names {:level=>:warn}
Created package {:path=>"libdigest-md5-perl_2.59_amd64.deb"}
plugin directory {:plugin=>"cpan", :pathtype=>:staging_path, :path=>"/tmp/package-cpan-staging-99aad5497243c8258ad3df05bb8b41baa07ab3672d899b0b97f5f3a96bed"}
plugin directory {:plugin=>"cpan", :pathtype=>:build_path, :path=>"/tmp/package-cpan-build-1ced1496cb4690069dc37ecd105e3ec3e77e87f5a42d32c8a3db00c659cc"}
plugin directory {:plugin=>"deb", :pathtype=>:staging_path, :path=>"/tmp/package-cpan-staging-99aad5497243c8258ad3df05bb8b41baa07ab3672d899b0b97f5f3a96bed"}
plugin directory {:plugin=>"deb", :pathtype=>:build_path, :path=>"/tmp/package-deb-build-084fa0ff5c260557a6d91154c4fd667e886782d9716ec4cf77d29621ae0b"}
$ cd /tmp/package-deb-build-084fa0ff5c260557a6d91154c4fd667e886782d9716ec4cf77d29621ae0b
$ tar -xf control.tar.gz
$ grep ^Depends control
Depends: libxsloader-perl, perl (>= 5.006)

Let me know how everything looks. Also thanks again to @aranc23 for this nice contribution!

@wbraswell
Copy link
Contributor

@jordansissel
I have reviewed and approved this PR, as per usual. :-)

@jordansissel
Copy link
Owner

Thank you both for your work on this! I’ll review it shortly and will let you know if anything needs changing.

Bonus bug fixing, yes!! 👍

@wbraswell
Copy link
Contributor

Howdy @jordansissel , I hope you are doing well sir! :-)

Nicholas and I have been hard at work on fpm-related fixes as usual, and now this PR is our latest blocker that we need to have merged as soon as possible please.

I believe everything is good to go on this PR, and you should be able to immediately approve it as-is!

Thanks,
~ Will

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants