Fedora Account System
Red Hat Associate
Red Hat Customer
python-iso4217 - Python package contains ISO 4217 currency data, represented as enum module Reproducible: Always
spec: https://download.copr.fedorainfracloud.org/results/averbyts/I3313/fedora-rawhide-x86_64/10352098-python-iso4217/python-iso4217.spec srpm: https://packages.redhat.com/api/pulp-content/public-copr/averbyts/I3313/fedora-44-x86_64/Packages/p/python-iso4217-1.16-1.fc44.src.rpm
Cannot find any valid SRPM URL for this ticket. Common causes are: - You didn't specify `SRPM URL: ...` in the ticket description or any of your comments - The URL schema isn't HTTP or HTTPS - The SRPM package linked in your URL doesn't match the package name specified in the ticket summary --- 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.
Copr build: https://copr.fedorainfracloud.org/coprs/build/10355232 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2460052-python-iso4217/fedora-rawhide-x86_64/10355232-python-iso4217/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- 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.
New packages “MUST” use the current Python packaging guidelines with pyproject-rpm-macros, https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/, rather than the “201x-era” Python packaging guidelines, https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/, as in this submission. This is a simple package, so you should find it straightforward to update it to current practices. See https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_example_spec_file for an example. This is particularly messy and weird: Provides: python%{python3_version}dist(iso4217) This is pointless because Provides will be generated automatically, https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_provides_for_importable_modules, and this style, using a modern python3dist(…) dependency with the %{python3_version} is a weird mishmash that I wouldn’t expect someone to come up with manually. Even an outdated tool like pyp2rpm shouldn’t emit something this weird. Are you using an LLM to author these spec files? “Public Domain” is not a valid SPDX license expression. It should be (assuming you have identified the license correctly, which I haven’t checked) LicenseRef-Fedora-Public-Domain, and you must submit the license text for validation, similar to https://gitlab.com/fedora/legal/fedora-license-data/-/work_items/728. See https://docs.fedoraproject.org/en-US/legal/update-existing-packages/#_licenseref_callaway_public_domain, which mostly applies to new packages even though it is under the documentation for updating existing packages to SPDX.
Dear Ben, Many thanks for the suggestions. I've implemented them, however the license issue is not very clear to me. I've submitted a PR in the upstream with the explicit LICENSE file and it was accepted https://github.com/dahlia/iso4217/pull/30, so I would assume one can just create a LICENSE file in the spec manually. spec: https://download.copr.fedorainfracloud.org/results/averbyts/I3313/fedora-rawhide-x86_64/10356720-python-iso4217/python-iso4217.spec srpm: https://packages.redhat.com/api/pulp-content/public-copr/averbyts/I3313/fedora-rawhide-x86_64/Packages/p/python-iso4217-1.16-1.fc45.src.rpm
Created attachment 2138037 [details] The .spec file difference from Copr build 10355232 to 10359799
Copr build: https://copr.fedorainfracloud.org/coprs/build/10359799 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2460052-python-iso4217/fedora-rawhide-x86_64/10359799-python-iso4217/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- 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.
LICENSE file now matches upstream spec: https://download.copr.fedorainfracloud.org/results/averbyts/I3313/fedora-44-x86_64/10359847-python-iso4217/python-iso4217.spec srpm: https://packages.redhat.com/api/pulp-content/public-copr/averbyts/I3313/fedora-rawhide-x86_64/Packages/p/python-iso4217-1.16-1.fc45.src.rpm
Created attachment 2138039 [details] The .spec file difference from Copr build 10359799 to 10359855
Copr build: https://copr.fedorainfracloud.org/coprs/build/10359855 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2460052-python-iso4217/fedora-rawhide-x86_64/10359855-python-iso4217/builder-live.log.gz Please make sure the package builds successfully at least for Fedora Rawhide. - If the build failed for unrelated reasons (e.g. temporary network unavailability), please ignore it. - If the build failed because of missing BuildRequires, please make sure they are listed in the "Depends On" field --- 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.
(In reply to andrii.verbytskyi from comment #5) > Dear Ben, > > Many thanks for the suggestions. I've implemented them, however the license > issue is not very clear to me. > I've submitted a PR in the upstream with the explicit LICENSE file and it > was accepted https://github.com/dahlia/iso4217/pull/30, so I would assume > one can just create a LICENSE file in the spec manually. Thanks, this is starting to look better. ---- The LICENSE file is nice, but it’s not mandatory in this case. Please have a look at https://gitlab.com/fedora/legal/fedora-license-data/-/work_items/728 for an example. Since public-domain declarations are not standardized and take many forms (and since in theory something that looks like a public-domain declaration might have other conditions attached that make it ineffective, although this is unusual), Fedora Legal asks that all public-domain dedications be submitted for review and recorded in the file public-domain-text.txt in the fedora-license-data package. 1. Create an issue (“work item”) on https://gitlab.com/fedora/legal/fedora-license-data, using the license review template, similar to the one I linked. Fill in all the applicable fields. 2. Ideally, create the MR to update public-domain-text.txt yourself, referencing your work item and imitating https://gitlab.com/fedora/legal/fedora-license-data/-/merge_requests/838. 3. The dedication here is trivial, so you can expect that someone from Fedora Legal will mark it as approved within a couple of days or so. 4. Change the License field of this submission to LicenseRef-Fedora-Public-Domain (you already did this), and ideally add a comment linking your fedora-license-data issue so it’s easy to see that the text has been submitted for review. ---- Since you passed “-l” to %pyproject_save_files, asserting that a license file is properly marked in the dist-info metadata, you don’t need to also install an additional copy in /usr/share/licenses/…: you can remove “%license LICENSE”. ---- If you are going to do this: echo "This software is released into the public domain." > LICENSE you should be able to cite where that exact dedication came from, either in the source code or in a PR that was merged upstream. In fact, this text doesn’t seem to match what was added upstream in https://github.com/dahlia/iso4217/commit/aa87a85494ff240a3faadfaa84eb832b8168d4df. (On preview, I see that you addressed this in https://bugzilla.redhat.com/show_bug.cgi?id=2460052#c8, above.) Consider something like this instead: # Add LICENSE file with Public Domain declaration Patch: %{url}/commit/aa87a85494ff240a3faadfaa84eb832b8168d4df.patch […] %autosetup -n %{srcname}-%{version} -p1 ---- A better source URL would be https://github.com/dahlia/iso4217/archive/%{version}/iso4217-%{version}.tar.gz, or %{url}/archive/%{version}/iso4217-%{version}.tar.gz if you prefer. That way, the archive name matches the extraction directory. At minimum, please use the %{version} macro instead of hard-coding the version number in the URL. ---- It seems like it ought to be possible to run the test suite. You should try to do this, https://docs.fedoraproject.org/en-US/packaging-guidelines/Python/#_tests. If there’s something stopping you, please document it in a spec-file comment. ---- Putting the %check section after %files doesn’t make anything work differently, but it hurts legibility. Consider putting it after %install instead. ---- The build has to work offline. Upstream’s setup.py downloads an XML data file at build time and installs it into the source tree as iso4217/table.xml. https://github.com/dahlia/iso4217/blob/7f4c46981f72b571f3f461b6e8d971db5a3e19ef/setup.py#L20-L52 This is the reason that the “Fedora Review Service” test build failed. You could probably work around this by including the data table as additional Source, manually copying it into the source tree in %prep, and exporting ISO4217_DOWNLOAD=0 before %pyproject_generate_buildrequires and before %pyproject_wheel. There’s a bigger problem, though: it’s not clear what license applies to the XML data file, https://www.six-group.com/dam/download/financial-information/data-center/iso-currrency/lists/list-one.xml. I found the link to this data file at https://www.six-group.com/en/products-services/financial-information/market-reference-data/data-standards.html#scrollTo=currency-codes, but it doesn’t clarify the license terms. The corresponding ISO standard page, https://www.iso.org/iso-4217-currency-codes.html, doesn’t help either: “free of charge” is not a license. In my opinion, this will need to be clarified before the package can be approved.