fedora-python / pyp2rpm

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

Handle all-zero versions without crashing. #233

Closed gordonmessmer closed 3 years ago

hroncok commented 4 years ago

That makes sense. Is this for https://github.com/rpm-software-management/rpm/issues/1183 ?

torsava commented 4 years ago

Looks good. I think a new test would be in order too.

torsava commented 4 years ago

I'm adding a test to pythondistdeps script for this. So if we have a package version 0+unknown, is the correct thing to do to ignore the local version and just evaluate to RPM version 0? That's what the pythondistdeps script does currently.

@gordonmessmer @hroncok

hroncok commented 4 years ago

I think so, yes. The local version thing is described in https://www.python.org/dev/peps/pep-0440/#local-version-identifiers Most importantly:

Local version labels have no specific semantics assigned, but some syntactic restrictions are imposed.

However it also says:

Comparison and ordering of local versions considers each segment of the local version (divided by a .) separately. If a segment consists entirely of ASCII digits then that section should be considered an integer for comparison purposes and if a segment contains any ASCII letters then that segment is compared lexicographically with case insensitivity. When comparing a numeric and lexicographic segment, the numeric section always compares as greater than the lexicographic segment. Additionally a local version with a great number of segments will always compare as greater than a local version with fewer segments, as long as the shorter local version's segments match the beginning of the longer local version's segments exactly.

Which seems very complicated to check against how rpm behaves.

I am fine if we deliberately document and say that it is simply omitted. Consider also:

Local version identifiers SHOULD NOT be used when publishing upstream projects to a public index server...

If desired, we might convert it to ^ suffix, but meh.

gordonmessmer commented 3 years ago

@decathorpe