fpm icon indicating copy to clipboard operation
fpm copied to clipboard

python: Fix trailing spaces in package versions

Open Forty-Bot opened this issue 3 years ago • 4 comments

These extra spaces cause errors like warning: cannot resolve "python-requests " or warning: cannot resolve "python-flask >= 2.0" when converting python to pacman

Closes: #1858

Forty-Bot avatar Nov 11 '21 03:11 Forty-Bot

Hi, @jordansissel do you have any comments on this PR? Can it be merged?

Forty-Bot avatar Jan 07 '22 07:01 Forty-Bot

Hi! Thanks for helping make fpm better :)

This PR causes 6 new test failures, all of which seem directly related to this change and the fact that most of the tests seem to expect the extra spaces. I'll take a look and see if these are easy to fix.

jordansissel avatar Mar 31 '22 06:03 jordansissel

Beyond the test failures, it seems to actually cause fpm to create bad deb packages. I haven't dug further, but here's what I know:

Test case: fpm -f --python-bin python3 -s python -t deb requests

Looking at the Depends entry in the resulting python package:

  • main branch: Depends: python-urllib3 (<< 1.27), python-urllib3 (>= 1.21.1), python-certifi (>= 2017.4.17), python-charset-normalizer (= 2.0.0), python-idna (<< 4), python-idna (>= 2.5)
  • This PR: Depends: python-urllib3>=1.21.1, python-urllib3<1.27, python-certifi>=2017.4.17, python-charset-normalizer=2.0.0, python-idna>=2.5, python-idna<4

This PR seems to cause the python->debian package conversion to work incorrectly as the deb.rb's converted_from doesn't seem to yet understand a dependency string without spaces.

jordansissel avatar Mar 31 '22 07:03 jordansissel

This PR causes 6 new test failures, all of which seem directly related to this change and the fact that most of the tests seem to expect the extra spaces. I'll take a look and see if these are easy to fix.

How do I run tests? I had a look through your docs but I didn't see anything.

I've pushed a new version which should hopefully work better. Instead of removing spaces outright, instead strip excess spaces. This should preserve spaces for debian packages.

Forty-Bot avatar Mar 31 '22 13:03 Forty-Bot