fedora-python / pyp2rpm

Tool to convert a package from PyPI to RPM SPECFILE or to generate SRPM.
MIT License
128 stars 39 forks source link

Handle PEP 440 compatible-release "=~" #196

Closed gordonmessmer closed 4 years ago

gordonmessmer commented 5 years ago

This change will add another "requires" field, such as:

Requires: python3dist(pulpcore-plugin) < 0.2 Requires: python3dist(pulpcore-plugin) >= 0.1rc2

hroncok commented 5 years ago

Add a test maybe?

Conan-Kudo commented 5 years ago

@gordonmessmer That doesn't necessarily resolve to a single package. You need a rich dep to do that...

Requires: (python3dist(pulpcore-plugin) >= 0.1rc2 with python3dist(pulpcore-plugin) < 0.2)

Cf. http://lists.rpm.org/pipermail/rpm-ecosystem/2017-February/000471.html

gordonmessmer commented 5 years ago

Do rich deps work properly in epel6/7?

Conan-Kudo commented 5 years ago

@gordonmessmer No. You'd probably need a legacy rule for those. But it will work with epel8.

Conan-Kudo commented 5 years ago

@gordonmessmer That said, as far as I know, setuptools in EL6 and EL7 don't handle PEP 440 expressions, so they'd fail to build anyway...

gordonmessmer commented 5 years ago

I've added support for rich deps and tests, but if we want to build working spec files for epel6 and epel7, we either need to pass a feature flag into the metadata extractor, or possibly modify Convertor to detect rich deps and translate them into a pair of requirements.

In either case, epel < 8 templates could either generate Requires >= and Requires <, or Requires => and Conflicts >, neither of which is an ideal solution.

Thoughts on the preferred approach for epel6/epel7?

hroncok commented 5 years ago

Requires: python3dist(pulpcore-plugin) < 0.2 Requires: python3dist(pulpcore-plugin) >= 0.1rc2

works for me in pyp2rpm. pyp2rpm is legcay software that supports old stuff (like python2 or epel6/7). for never systems, we have automatic buildrequires.

Conan-Kudo commented 5 years ago

works for me in pyp2rpm. pyp2rpm is legcay software that supports old stuff (like python2 or epel6/7). for never systems, we have automatic buildrequires.

This is for runtime dependencies... And our dependency generator doesn't handle this case yet...

gordonmessmer commented 5 years ago

I added support using Requires/Conflicts before I saw your comment, and then worked out some other issues. Tests will fail for this branch until both PR 195 and 197 are merged. Requires/Requires is a trivial change, if that's the preferred approach.

Conan-Kudo commented 5 years ago

@gordonmessmer pyp2rpm is still generally useful for people using environments where generated buildrequires don't work, and this particular clause isn't supported in the runtime dependency generator.

So your approach makes sense to me.

gordonmessmer commented 5 years ago

I used a copy of the pulp_file module earlier simply because it was referenced in the issue that requested support for compatible-release. That pip module didn't actually install a python module, and started failing pip installs. Rather than updating it, I've added an internal micro-test module and instructed tox to create a tarball so that I don't have to commit a blob into the git repo. This seems more maintainable to me, but I'm interested in feedback on that approach.

hroncok commented 4 years ago

The Travis CI job exceeded the maximum time limit for jobs, and has been terminated.

Restarted.

evgeni commented 4 years ago

One thing that I encountered why using/testing this:

It is perfectly fine to define a dependency like this: drf-nested-routers~=0.91.0 (thus pinning it to any 0.91.*, but not allowing 0.92 or newer), even tho there never was a 0.91.0 release of drf-n-r, just 0.91. Python's version matcher does zero-padding, and is thus happy with 0.91 being installed, but RPM will bomb out.

Not sure generating >= X.Y, < X.(Y+1) for ~= X.Y.0 (and only for .0) is the most correct way to solve this, but it would at least work :)

hroncok commented 4 years ago

When you get a Python dependency, strip all .0 at the end first.

BTW cc @encukou who was looking into similar problem in Fedora macros/dep. generators.

evgeni commented 4 years ago

@hroncok you're right, this is not an issue of this PR, but of pyp2rpm in general :)

gordonmessmer commented 4 years ago

Right. I've seen issue #210, and I do plan to address handling x.y.0, but I was planning to do that in a separate PR, since it should be handled for all dependencies and not the =~ operator specifically.

gordonmessmer commented 4 years ago

@decathorpe Could you review this change set?

decathorpe commented 4 years ago

I'll look at it tomorrow.

decathorpe commented 4 years ago

I'll look at it tomorrow.

Correction: This is way above my paygrade, no idea what you're changing here, and if it's correct. Sorry.

gordonmessmer commented 4 years ago

no idea what you're changing here

The commit history has gotten a little long, which probably doesn't help. I'll reformat the changes into a small number of commits that are easier to explain.

gordonmessmer commented 4 years ago

I've re-written the changes to produce the same code, but in steps that are more understandable. Could you see if reading these four changes is easier than reviewing the change set as a whole?

dependency_convert is adapted from a related project, where we worked out ways to handle python setuptools '!=', and '~=' operators, and prefix-matching in rpm dependencies, where none of those things are supported.

https://github.com/gordonmessmer/pyreq2rpm/

decathorpe commented 4 years ago

Ok, looks pretty good from my POV.

The only gripe I have is that usually I would write dependencies this way round:

(python3dist(alabaster) >= 0.7 with python3dist(alabaster) < 0.8)

which makes much more sense when reading from left to right, instead of the opposite:

(python3dist(alabaster) < 0.8 with python3dist(alabaster) >= 0.7)

which, in my opinion, specifies the dependencies in the "wrong" order and only introduces a bigger cognitive burden.

gordonmessmer commented 4 years ago

Take a look at this patch. If that reads better to you, I can squash it and merge the branch.

decathorpe commented 4 years ago

@gordonmessmer yes, awesome. thanks!

decathorpe commented 4 years ago

@gordonmessmer looks good to me.