Spec URL: https://lbalhar.fedorapeople.org/python-y-py.spec SRPM URL: https://lbalhar.fedorapeople.org/python-y-py-0.5.5-1.fc37.src.rpm Description: Ypy is a Python binding for Y-CRDT. It provides distributed data types that enable real-time collaboration between devices. Fedora Account System Username: lbalhar I'm testing this package together with many more in COPR designed to package Jupyterlab and the latest Python notebook into Fedora: https://copr.fedorainfracloud.org/coprs/lbalhar/notebook/builds/ This package is very uncommon and uses bundled libs but everything is explained in the specfile.
Copr build: https://copr.fedorainfracloud.org/coprs/build/5239291 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2161518-python-y-py/fedora-rawhide-x86_64/05239291-python-y-py/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
1) I worry about the bundling. Apparently you MUST package the rust deps in versions that work. You can also package pyo3-0.16.5 compact package. https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/#_bundled_dependencies """For Rust, this means that packages MUST NOT use dependencies from a "vendor tarball" (e.g. created by running cargo vendor), but package all library dependencies separately.""" Apparently, "the thing is not compatible with the latest versions" is not an approved reason to bundle Rust crates. If you want to bundle I guess you'll need an exception from FPC or Rust SIG, I cannot grant it myself. 2) "# This archive can be generated by update-vendored-tarball.sh" Could you please include this script as a source? In the spirit of https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/#when-upstream-uses-prohibited-code (This is moot if bundling is removed.) 3) Source: y_py-%{version}-vendor.tar.gz You don't enumerate sources yet you use %{SOURCE1} in the spec. I find that a bit confusing, WDYT? 4) The License tag MUST use SPDX expressions, i.e. capital "AND": https://docs.fedoraproject.org/en-US/legal/license-field/#_conjunctive_and_licensing 5) The License tag is surprisingly simple and the license comment only mentions direct dependencies. This bundles 51 crates and all of their licenses need to be listed and reflected in the license tag. Unfortunately, this is true also if you don't bundle, as Rust builds everything statically anyway: https://docs.fedoraproject.org/en-US/legal/license-field/#_rust_packages 6) url = https://github.com/y-crdt/ypy Tip: Use url = %{url}. 7) .cargo/config I don't understand this file. Maybe add a comment that it is inspired by %%cargo_prep from RHEL?
> Apparently, "the thing is not compatible with the latest versions" is not an approved reason to bundle Rust crates. Of course it's not :) It's pretty easy to do compat packages for Rust crates (we already have hundreds of them), so there's no reason to bundle old (or in this case, new) versions. I've been busy with $LIFE and haven't had much time for Fedora packaging in the last few months, but I'll be working through the backlog of things that are needed for y-py in the near future (unless some other Rust SIG member appears from thin air).
All right, I've changed the version of lib0 and yrs in review requests to the older versions which work with y-py and I'll help you update and package compatible packages for pyo3. This now transitively or directly depends on: lib0: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2153697 atomic_refcel: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2153699 yrs: https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=2153703
Thanks! I have prepared the pyo3 0.16 update in COPR: https://copr.fedorainfracloud.org/coprs/decathorpe/pyo3-0.16/monitor/ Actual builds are blocked by needing human review for the dist-git repo requests. I'll take a look at reviewing the other packages tomorrow.
So I've built all the pyo3 packages in version 0.16 once again in my COPR repo lbalhar/notebook and then this package on top of them without bundled dependencies and it seems it works fine. Spec and SRPM reuploaded. I'm gonna try to do the review of rust-pyo3-ffi as soon as possible.
Let me see if the Copr bot thing reads comments by others: Spec URL: https://lbalhar.fedorapeople.org/python-y-py.spec SRPM URL: https://lbalhar.fedorapeople.org/python-y-py-0.5.5-1.fc37.src.rpm
Apparently not.
Spec URL: https://lbalhar.fedorapeople.org/python-y-py.spec SRPM URL: https://lbalhar.fedorapeople.org/python-y-py-0.5.5-1.fc37.src.rpm
Created attachment 1940578 [details] The .spec file difference from Copr build 5239291 to 5308776
Copr build: https://copr.fedorainfracloud.org/coprs/build/5308776 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2161518-python-y-py/fedora-rawhide-x86_64/05308776-python-y-py/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
Spec sanity =========== > %global debug_package %{nil} I would add a comment saying this is because of Rust. For an average Python packager it might not be obvious. Note that python-cryptography does not have this. > ExclusiveArch: %{rust_arches} This is no longer required or mentioned in the Rust packaging guidelines. > url = https://github.com/y-crdt/ypy Suggestion: url = %{url} > setup_requires = setuptools-rust > python_requires = >=3.6 Is either of these actually required to build the package? Package Review ============== Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated TODO: What about "unstripped-binary-or-object" rpmlint warning? I don't get it from e.g. python3-cryptography. ===== MUST items ===== C/C++: [-]: Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files is Python extension module. Generic: [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [x]: Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. [x]: License field in the package spec file matches the actual license. Note: Checking patched sources after %prep for licenses. Licenses found: "Unknown or generated", "MIT License". [x]: %build honors applicable compiler flags or justifies otherwise. [x]: Package contains no bundled libraries without FPC exception. [x]: Changelog in prescribed format. [x]: Sources contain only permissible code or content. [-]: Package contains desktop file if it is a GUI application. [-]: Development files must be in a -devel package [x]: Package uses nothing in %doc for runtime. [x]: Package consistently uses macros (instead of hard-coded directory names). [x]: Package is named according to the Package Naming Guidelines. [x]: Package does not generate any conflict. [x]: Package obeys FHS, except libexecdir and /usr/target. [-]: If the package is a rename of another package, proper Obsoletes and Provides are present. [x]: Requires correct, justified where necessary. [x]: Spec file is legible and written in American English. [-]: Package contains systemd file(s) if in need. [-]: Useful -debuginfo package or justification otherwise. [!]: Package is not known to require an ExcludeArch tag. [-]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 10240 bytes in 1 files. [x]: Package complies to the Packaging Guidelines [x]: Package installs properly. [x]: Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). [x]: If (and only if) the source package includes the text of the license(s) in its own file, then that file, containing the text of the license(s) for the package is included in %license. [x]: Package requires other packages for directories it uses. [x]: Package must own all directories that it creates. [x]: Package does not own files or directories owned by other packages. [x]: Package uses either %{buildroot} or $RPM_BUILD_ROOT [x]: Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. [x]: Macros in Summary, %description expandable at SRPM build time. [x]: Dist tag is present. [x]: Package does not contain duplicates in %files. [x]: Permissions on files are set properly. [x]: Package must not depend on deprecated() packages. [x]: Package use %makeinstall only when make install DESTDIR=... doesn't work. [x]: Package is named using only allowed ASCII characters. [x]: Package does not use a name that already exists. [x]: Package is not relocatable. [x]: Sources used to build the package match the upstream source, as provided in the spec URL. [x]: Spec file name must match the spec package %{name}, in the format %{name}.spec. [x]: File names are valid UTF-8. [x]: Packages must not store files under /srv, /opt or /usr/local Python: [x]: Python eggs must not download any dependencies during the build process. [x]: A package which is used by another package via an egg interface should provide egg info. [x]: Package meets the Packaging Guidelines::Python [x]: Package contains BR: python2-devel or python3-devel [x]: Packages MUST NOT have dependencies (either build-time or runtime) on packages named with the unversioned python- prefix unless no properly versioned package exists. Dependencies on Python packages instead MUST use names beginning with python2- or python3- as appropriate. [x]: Python packages must not contain %{pythonX_site(lib|arch)}/* in %files [x]: Binary eggs must be removed in %prep ===== SHOULD items ===== Generic: [x]: Reviewer should test that the package builds in mock. [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [x]: Final provides and requires are sane (see attachments). [?]: Package functions as described. [?]: Latest version is packaged. [x]: Package does not include license text files separate from upstream. [-]: Sources are verified with gpgverify first in %prep if upstream publishes signatures. Note: gpgverify is not used. [?]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [?]: Packages should try to preserve timestamps of original installed files. [x]: Buildroot is not present [x]: Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) [x]: No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. [x]: Packager, Vendor, PreReq, Copyright tags should not be in spec file [x]: Sources can be downloaded from URI in Source: tag [x]: SourceX is a working URL. [x]: Spec use %global instead of %define unless justified. ===== EXTRA items ===== Generic: [x]: Rpmlint is run on all installed packages. Note: There are rpmlint messages (see attachment). [-]: Large data in /usr/share should live in a noarch subpackage if package is arched. Rpmlint ------- Checking: python3-y-py-0.5.5-1.fc38.x86_64.rpm python-y-py-0.5.5-1.fc38.src.rpm ============================ rpmlint session starts ============================ rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml rpmlintrc: [PosixPath('/tmp/tmpx96x_s3s')] checks: 31, packages: 2 python3-y-py.x86_64: W: unstripped-binary-or-object /usr/lib64/python3.11/site-packages/y_py.cpython-311-x86_64-linux-gnu.so 2 packages and 0 specfiles checked; 0 errors, 1 warnings, 0 badness; has taken 0.2 s unstripped-binary-or-object is weird. is it normal for Rust? Rpmlint (installed packages) ---------------------------- ============================ rpmlint session starts ============================ rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 31, packages: 1 python3-y-py.x86_64: W: unstripped-binary-or-object /usr/lib64/python3.11/site-packages/y_py.cpython-311-x86_64-linux-gnu.so 1 packages and 0 specfiles checked; 0 errors, 1 warnings, 0 badness; has taken 0.1 s Same. Source checksums ---------------- https://files.pythonhosted.org/packages/source/y/y_py/y_py-0.5.5.tar.gz : CHECKSUM(SHA256) this package : f222bab71d8d3df9a40b2e5ab3a767d734c6ce11998e9a30a02fb83ab3e090b3 CHECKSUM(SHA256) upstream package : f222bab71d8d3df9a40b2e5ab3a767d734c6ce11998e9a30a02fb83ab3e090b3 Requires -------- python3-y-py (rpmlib, GLIBC filtered): ld-linux-x86-64.so.2()(64bit) libc.so.6()(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3)(64bit) libgcc_s.so.1(GCC_4.2.0)(64bit) libm.so.6()(64bit) python(abi) = 3.11 rtld(GNU_HASH) BuildRequires ------------- (crate(lib0/default) >= 0.12.2 with crate(lib0/default) < 0.13.0~) (crate(pyo3/default) >= 0.16.5 with crate(pyo3/default) < 0.17.0~) (crate(pyo3/extension-module) >= 0.16.5 with crate(pyo3/extension-module) < 0.17.0~) (crate(yrs/default) >= 0.12.2 with crate(yrs/default) < 0.13.0~) (python3dist(toml) if python3-devel < 3.11) pyproject-rpm-macros python3-devel python3-pytest python3dist(packaging) python3dist(pip) >= 19 python3dist(setuptools) python3dist(setuptools) >= 40.8 python3dist(setuptools-rust) python3dist(wheel) rust-packaging Provides -------- python3-y-py: python-y-py = 0.5.5-1.fc38 python3-y-py = 0.5.5-1.fc38 python3-y-py(x86-64) = 0.5.5-1.fc38 python3.11-y-py = 0.5.5-1.fc38 python3.11dist(y-py) = 0.5.5 python3dist(y-py) = 0.5.5
(In reply to Miro Hrončok from comment #12) Sorry for the drive-by comment - I wanted to spend more time on helping to package this, but university stuff got in the way. > Spec sanity > =========== > > > %global debug_package %{nil} > > I would add a comment saying this is because of Rust. For an average Python > packager it might not be obvious. > > Note that python-cryptography does not have this. This looks fishy. Rust binaries should have valid debug symbols - (i.e. the .so file in this package). If removing this line breaks build of the package, then something is going wrong with setting Rust compiler flags. This might happen if you don't call `%cargo_prep` correctly, or if upstream Cargo.toml specifies to build without debuginfo - but this is not the case for this project. > > ExclusiveArch: %{rust_arches} > > This is no longer required or mentioned in the Rust packaging guidelines. Correct. Since all Fedora architectures now need support for Rust by definition (because the GPG backend for RPM itself is now Rust based), the ExclusiveArch is now basically a noop. c.f. https://pagure.io/packaging-committee/pull-request/1222 > > url = https://github.com/y-crdt/ypy > > Suggestion: url = %{url} > > > > setup_requires = setuptools-rust > > python_requires = >=3.6 > > Is either of these actually required to build the package? I assume so. `setup.py` imports setuptools_rust, doesn't it?
setup.py indeed imports setuptools_rust but it is guaranteed to be installed as it is part of pyproject.toml [build-system] requires.
Spec URL: https://lbalhar.fedorapeople.org/python-y-py.spec SRPM URL: https://lbalhar.fedorapeople.org/python-y-py-0.5.5-1.fc37.src.rpm I have applied all the suggestions and it doesn't build now - the problem is with the debuginfo package: Provides: python-y-py = 0.5.5-1.fc38 python3-y-py = 0.5.5-1.fc38 python3-y-py(x86-64) = 0.5.5-1.fc38 python3.11-y-py = 0.5.5-1.fc38 python3.11dist(y-py) = 0.5.5 python3dist(y-py) = 0.5.5 Requires(rpmlib): rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(FileDigests) <= 4.6.0-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 Requires: ld-linux-x86-64.so.2()(64bit) ld-linux-x86-64.so.2(GLIBC_2.3)(64bit) libc.so.6()(64bit) libc.so.6(GLIBC_2.14)(64bit) libc.so.6(GLIBC_2.17)(64bit) libc.so.6(GLIBC_2.18)(64bit) libc.so.6(GLIBC_2.2.5)(64bit) libc.so.6(GLIBC_2.25)(64bit) libc.so.6(GLIBC_2.28)(64bit) libc.so.6(GLIBC_2.3)(64bit) libc.so.6(GLIBC_2.3.2)(64bit) libc.so.6(GLIBC_2.3.4)(64bit) libc.so.6(GLIBC_2.33)(64bit) libc.so.6(GLIBC_2.34)(64bit) libgcc_s.so.1()(64bit) libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3)(64bit) libgcc_s.so.1(GCC_4.2.0)(64bit) libm.so.6()(64bit) libm.so.6(GLIBC_2.2.5)(64bit) python(abi) = 3.11 rtld(GNU_HASH) Processing files: python-y-py-debugsource-0.5.5-1.fc38.x86_64 error: Empty %files file /builddir/build/BUILD/y_py-0.5.5/debugsourcefiles.list RPM build errors: Empty %files file /builddir/build/BUILD/y_py-0.5.5/debugsourcefiles.list Finish: rpmbuild python-y-py-0.5.5-1.fc38.src.rpm Finish: build phase for python-y-py-0.5.5-1.fc38.src.rpm The last build can be found here: https://copr.fedorainfracloud.org/coprs/lbalhar/notebook/build/5310880/ I cannot remember why I added the %global definition there so I looked at the other rust packages and it seems that a lot of them have it as well, I've found 1780 of them. The reason might be that rust2rpm has it in the template: https://pagure.io/fedora-rust/rust2rpm/blob/main/f/rust2rpm/templates/main.spec#_6 Although I'm not sure what the condition around that macro in the template means.
Created attachment 1940623 [details] The .spec file difference from Copr build 5308776 to 5310899
Copr build: https://copr.fedorainfracloud.org/coprs/build/5310899 (failed) Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2161518-python-y-py/fedora-rawhide-x86_64/05310899-python-y-py/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
(In reply to Lumír Balhar from comment #15) > Spec URL: https://lbalhar.fedorapeople.org/python-y-py.spec > SRPM URL: https://lbalhar.fedorapeople.org/python-y-py-0.5.5-1.fc37.src.rpm > > I have applied all the suggestions and it doesn't build now - the problem is > with the debuginfo package: > > Provides: python-y-py = 0.5.5-1.fc38 python3-y-py = 0.5.5-1.fc38 > python3-y-py(x86-64) = 0.5.5-1.fc38 python3.11-y-py = 0.5.5-1.fc38 > python3.11dist(y-py) = 0.5.5 python3dist(y-py) = 0.5.5 > Requires(rpmlib): rpmlib(CompressedFileNames) <= 3.0.4-1 rpmlib(FileDigests) > <= 4.6.0-1 rpmlib(PayloadFilesHavePrefix) <= 4.0-1 > Requires: ld-linux-x86-64.so.2()(64bit) > ld-linux-x86-64.so.2(GLIBC_2.3)(64bit) libc.so.6()(64bit) > libc.so.6(GLIBC_2.14)(64bit) libc.so.6(GLIBC_2.17)(64bit) > libc.so.6(GLIBC_2.18)(64bit) libc.so.6(GLIBC_2.2.5)(64bit) > libc.so.6(GLIBC_2.25)(64bit) libc.so.6(GLIBC_2.28)(64bit) > libc.so.6(GLIBC_2.3)(64bit) libc.so.6(GLIBC_2.3.2)(64bit) > libc.so.6(GLIBC_2.3.4)(64bit) libc.so.6(GLIBC_2.33)(64bit) > libc.so.6(GLIBC_2.34)(64bit) libgcc_s.so.1()(64bit) > libgcc_s.so.1(GCC_3.0)(64bit) libgcc_s.so.1(GCC_3.3)(64bit) > libgcc_s.so.1(GCC_4.2.0)(64bit) libm.so.6()(64bit) > libm.so.6(GLIBC_2.2.5)(64bit) python(abi) = 3.11 rtld(GNU_HASH) > Processing files: python-y-py-debugsource-0.5.5-1.fc38.x86_64 > error: Empty %files file > /builddir/build/BUILD/y_py-0.5.5/debugsourcefiles.list > > RPM build errors: > Empty %files file /builddir/build/BUILD/y_py-0.5.5/debugsourcefiles.list > Finish: rpmbuild python-y-py-0.5.5-1.fc38.src.rpm > Finish: build phase for python-y-py-0.5.5-1.fc38.src.rpm This looks like debuginfo is indeed not valid / not present. Looking at the build log, the package doesn't use default Rust compiler flags, either (i.e. all `%build_rustflags` are absent from rustc calls in the log). Maybe there's an issue with how you ported the package from maturin to setuptools_rust that makes it ignore the flags set in ".cargo/config"? > The last build can be found here: > https://copr.fedorainfracloud.org/coprs/lbalhar/notebook/build/5310880/ > > I cannot remember why I added the %global definition there so I looked at > the other rust packages and it seems that a lot of them have it as well, > I've found 1780 of them. The reason might be that rust2rpm has it in the > template: > https://pagure.io/fedora-rust/rust2rpm/blob/main/f/rust2rpm/templates/main. > spec#_6 > Although I'm not sure what the condition around that macro in the template > means. It means that source-only packages don't get debuginfo. All packages that ship binaries *do* get debuginfo.
Thank you for your help! It seems that there is a way how to set the flags for rustc directly via RustExtension class from setuptools_rust. I did that and it works now. The way I did it is not pretty: setup( rust_extensions=[RustExtension("y_py", args=["--offline"], rustc_flags="%build_rustflags".split())], ) But it works now. I'll try to find a better way. Spec URL: https://lbalhar.fedorapeople.org/python-y-py.spec SRPM URL: https://lbalhar.fedorapeople.org/python-y-py-0.5.5-1.fc37.src.rpm
Maybe we need to adjust %set_build_flagsd and/or Python build macros to set some environment variables with Rust flags?
A better way is to use env variable RUSTFLAGS. It works as well and makes it possible to move the source files out of the spec eventually. Spec URL: https://lbalhar.fedorapeople.org/python-y-py.spec SRPM URL: https://lbalhar.fedorapeople.org/python-y-py-0.5.5-1.fc37.src.rpm
Copr build: https://copr.fedorainfracloud.org/coprs/build/5311511 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2161518-python-y-py/fedora-rawhide-x86_64/05311511-python-y-py/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
Created attachment 1940626 [details] The .spec file difference from Copr build 5311511 to 5311523
Copr build: https://copr.fedorainfracloud.org/coprs/build/5311523 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2161518-python-y-py/fedora-rawhide-x86_64/05311523-python-y-py/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
Does setuptools_rust set the RUSTFLAGS environment variable to an empty string by any chance? I have encountered a similar problem with maturin before, so it's possible that setuptools_rust has a similar bug.
It seems that setuptools_rust combines the RUSTFLAGS env variable with rustflags argument, see https://github.com/PyO3/setuptools-rust/pull/103 So it should not matter which one you use.
I see the problem. If RUSTFLAGS is not set yet, it sets it to an empty string, which seems to override the .cargo/config file generated by %cargo_prep. I wonder why this isn't a problem for python-cryptography ...
Meh, I feel stupid now. I forgot that we no longer use .cargo/config for setting RUSTFLAGS, but set them in the %cargo_build / %cargo_test macro. You should be able to achieve the same thing by doing export RUSTFLAGS="%build_rustflags" in %build (and whereever else required). That should also fix the missing debuginfo problem.
I already figured it out, see comment 21 here. And thank you!
Argh. Sorry for the confusion. I'm not at my best today :( Turns out it still was a good idea to look into this, because python-cryptography is also broken right now ... and I submitted a PR with the RUSTFLAGS fix: https://src.fedoraproject.org/rpms/python-cryptography/pull-request/20
Fabio, should we add RUSTFLAGS="%build_rustflags" to %py3_build and %pyproject_wheel? Should we add it to %set_build_flags? (Not a blocker for this review.)
Package APPROVED. ============================ rpmlint session starts ============================ rpmlint: 2.4.0 configuration: /usr/lib/python3.11/site-packages/rpmlint/configdefaults.toml /etc/xdg/rpmlint/fedora-legacy-licenses.toml /etc/xdg/rpmlint/fedora-spdx-licenses.toml /etc/xdg/rpmlint/fedora.toml /etc/xdg/rpmlint/scoring.toml /etc/xdg/rpmlint/users-groups.toml /etc/xdg/rpmlint/warn-on-functions.toml checks: 31, packages: 2 2 packages and 0 specfiles checked; 0 errors, 0 warnings, 0 badness; has taken 0.1 s
(In reply to Miro Hrončok from comment #31) > Fabio, should we add RUSTFLAGS="%build_rustflags" to %py3_build and > %pyproject_wheel? Should we add it to %set_build_flags? (Not a blocker for > this review.) Probably to %set_build_flags ... more and more non-Rust projects are starting to include Rust code, so that's likely the best place for it. That would mean the definition would need to move from rust-packaging to redhat-rpm-config? Side note: You'll probably want to fix the License tag of the python3-y-py subpackage, as well. It contains a statically linked Rust binary, with components under many different licenses. Most dependencies are either MIT or "MIT OR Apache-2.0", but notably pyo3 itself is Apache-2.0 only, so just having python3-y-py subpackage inherit "MIT" is wrong. (And there's likely components under other licenses involved, as well.) You should be able to use the %cargo_license macro to generate a list of statically linked components and their licenses (or %cargo_license_summary prints only the license summary, which is more useful for determining the License tag). See https://src.fedoraproject.org/rpms/rust-rpm-sequoia/blob/rawhide/f/rust-rpm-sequoia.spec#_64 for example usage in a Rust package that will be installed on every Fedora 38 system. :)
> That would mean the definition would need to move from rust-packaging to redhat-rpm-config? Not necessarily. It can be included as %{?build_rustflags:RUSTFLAGS="%build_rustflags"}. ---------------- > You'll probably want to fix the License tag of the python3-y-py subpackage, as well. It contains a statically linked Rust binary, with components under many different licenses. Oh my, I completely missed this. Sorry. I withdraw my approval for now, this needs to be fixed.
The relevant guidelines are: https://docs.fedoraproject.org/en-US/packaging-guidelines/Rust/#_license_for_binary_packages -> https://docs.fedoraproject.org/en-US/legal/license-field/#_rust_packages
Thanks Fabio. The macros really make that job easier. Do you think that they should be mentioned somewhere in the docs? Now, the specfile contains the correct License tag with a comment describing where it comes from and the RPM contains the generated file. Spec URL: https://lbalhar.fedorapeople.org/python-y-py.spec SRPM URL: https://lbalhar.fedorapeople.org/python-y-py-0.5.5-1.fc37.src.rpm
Created attachment 1940707 [details] The .spec file difference from Copr build 5311523 to 5322079
Copr build: https://copr.fedorainfracloud.org/coprs/build/5322079 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2161518-python-y-py/fedora-rawhide-x86_64/05322079-python-y-py/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
The comment above the License tag does not explicitly mention under what license is this very Python project. I find that a bit confusing for the reader, as the License tga now looks like a sum of the Rust licenses only. --- Just to verify. A naïve sum of the licenses is: MIT AND (MIT OR Apache-2.0) AND Apache-2.0 If I always choose MIT int he OR situation, the sum is: MIT AND MIT AND Apache-2.0 -> MIT AND Apache-2.0 If I always choose Apache-2.0: MIT AND Apache-2.0 AND Apache-2.0 -> MIT AND Apache-2.0 If I choose arbitrary combination: MIT (AND MIT)* (AND Apache-2.0)* AND Apache-2.0 -> MIT AND Apache-2.0 So the combined tag is technically correct. However: """ The license expression must reflect the disjunctive license choice even if one or both of the license identifiers in the OR expression also appear separately in the composite license expression. ... License: (GPL-3.0-or-later OR MPL-1.1) AND GPL-3.0-or-later AND MIT Here we repeat GPL-3.0-or-later because for one binary component it appears as part of an OR subexpression. That is, OR expressions must be treated as though they were a single distinct license. """ https://docs.fedoraproject.org/en-US/legal/license-field/#_combined_disjunctive_and_conjunctive_license_expressions Hence, I believe the License tag MUST be: License: MIT AND (MIT OR Apache-2.0) AND Apache-2.0
(In reply to Lumír Balhar from comment #36) > Thanks Fabio. The macros really make that job easier. Do you think that they > should be mentioned somewhere in the docs? The implementation of these macros is pretty new, it was only added with the most recent version of rust2rpm (i.e. v23). I wanted to wait for a bit of real-world usage to iron out any kinks before documenting them (and their limitations), but at this point I think they're OK. I'll put documenting them in the Rust Packaging Guidelines on my TODO list. > Hence, I believe the License tag MUST be: > License: MIT AND (MIT OR Apache-2.0) AND Apache-2.0 According to my understanding of the new Legal Guidelines, yes. It doesn't make a ton of sense to list them like that (because, as you pointed out, any combination of these will just result in "MIT AND Apache-2.0"), but IANAL.
(In reply to Miro Hrončok from comment #39) > Hence, I believe the License tag MUST be: > > License: MIT AND (MIT OR Apache-2.0) AND Apache-2.0 Done. Spec URL: https://lbalhar.fedorapeople.org/python-y-py.spec SRPM URL: https://lbalhar.fedorapeople.org/python-y-py-0.5.5-1.fc37.src.rpm
Created attachment 1940798 [details] The .spec file difference from Copr build 5322079 to 5333586
Copr build: https://copr.fedorainfracloud.org/coprs/build/5333586 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2161518-python-y-py/fedora-rawhide-x86_64/05333586-python-y-py/fedora-review/review.txt Please take a look if any issues were found. --- This comment was created by the fedora-review-service https://github.com/FrostyX/fedora-review-service
Could you please add this to the beginning of the License comment? # python-y-py code is MIT Thanks. Anyway, the package is APPROVED.
The Pagure repository was created at https://src.fedoraproject.org/rpms/python-y-py
$ koji wait-repo f38-build --build python-y-py-0.5.5-1.fc38 Successfully waited 0:00 for python-y-py-0.5.5-1.fc38 to appear in the f38-build repo