Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

scripts/pythondistdeps: Implement provides/requires for extras packages + Rework error messages #1546

Closed
wants to merge 2 commits into from

Conversation

torsava
Copy link
Contributor

@torsava torsava commented Feb 18, 2021

In Fedora we have adapted the pythondistdeps.py script (that generates python3.Xdist(foo) dependencies) to also generate requirements on Python extras (e.g. python3.Xdist(foo[bar])) whenever upstream metadata indicate such dependency.

The script was also adapted to be able to process a new Python extras packages that have the format base_package+extras_name (e.g. python3-setuptools_scm+toml) and generate provides and requires for them.

This has been implemented in Fedora 33: https://fedoraproject.org/wiki/Changes/PythonExtras

The change also includes an improvement of the error messages: Previously the error messages were printed to stdout, they were part of the output of the script, and thus the words were treated like dependencies, sorted alphabetically, thus rendering the message unreadable. This change prints a single keyword indicating the error and details are outputted to stderr. It also outputs *** on purpose to ensure the dependency would not be valid, causing a failure of the build.

Previously the error messages were printed to stdout, they were part of
the output of the script. Thus the words were treated like
dependencies, sorted alphabetically, thus rendering the message
unreadable. This change prints a single keyword indicating the error and
details are outputted to stderr. It also outputs *** on purpose to
ensure the dependency would not be valid, causing a failure of the
build.
jollaitbot pushed a commit to sailfishos-mirror/python-rpm-generators that referenced this pull request Feb 19, 2021
…stream

Upstream change to importlib.metadata: rpm-software-management/rpm#1317

Due to extras packages being hadled slightly differently by importlib,
one test case for this was added.  And due to changes in handling
requires.txt files, comments were removed from the pyreq2rpm.tests
testing package.

Also because of the switch, we removed the dependency on setuptools and
added a dependency on packaging.

Note: Some packages with egg-info files might provide a different name
due to this change if there is a conflict between the filename and the
name in the metadata. Previously, the filename was sometimes used to
parse the name, now it is always the content of that file, which is what
packaging does, and thus also pip and other Python tooling. Currently,
this is known to affect only 1 package in Fedora (ntpsec).

The resulting script is different from upstream because of not yet upstreamed changes in Fedora:
- scripts/pythondistdeps: Rework error messages
- scripts/pythondistdeps: Add parameter --package-name
- scripts/pythondistdeps: Implement provides/requires for extras packages
- pythondistdeps.py: When parsing extras name, take the rightmost +

These changes are proposed in this upstream PR: rpm-software-management/rpm#1546
@hroncok
Copy link
Contributor

hroncok commented Mar 13, 2021

Note: We should include https://src.fedoraproject.org/rpms/python-rpm-generators/pull-request/35

@hroncok
Copy link
Contributor

hroncok commented Apr 19, 2021

@torsava Could you please reopen this to https://github.com/rpm-software-management/python-rpm-packaging ?

@torsava
Copy link
Contributor Author

torsava commented Apr 21, 2021

I'm closing this PR as it has been reopened against the newly separate python-rpm-packaging project: rpm-software-management/python-rpm-packaging#7

@torsava torsava closed this Apr 21, 2021
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.

None yet

2 participants