search

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

Include rc/post/dev in version. #260

Closed gordonmessmer closed 3 years ago

gordonmessmer commented 3 years ago

@decathorpe Would you review these changes?

Overall: rather than discarding a/b/rc/dev/post information provided in Python versions, those will be preserved and transformed into something that rpm understands. Because the version string may be modified for the rpm Version:, but used literally in filesystem paths, the original value needs to be stored in a define/global value in the spec file, the same way that package names are handled. Most values of %{version} are replaced with %{pypi_version}, similar to the use of %{pypi_name}

In filters.py, I've introduced two new filters. One will convert a/b/rc/dev to a tilde suffix which should be supported on RHEL 6 and 7 (I'll test that asap), and the other will convert a/b/rc/dev to a tilde suffix and also convert post to a caret suffix. If the original version doesn't need any transformation, the filter returns "%{pypi_version}", which produces a spec file that needs minimal modification for future releases. The exception is for the changelog, where the pypi_version macro can't be used, so the filters accept a flag indicating whether or not the macro or the literal version is returned.

Also in filters, macroed_url is updated to recognize the use of pypi_version rather than version.

package_getters.py is updated to preserve the version suffix which was previously discarded.

The majority of this PR is changes to the templates and the tests.

gordonmessmer commented 3 years ago

CentOS 7 can install packages with ~ version, and:

# rpmdev-vercmp 1.9 1.9.rc1
1.9 < 1.9.rc1
# rpmdev-vercmp 1.9 1.9~rc1
1.9 > 1.9~rc1
decathorpe commented 3 years ago

This is a bit above my paygrade ...

The thing that*s weird though is that this PR has six commits of which 5 have the same message, making it hard to review, since you can't tell which commit is supposed to do what. Breaking the "flesh" of this change into multiple commits would also make it easier to review :)

gordonmessmer commented 3 years ago

I'd intended to squash it all to one since the vast majority of this change is just the same update to many templates and the corresponding test spec files. I've reorganized the commits.

decathorpe commented 3 years ago

Thanks! I'll leave some comments inline.

decathorpe commented 3 years ago

Last question: When running pyp2rpm now, will it choose the latest version regardless of alpha / beta / rc status now? At least I think by default it should use the latest "stable" release, and using a newer prerelease should be optional (either by specifying the version manually or by setting a --use-prereleases flag or similar).

hroncok commented 3 years ago

https://github.com/fedora-python/pyp2rpm/blob/24002cc7df516332bf2f4198cb145d54c8ce3c98/pyp2rpm/bin.py#L127-L130

hroncok commented 3 years ago

Oh. So I was thinking. What if we have a %{pypi_version} macro in Fedora and EPEL?

Usage:

Version: 7.5.0~~dev1
%{pypi_version} -> 7.5.0dev1

Version: 7.5.0~b4
%{pypi_version} -> 7.5.0b4

Version: 7.5.0
%{pypi_version} -> 7.5.0

Version: 7.5.0^post1
%{pypi_version} -> 7.5.0post1

%{pypi_version 7.5.0~~dev1} -> 7.5.0dev1
%{pypi_version 7.5.0~b4} -> 7.5.0b4
%{pypi_version 7.5.0} -> 7.5.0
%{pypi_version 7.5.0^post1} -> 7.5.0post1

I would definitively use it in some of the packages. There is %{version_nodots} from rust srpm macros, but it adds - by default :/ See also https://github.com/rpm-software-management/rpm/issues/1219

gordonmessmer commented 3 years ago

Oh. So I was thinking. What if we have a %{pypi_version} macro in Fedora and EPEL?

From a maintenance point of view: I lean toward consistency with existing behavior. pyp2rpm already uses %{pypi_name} for the unmodified name, and converts to an rpm-semantics name where those are required. If we do the inverse here, storing the converted version in the Version field and converting to the pypi semantics for the filesystem paths, we should have a good reason for the inconsistent behavior.

From an implementation point of view: Python is a bit permissive with version parsing, and I worry that someday we'd see packages where attempting to convert back to the original version didn't work (as it would not have in issue 259, which prompted this change).

>>> from pkg_resources import parse_version
>>> parse_version('0.1.post8')
<Version('0.1.post8')>
>>> parse_version('0.1post8')
<Version('0.1.post8')>
hroncok commented 3 years ago

I see. I'd still like to have macro I can use that would strip tildes and carets from version :(

decathorpe commented 3 years ago

I see. I'd still like to have macro I can use that would strip tildes and carets from version :(

The Rust macros have some version of this, at least for stripping tilde to convert back to semver (%{version_no_tilde}).

hroncok commented 3 years ago

Yes but that replaces the tilde with dash by default and the only way to wwork around it is to provide empty string as the first argument to that macro. Which is not readable: %{version_no_tilde %{quote:}}

gordonmessmer commented 3 years ago

@decathorpe

gordonmessmer commented 3 years ago

I see. I'd still like to have macro I can use that would strip tildes and carets from version :(

https://src.fedoraproject.org/rpms/python-rpm-macros/pull-request/101

decathorpe commented 3 years ago

I think I already reviewed this once ... unless you made some drastic changes, and it works ... :shipit: