https://bugzilla.redhat.com/show_bug.cgi?id=2313784
Bug ID: 2313784 Summary: Review Request: python-jiter - Fast iterable JSON parser Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: mhayden@redhat.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://download.copr.fedorainfracloud.org/results/mhayden/shell_gpt/fedora-... SRPM URL: https://download.copr.fedorainfracloud.org/results/mhayden/shell_gpt/fedora-... Description: Fast iterable JSON parser Fedora Account System Username: mhayden
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
Fedora Review Service fedora-review-bot@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- URL| |https://github.com/pydantic | |/jiter/
--- Comment #1 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8043692 (failed)
Build log: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |decathorpe@gmail.com
--- Comment #2 from Fabio Valentini decathorpe@gmail.com --- These are wrong (they don't specify *which version* of these dependencies you need, so it will just pull in the latest one, whether that's the one you want or not), and they should also not be needed:
BuildRequires: rust-pyo3-devel BuildRequires: rust-bencher-devel BuildRequires: rust-codspeed-bencher-compat-devel BuildRequires: rust-indexmap-devel BuildRequires: rust-paste-devel
When using `%cargo_generate_buildrequires`, these should get pulled in automatically.
If you're getting error messages about them being missing, you need to ensure that they're pulled *first* so they are found when the Python dep generator is run.
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|nobody@fedoraproject.org |decathorpe@gmail.com
--- Comment #3 from Fabio Valentini decathorpe@gmail.com --- I'm also pretty sure that you don't need to package codspeed. It's only used in benchmark code, and benchmarks aren't compiled *or* run during package builds.
Note that the `jiter` Rust package is already in Fedora: https://src.fedoraproject.org/rpms/rust-jiter/
You might want to depend on that instead of bundling a different copy in this "Python" package.
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #4 from Major Hayden 🤠 mhayden@redhat.com --- Thanks, Fabio! So I have two main problems:
1) It seems like the Python dependency generator is always running first. 🤔 2) I'm a rust novice, but it *looks* like upstream forces the Python build to build the jiter crate each time:
https://github.com/pydantic/jiter/blob/main/crates/jiter-python/Cargo.toml#L...
[dependencies] pyo3 = { workspace = true, features = ["num-bigint"] } jiter = { path = "../jiter", features = ["python", "num-bigint"] }
Is there a way I can patch this to use the jiter from Fedora's rust-jiter-devel or rust-jiter+python-devel? No matter what I try, cargo want sto download stuff from index.crates.io during the build.
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #5 from Fabio Valentini decathorpe@gmail.com --- Regarding 1)
%generate_buildrequires %pyproject_buildrequires %cargo_generate_buildrequires
In this case, the Python dependency generator runs first because ... it is first in the script. 🥇 If switching these two around isn't enough, then we'll need to figure out something else. I seem to remember having seen this issue in some place before, but I can't remember details right now.
Regerding 2)
I think replacing this line:
jiter = { path = "../jiter", features = ["python", "num-bigint"] }
with this one:
jiter = { version = "0.5.0", features = ["python", "num-bigint"] }
should work to use the jiter sources from the rust-jiter package instead of the ones bundled here.
Keep in mind that this will likely tie the update cycle of this package with the one of rust-jiter, since they're published together from https://github.com/pydantic/jiter/, as far as I can tell.
If you want to keep things independent, building *without* adding a dependency on rust-jiter might make things more flexible. In this case, I would recommend taking the patches from rust-jiter to avoid the codspeed dependency.
https://bugzilla.redhat.com/show_bug.cgi?id=2313784 Bug 2313784 depends on bug 2313782, which changed state.
Bug 2313782 Summary: Review Request: rust-codspeed-bencher-compat - Bencher compatibility layer for CodSpeed https://bugzilla.redhat.com/show_bug.cgi?id=2313782
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Resolution|--- |WONTFIX
https://bugzilla.redhat.com/show_bug.cgi?id=2313784 Bug 2313784 depends on bug 2313781, which changed state.
Bug 2313781 Summary: Review Request: rust-codspeed - Core instrumentation library for CodSpeed https://bugzilla.redhat.com/show_bug.cgi?id=2313781
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |CLOSED Resolution|--- |WONTFIX
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #6 from Major Hayden 🤠 mhayden@redhat.com --- Thank you, Fabio. I've updated the RPM.
Spec URL: https://download.copr.fedorainfracloud.org/results/mhayden/shell_gpt/fedora-... SRPM URL: https://download.copr.fedorainfracloud.org/results/mhayden/shell_gpt/fedora-...
Koji scratch build: https://koji.fedoraproject.org/koji/taskinfo?taskID=123853162
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #7 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2048365 --> https://bugzilla.redhat.com/attachment.cgi?id=2048365&action=edit The .spec file difference from Copr build 8043692 to 8062714
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #8 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8062714 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Found issues:
- python3-pytest7 is deprecated, you must not depend on it. Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/deprecating-packag... - Upstream MD5sum check error, diff is in /var/lib/copr-rpmbuild/results/python-jiter/diff.txt Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #9 from Fabio Valentini decathorpe@gmail.com --- Thanks for the update - looks pretty good already! There's just some minor things left, I think:
# For included rust code BuildRequires: rust-packaging >= 25
I don't know where you got this from, but it's ... weird. rust-packaging no longer exists, and is only provided for backwards compatibility since version 24.
Please replace this BuildRequires "cargo-rpm-macros". Not sure why you'd need version >= 25, or if >= 24 would do.
# Upstream does not provide a license file inside the PyPi package. Source1: https://github.com/pydantic/jiter/blob/main/LICENSE
Hm, the tarball *does* contain a license text, but the directory layout in it is weirdly different from the one in the upstream project. I think in your last version you copied the one from the crates/jiter subdirectory? It's a copy of the one in the root of the project's repository, so it *should* also be applicable to the Python module, and you wouldn't need to include it separately here.
export RUSTFLAGS="%build_rustflags"
This is no longer necessary on Fedora >= 40. So if you don't intend to build this package on Fedora 39 (while it's still alive) or EPEL 9, you should be able to drop this. It is set by "%set_build_flags" in Fedora 40+ automatically.
%cargo_license_summary
This is nice and all, but you actually need to take the output of that macro and put it into the spec file, and adjust the License tag of the python3-jiter subpackage accordingly.
- python3-pytest7 is deprecated, you must not depend on it. Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/deprecating-packag...
This looks like a false positive - I only see an unversioned dependency on py3dist(pytest), which should pull in the latest version, and not the v7 compat package?
- Upstream MD5sum check error, diff is in /var/lib/copr-rpmbuild/results/python-jiter/diff.txt
Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/SourceURL/
The URL you provided points at the GitHub HTML view of that file, but not the raw contents.
Replacing https://github.com/pydantic/jiter/blob/main/LICENSE with https://github.com/pydantic/jiter/raw/main/LICENSE
should resolve this issue.
And I would also recommend to replace "main" in that URL with "v0.5.0" to specifically link to the contents of the LICENSE file at the point v0.5.0 was tagged, not the current "HEAD" of the "main" branch.
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #10 from Major Hayden 🤠 mhayden@redhat.com --- Thanks for catching those, Fabio. I went with the LICENSE file in the PyPi package, added the licenses from %cargo_license_summary, fixed the cargo-rpm-macros BuildRequires and dropped the unnecessary RUSTFLAGS.
Spec URL: https://download.copr.fedorainfracloud.org/results/mhayden/shell_gpt/fedora-... SRPM URL: https://download.copr.fedorainfracloud.org/results/mhayden/shell_gpt/fedora-...
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #11 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2048480 --> https://bugzilla.redhat.com/attachment.cgi?id=2048480&action=edit The .spec file difference from Copr build 8062714 to 8065743
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #12 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8065743 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Found issues:
- python3-pytest7 is deprecated, you must not depend on it. Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/deprecating-packag...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
Ben Beasley code@musicinmybrain.net changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |code@musicinmybrain.net
--- Comment #13 from Ben Beasley code@musicinmybrain.net --- Can we remove the bundled jiter crate in %prep to prove we aren’t using it? Something like this should work:
%prep %autosetup -p1 -n %{srcname}-%{version} # There is no top-level LICENSE file, but the one from the bundled jiter crate # is the correct license. mv crates/jiter/LICENSE ./ # However, we want to use the system copy of the jiter crate. rm -r crates/jiter # E.g., for 0.5.0, this would allow 0.5.x. tomcli set crates/jiter-python/Cargo.toml str dependencies.jiter.version "%{version}" tomcli set crates/jiter-python/Cargo.toml del dependencies.jiter.path %cargo_prep
Also, it’s kind of a convention to put the cargo_license_summary output above the License tag – and it looks like the lines from cargo_license_summary have been incorrectly combined. Something like this should be right:
# python-jiter is MIT only, but the rest are rust libraries # based on cargo_license_summary output: # # Apache-2.0 OR MIT # BSD-2-Clause OR Apache-2.0 OR MIT # MIT # MIT OR Apache-2.0 (duplicate) License: %{shrink: (Apache-2.0 OR MIT) AND (BSD-2-Clause OR Apache-2.0 OR MIT) AND MIT }
By the way, you are welcome to add me as a co-maintainer on this package once it’s approved, since I have been doing most of the updates to the rust-jiter package and the two are closely linked.
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #14 from Major Hayden 🤠 mhayden@redhat.com --- Ben -- Just made those changes. I'm happy to add you as a co-maintainer.
Spec URL: https://download.copr.fedorainfracloud.org/results/mhayden/shell_gpt/fedora-... SRPM URL: https://download.copr.fedorainfracloud.org/results/mhayden/shell_gpt/fedora-...
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #15 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2048682 --> https://bugzilla.redhat.com/attachment.cgi?id=2048682&action=edit The .spec file difference from Copr build 8065743 to 8071621
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #16 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8071621 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Found issues:
- python3-pytest7 is deprecated, you must not depend on it. Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/deprecating-packag...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags| |fedora-review?
--- Comment #17 from Fabio Valentini decathorpe@gmail.com --- Thanks Ben for the additional feedback - latest version of the package looks good to me, with one minor exception that might need a clarification:
[?]: Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files in private %_libdir subdirectory (see attachment). Verify they are not in ld path.
This is due to "libjiter_python.so()(64bit)".
I looked at other Python packages that provide native modules, and none of them had RPM provides for their native .so modules - with one exception that I could find, pydantic-core. I'm not sure what the difference is between these two and other "native" Python extension modules, and if RPM should be prevented from generating Provides for this .so file.
===
Rust-specific checklist:
✅ package contains only permissible content ✅ package builds and installs without errors on rawhide ✅ test suite is run and all unit tests pass ✅ license matches upstream specification and is acceptable for Fedora ✅ licenses of statically linked dependencies are correctly taken into account ✅ license file is included with %license in %files ✅ package complies with Rust Packaging Guidelines
===
Generic checklist:
Package Review ==============
Legend: [x] = Pass, [!] = Fail, [-] = Not applicable, [?] = Not evaluated [ ] = Manual review needed
Issues: ======= - Package must not depend on deprecated() packages. Note: python3-pytest7 is deprecated, you must not depend on it. See: https://docs.fedoraproject.org/en-US/packaging- guidelines/deprecating-packages/
-> This is a false positive, because both python3-pytest7 and python3-pytest (non-compat) provide "python3-pytest".
===== MUST items =====
C/C++: [?]: Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files in private %_libdir subdirectory (see attachment). Verify they are not in ld path.
-> Needs to be checked.
Generic: [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. [x]: License file installed when any subpackage combination is installed. [x]: If the package is under multiple licenses, the licensing breakdown must be documented in the spec. [x]: Package must own all directories that it creates. [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. [x]: Useful -debuginfo package or justification otherwise. [x]: Package is not known to require an ExcludeArch tag. [x]: Package complies to the Packaging Guidelines [x]: Package successfully compiles and builds into binary rpms on at least one supported primary architecture. [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]: The License field must be a valid SPDX expression. [x]: Package requires other packages for directories it uses. [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 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]: Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 6748 bytes in 1 files. [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. [-]: 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: [-]: If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. [?]: Final provides and requires are sane (see attachments). [-]: Fully versioned dependency in subpackages if applicable. [?]: Package functions as described. [x]: 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. [x]: Package should compile and build into binary rpms on all supported architectures. [x]: %check is present and all tests pass. [x]: Packages should try to preserve timestamps of original installed files. [x]: Reviewer should test that the package builds in mock. [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). [x]: Large data in /usr/share should live in a noarch subpackage if package is arched. [x]: Spec file according to URL is the same as in SRPM.
Rpmlint ------- Checking: python3-jiter-0.5.0-1.fc42.x86_64.rpm python-jiter-debugsource-0.5.0-1.fc42.x86_64.rpm python-jiter-0.5.0-1.fc42.src.rpm ============================ rpmlint session starts ============================ rpmlint: 2.5.0 configuration: /usr/lib/python3.12/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/tmpghns5cg4')] checks: 32, packages: 3
python-jiter.src: E: spelling-error ('iterable', 'Summary(en_US) iterable -> alterable, tolerable, iterate') python-jiter.src: E: spelling-error ('iterable', '%description -l en_US iterable -> alterable, tolerable, iterate') python3-jiter.x86_64: E: spelling-error ('iterable', 'Summary(en_US) iterable -> alterable, tolerable, iterate') python3-jiter.x86_64: E: spelling-error ('iterable', '%description -l en_US iterable -> alterable, tolerable, iterate') 3 packages and 0 specfiles checked; 4 errors, 0 warnings, 11 filtered, 4 badness; has taken 0.4 s
Rpmlint (installed packages) ---------------------------- ============================ rpmlint session starts ============================ rpmlint: 2.5.0 configuration: /usr/lib/python3.13/site-packages/rpmlint/configdefaults.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: 32, packages: 2
python3-jiter.x86_64: E: spelling-error ('iterable', 'Summary(en_US) iterable -> alterable, tolerable, iterate') python3-jiter.x86_64: E: spelling-error ('iterable', '%description -l en_US iterable -> alterable, tolerable, iterate') 2 packages and 0 specfiles checked; 2 errors, 0 warnings, 7 filtered, 2 badness; has taken 0.1 s
Unversioned so-files -------------------- python3-jiter: /usr/lib64/python3.13/site-packages/jiter/jiter.cpython-313-x86_64-linux-gnu.so
Source checksums ---------------- https://files.pythonhosted.org/packages/source/j/jiter/jiter-0.5.0.tar.gz : CHECKSUM(SHA256) this package : 1d916ba875bcab5c5f7d927df998c4cb694d27dceddf3392e58beaf10563368a CHECKSUM(SHA256) upstream package : 1d916ba875bcab5c5f7d927df998c4cb694d27dceddf3392e58beaf10563368a
Requires -------- python3-jiter (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) rtld(GNU_HASH)
python-jiter-debugsource (rpmlib, GLIBC filtered):
Provides -------- python3-jiter: libjiter_python.so()(64bit) python-jiter python3-jiter python3-jiter(x86-64) python3.13-jiter python3.13dist(jiter) python3dist(jiter)
python-jiter-debugsource: python-jiter-debugsource python-jiter-debugsource(x86-64)
Generated by fedora-review 0.10.0 (e79b66b) last change: 2023-07-24 Command line :/usr/bin/fedora-review -b 2313784 Buildroot used: fedora-rawhide-x86_64 Active plugins: Generic, Shell-api, Python Disabled plugins: fonts, PHP, Perl, SugarActivity, R, Ocaml, C/C++, Java, Haskell Disabled flags: EXARCH, EPEL6, EPEL7, DISTTAG, BATCH
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #18 from Ben Beasley code@musicinmybrain.net --- (In reply to Fabio Valentini from comment #17)
Thanks Ben for the additional feedback - latest version of the package looks good to me, with one minor exception that might need a clarification:
[?]: Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files in private %_libdir subdirectory (see attachment). Verify they are not in ld path.
This is due to "libjiter_python.so()(64bit)".
I looked at other Python packages that provide native modules, and none of them had RPM provides for their native .so modules - with one exception that I could find, pydantic-core. I'm not sure what the difference is between these two and other "native" Python extension modules, and if RPM should be prevented from generating Provides for this .so file.
After quite a bit of group investigation in the Fedora Python Matrix channel, we’ve concluded that this changed in Rust 1.81, and Fabio filed bug 2314879.
I recommend adding the following workaround:
# The Python extension module now gets a SONAME of libjiter_python.so; we # must ensure it is not used to generate automatic Provides. See: # Rust 1.81+ implicitly / automatically sets soname on cdylib targets # https://bugzilla.redhat.com/show_bug.cgi?id=2314879 # https://docs.fedoraproject.org/en-US/packaging-guidelines/AutoProvidesAndReq... %global __provides_exclude ^lib_jiter_python\.so.*$
I’m working on adding similar workarounds to other Rust-based Python extensions, which will all be affected once they are rebuilt with Rust 1.81.
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #19 from Major Hayden 🤠 mhayden@redhat.com --- Done!
Spec URL: https://download.copr.fedorainfracloud.org/results/mhayden/shell_gpt/fedora-... SRPM URL: https://download.copr.fedorainfracloud.org/results/mhayden/shell_gpt/fedora-...
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #20 from Major Hayden 🤠 mhayden@redhat.com --- Actually, I think that excludes might have an extra underscore as I spotted the .so in the provides output. I'll get another build going.
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #21 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2048945 --> https://bugzilla.redhat.com/attachment.cgi?id=2048945&action=edit The .spec file difference from Copr build 8071621 to 8074826
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #22 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8074826 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Found issues:
- python3-pytest7 is deprecated, you must not depend on it. Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/deprecating-packag...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #23 from Major Hayden 🤠 mhayden@redhat.com --- Fixed the provides regex:
Spec URL: https://download.copr.fedorainfracloud.org/results/mhayden/shell_gpt/fedora-... SRPM URL: https://download.copr.fedorainfracloud.org/results/mhayden/shell_gpt/fedora-...
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #24 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2048946 --> https://bugzilla.redhat.com/attachment.cgi?id=2048946&action=edit The .spec file difference from Copr build 8074826 to 8074835
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #25 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/8074835 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
Found issues:
- python3-pytest7 is deprecated, you must not depend on it. Read more: https://docs.fedoraproject.org/en-US/packaging-guidelines/deprecating-packag...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #26 from Ben Beasley code@musicinmybrain.net --- (In reply to Major Hayden 🤠 from comment #23)
Fixed the provides regex:
That was my typo (%global __provides_exclude ^lib_jiter_python\.so.*$ should have been %global __provides_exclude ^libjiter_python\.so.*$) – sorry about that!
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #27 from Major Hayden 🤠 mhayden@redhat.com --- No problem, Ben! If you only saw the quantity of typos I make every day... 😅
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+ Status|ASSIGNED |POST
--- Comment #28 from Fabio Valentini decathorpe@gmail.com --- Thank you both, the package now looks good to me!
Package Review ==============
Legend: ✅ = Pass, ⛔ = Fail, ⭕ = Not applicable, ❓ = Not evaluated
Issues: ======= ⛔ Package must not depend on deprecated() packages. Note: python3-pytest7 is deprecated, you must not depend on it. See: https://docs.fedoraproject.org/en-US/packaging- guidelines/deprecating-packages/
➡️ This is a false positive. The package depends on "python3dist(pytest)", which is provided by both python3-pytest and python3-pytest7, but it pulls in the non-deprecated version.
===== MUST items =====
C/C++: ✅ Development (unversioned) .so files in -devel subpackage, if present. Note: Unversioned so-files in private %_libdir subdirectory (see attachment). Verify they are not in ld path.
➡️ This is normal and expected for a native Python extensions. While this file *does* have an SONAME ELF header (caused by a change in Rust 1.81), the RPM Provides for this shared library are correctly filtered out.
Generic: ✅ Package is licensed with an open-source compatible license and meets other legal requirements as defined in the legal section of Packaging Guidelines. ✅ License field in the package spec file matches the actual license. ✅ License file installed when any subpackage combination is installed. ✅ If the package is under multiple licenses, the licensing breakdown must be documented in the spec. ✅ Package must own all directories that it creates. ✅ %build honors applicable compiler flags or justifies otherwise. ✅ Package contains no bundled libraries without FPC exception. ✅ Changelog in prescribed format. ✅ 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 ✅ Package uses nothing in %doc for runtime. ✅ Package consistently uses macros (instead of hard-coded directory names). ✅ Package is named according to the Package Naming Guidelines. ✅ Package does not generate any conflict. ✅ Package obeys FHS, except libexecdir and /usr/target. ⭕ If the package is a rename of another package, proper Obsoletes and Provides are present. ✅ Requires correct, justified where necessary. ✅ 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. ✅ Package complies to the Packaging Guidelines ✅ Package successfully compiles and builds into binary rpms on at least one supported primary architecture. ✅ Package installs properly. ✅ Rpmlint is run on all rpms the build produces. Note: There are rpmlint messages (see attachment). ✅ 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. ✅ The License field must be a valid SPDX expression. ✅ Package requires other packages for directories it uses. ✅ Package does not own files or directories owned by other packages. ✅ Package uses either %{buildroot} or $RPM_BUILD_ROOT ✅ Package does not run rm -rf %{buildroot} (or $RPM_BUILD_ROOT) at the beginning of %install. ✅ Macros in Summary, %description expandable at SRPM build time. ✅ Dist tag is present. ✅ Package does not contain duplicates in %files. ✅ Permissions on files are set properly. ✅ Package use %makeinstall only when make install DESTDIR=... doesn't work. ✅ Package is named using only allowed ASCII characters. ✅ Package does not use a name that already exists. ✅ Package is not relocatable. ✅ Sources used to build the package match the upstream source, as provided in the spec URL. ✅ Spec file name must match the spec package %{name}, in the format %{name}.spec. ✅ File names are valid UTF-8. ✅ Large documentation must go in a -doc subpackage. Large could be size (~1MB) or number of files. Note: Documentation size is 6748 bytes in 1 files. ✅ Packages must not store files under /srv, /opt or /usr/local
Python: ✅ Python eggs must not download any dependencies during the build process. ⭕ A package which is used by another package via an egg interface should provide egg info. ✅ Package meets the Packaging Guidelines::Python ✅ Package contains BR: python2-devel or python3-devel ✅ 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. ✅ Python packages must not contain %{pythonX_site(lib|arch)}/* in %files ✅ Binary eggs must be removed in %prep
===== SHOULD items =====
Generic: ⭕ If the source package does not include license text(s) as a separate file from upstream, the packager SHOULD query upstream to include it. ✅ Final provides and requires are sane (see attachments). ⭕ Fully versioned dependency in subpackages if applicable. Note: No Requires: %{name}%{?_isa} = %{version}-%{release} in python3-jiter ❓ Package functions as described. ✅ Latest version is packaged. ✅ Package does not include license text files separate from upstream. ⭕ Sources are verified with gpgverify first in %prep if upstream publishes signatures. ✅ Package should compile and build into binary rpms on all supported architectures. ✅ %check is present and all tests pass. ✅ Packages should try to preserve timestamps of original installed files. ✅ Reviewer should test that the package builds in mock. ✅ Buildroot is not present ✅ Package has no %clean section with rm -rf %{buildroot} (or $RPM_BUILD_ROOT) ✅ No file requires outside of /etc, /bin, /sbin, /usr/bin, /usr/sbin. ✅ Packager, Vendor, PreReq, Copyright tags should not be in spec file ✅ Sources can be downloaded from URI in Source: tag ✅ SourceX is a working URL. ✅ Spec use %global instead of %define unless justified.
===== EXTRA items =====
Generic: ✅ 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. ✅ Spec file according to URL is the same as in SRPM.
Rpmlint ------- Checking: python3-jiter-0.5.0-1.fc42.x86_64.rpm python-jiter-debugsource-0.5.0-1.fc42.x86_64.rpm python-jiter-0.5.0-1.fc42.src.rpm ============================ rpmlint session starts ============================ rpmlint: 2.5.0 configuration: /usr/lib/python3.12/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/tmpsz7l8sxk')] checks: 32, packages: 3
python-jiter.src: E: spelling-error ('iterable', 'Summary(en_US) iterable -> alterable, tolerable, iterate') python-jiter.src: E: spelling-error ('iterable', '%description -l en_US iterable -> alterable, tolerable, iterate') python3-jiter.x86_64: E: spelling-error ('iterable', 'Summary(en_US) iterable -> alterable, tolerable, iterate') python3-jiter.x86_64: E: spelling-error ('iterable', '%description -l en_US iterable -> alterable, tolerable, iterate') 3 packages and 0 specfiles checked; 4 errors, 0 warnings, 11 filtered, 4 badness; has taken 0.5 s
➡️ This is a false positive, "iterable" is the correct spelling of the technical term.
Rpmlint (installed packages) ---------------------------- ============================ rpmlint session starts ============================ rpmlint: 2.5.0 configuration: /usr/lib/python3.13/site-packages/rpmlint/configdefaults.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: 32, packages: 2
python3-jiter.x86_64: E: spelling-error ('iterable', 'Summary(en_US) iterable -> alterable, tolerable, iterate') python3-jiter.x86_64: E: spelling-error ('iterable', '%description -l en_US iterable -> alterable, tolerable, iterate') 2 packages and 0 specfiles checked; 2 errors, 0 warnings, 7 filtered, 2 badness; has taken 0.1 s
➡️ This is a false positive, "iterable" is the correct spelling of the technical term.
Unversioned so-files -------------------- python3-jiter: /usr/lib64/python3.13/site-packages/jiter/jiter.cpython-313-x86_64-linux-gnu.so
➡️ This is OK and expected for native Python extensions.
Source checksums ---------------- https://files.pythonhosted.org/packages/source/j/jiter/jiter-0.5.0.tar.gz : CHECKSUM(SHA256) this package : 1d916ba875bcab5c5f7d927df998c4cb694d27dceddf3392e58beaf10563368a CHECKSUM(SHA256) upstream package : 1d916ba875bcab5c5f7d927df998c4cb694d27dceddf3392e58beaf10563368a
Requires -------- python3-jiter (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) rtld(GNU_HASH)
python-jiter-debugsource (rpmlib, GLIBC filtered):
Provides -------- python3-jiter: python-jiter python3-jiter python3-jiter(x86-64) python3.13-jiter python3.13dist(jiter) python3dist(jiter)
python-jiter-debugsource: python-jiter-debugsource python-jiter-debugsource(x86-64)
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #29 from Major Hayden 🤠 mhayden@redhat.com --- Thank you, Fabio and Ben! 💙
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |RELEASE_PENDING
--- Comment #30 from Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org --- The Pagure repository was created at https://src.fedoraproject.org/rpms/python-jiter
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RELEASE_PENDING |MODIFIED
--- Comment #31 from Fedora Update System updates@fedoraproject.org --- FEDORA-2024-8faa4962bc (python-jiter-0.5.0-1.fc42) has been submitted as an update to Fedora 42. https://bodhi.fedoraproject.org/updates/FEDORA-2024-8faa4962bc
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA Last Closed| |2024-09-30 18:41:52
--- Comment #32 from Fedora Update System updates@fedoraproject.org --- FEDORA-2024-8faa4962bc (python-jiter-0.5.0-1.fc42) has been pushed to the Fedora 42 stable repository. If problem still persists, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #33 from Fedora Update System updates@fedoraproject.org --- FEDORA-2024-3f4d4c0d43 (python-fastapi-0.115.5-3.fc41, python-jiter-0.7.1-1.fc41, and 5 more) has been submitted as an update to Fedora 41. https://bodhi.fedoraproject.org/updates/FEDORA-2024-3f4d4c0d43
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #34 from Fedora Update System updates@fedoraproject.org --- FEDORA-2024-3f4d4c0d43 has been pushed to the Fedora 41 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2024-3f4d4c0d43` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2024-3f4d4c0d43
See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #35 from Fedora Update System updates@fedoraproject.org --- FEDORA-2024-3f4d4c0d43 has been pushed to the Fedora 41 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf upgrade --enablerepo=updates-testing --refresh --advisory=FEDORA-2024-3f4d4c0d43` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2024-3f4d4c0d43
See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
https://bugzilla.redhat.com/show_bug.cgi?id=2313784
--- Comment #36 from Fedora Update System updates@fedoraproject.org --- FEDORA-2024-3f4d4c0d43 (python-fastapi-0.115.5-3.fc41, python-jiter-0.7.1-1.fc41, and 5 more) has been pushed to the Fedora 41 stable repository. If problem still persists, please make note of it in this bug report.
package-review@lists.fedoraproject.org
-
bugzilla＠redhat.com