Spec URL: https://ppfeister.fedorapeople.org/python-types-colorama.spec SRPM URL: https://ppfeister.fedorapeople.org/python-types-colorama-0.4.15.20240311-1.fc42.src.rpm Description: This is a PEP 561 type stub package for the colorama package. It can be used by type-checking tools like mypy, pyright, pytype, PyCharm, etc. to check code that uses colorama. Fedora Account System Username: ppfeister
This package built on koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=121934082
Disregard Koji build (old revision, using deprecated macros) Otherwise ready for review
[fedora-review-service-build]
Copr build: https://copr.fedorainfracloud.org/coprs/build/7948464 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2304820-python-types-colorama/fedora-rawhide-x86_64/07948464-python-types-colorama/fedora-review/review.txt Found issues: - License file LICENSE is not marked as %license Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_text Please know that there can be false-positives. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service If you want to trigger a new Copr build, add a comment containing new Spec and SRPM URLs or [fedora-review-service-build] string.
Hi Paul, are you still interested in getting this package into Fedora? If so, I can review it.
Hey Felix Let's do it! This is one of many dependencies for a larger project I'd like to push down the line. Not time sensitive but a WIP.
I'll look into this bug tomorrow. Feel free to suggest also other (hopefully simple :-) python packages to review. If you have time, you can also have a look at https://bugzilla.redhat.com/show_bug.cgi?id=2336812 which I need for WeasyPrint 63.1 (don't worry, I'll review this package anyway)
The package itself looks fine, the only problematic thing the license file (as indicated also above by Fedora review): > License file LICENSE is not marked as %license This is mostly a formal point but I think it would be good if you could raise the issue upstream so they can fix it for everyone. Relevant parts from the Fedora packaging guidelines: https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuidelines/#_license_text > If the source package does not include the text of the license(s), the packager should contact upstream and encourage them to correct this mistake. > > In cases where the upstream has chosen a license that requires that a copy of the license text be distributed along with the binaries and/or source code, but does not provide a copy of the license text (in the source tree, or in some rare cases, anywhere), the packager should do their best to point out this confusion to upstream. From my point of view the ASL requires shipping the license text: > 4. Redistribution. You may reproduce and distribute copies of the > Work or Derivative Works thereof in any medium, with or without > modifications, and in Source or Object form, provided that You > meet the following conditions: > > (a) You must give any other recipients of the Work or > Derivative Works a copy of this License; and Can you please open an upstream issue for this? I believe the proper fix should look like this: setup( ... license_files = ('LICENSE',), ... ) If upstream does not provide a timely response, at least add a comment pointing to the upstream issue. Your approach of retrieving the license from the github repo seems to be covered by the guidelines: > However, in situations where upstream is unresponsive, unable, or unwilling to provide proper full license text as part of the source code, and the indicated license requires that the full license text be included, Fedora Packagers must either: > - Include a copy of what they believe the license text is intended to be, as part of the Fedora package in %license, in order to remain in compliance. [...] Packagers who choose to do this should ensure that they have exhausted all attempts to work with upstream to include the license text as part of the source code, or at least, to confirm the full license text explicitly with the upstream, as this minimizes the risk on the packager. Packagers may also take copies of license texts from reliable and canonical sources (such as the original license text from the license steward, Fedora licenses page, the FSF licenses page, or the OSI license list), whenever possible. Currently you just use the latest version from the "main" branch (https://raw.githubusercontent.com/python/typeshed/main/LICENSE). Maybe it is safer to use the contents for a specific commit id? E.g. https://raw.githubusercontent.com/python/typeshed/a4e3cfefacbfa6d1b519b36fc67362cb3a199022/LICENSE Then you MUST ensure, the license file shows up in the final RPM. You can either add `license_files = ...` in %prep (my preferrence) or list it via %license in %files. Depending on the approach, I recommend that you use "-L" or "-l" for %pyproject_buildrequires (see https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_build_macros) Also I believe just using "%{pypi_source}" is deprecated. Please use "%{pypi_source types-colorama}". Other than that, the package looks good for approval.
btw. "the license file shows up in the final RPM" means "rpm -qp --licensefiles <...noarch.rpm>" returns the path of the installed license file.
I just had a look, seems like upstream actually tackled part of the problem recently: - https://github.com/typeshed-internal/stub_uploader/issues/89 - https://github.com/typeshed-internal/stub_uploader/pull/158 However when I look at https://github.com/typeshed-internal/stub_uploader/blob/main/stub_uploader/build_wheel.py I think the ideal solution would be to list the license file exlicitly in their setup.py as license_files=. However I think you can assume that the basic problem (inclusion of license file) is fixed upstream already, so if you just add a note, that might ok (+ make sure that RPM is ships the license file).
Well, life happened. I'm back finally. Can't believe it's already been 3 weeks. Thanks for the notes Felix -- I plan on addressing everything either tomorrow or Friday as long as everything goes smoothly
This is an automatic check from review-stats script. This review request ticket hasn't been updated for some time, but it seems that the review is still being working out by you. If this is right, please respond to this comment clearing the NEEDINFO flag and try to reach out the submitter to proceed with the review. If you're not interested in reviewing this ticket anymore, please clear the fedora-review flag and reset the assignee, so that a new reviewer can take this ticket. Without any reply, this request will shortly be resetted.
No response from reporter