https://bugzilla.redhat.com/show_bug.cgi?id=2292522
Bug ID: 2292522 Summary: Review Request: rust-zune-jpeg - Fast, correct and safe jpeg decoder Product: Fedora Version: rawhide Status: NEW Component: Package Review Assignee: nobody@fedoraproject.org Reporter: decathorpe@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://decathorpe.fedorapeople.org/rust-zune-jpeg.spec SRPM URL: https://decathorpe.fedorapeople.org/rust-zune-jpeg-0.4.11-1.fc40.src.rpm
Description: A fast, correct and safe jpeg decoder.
Fedora Account System Username: decathorpe
https://bugzilla.redhat.com/show_bug.cgi?id=2292522
--- Comment #1 from Fabio Valentini decathorpe@gmail.com --- This package built on koji: https://koji.fedoraproject.org/koji/taskinfo?taskID=119071015
https://bugzilla.redhat.com/show_bug.cgi?id=2292522
Fedora Review Service fedora-review-bot@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |AutomationTriaged URL| |https://crates.io/crates/zu | |ne-jpeg
--- Comment #2 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/7617617 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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
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=2292522
wojnilowicz lukasz.wojnilowicz@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Assignee|nobody@fedoraproject.org |lukasz.wojnilowicz@gmail.co | |m Flags| |fedora-review? CC| |lukasz.wojnilowicz@gmail.co | |m Status|NEW |ASSIGNED
--- Comment #3 from wojnilowicz lukasz.wojnilowicz@gmail.com --- 1) Shouldn't the following subpackage:
%package -n %{name}+x86-devel Summary: %{summary} BuildArch: noarch
be somehow restricted to x86_64, and not be noarch? At https://github.com/etemesi254/zune-image/blob/dev/crates/zune-jpeg/README.md... it says that this "Enables x86 specific instructions"
2) x86 requires x86_64-v2, x86_64-v3. Wouldn't that be an issue for x86_64-v1 when used in an application using this as a dependency?
[fedora-review-service-build]
https://bugzilla.redhat.com/show_bug.cgi?id=2292522
--- Comment #4 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/7689515 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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
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=2292522
--- Comment #5 from Fabio Valentini decathorpe@gmail.com --- (In reply to wojnilowicz from comment #3)
- Shouldn't the following subpackage:
%package -n %{name}+x86-devel Summary: %{summary} BuildArch: noarch
be somehow restricted to x86_64, and not be noarch?
No, this is not possible. The "-devel" package must be noarch and be available on all architectures, otherwise you get broken dependencies somewhere.
We even have special machinery in rust2rpm for crates that only compile on specific architectures (the "supported-arches" setting in rust2rpm.toml) because of this.
At https://github.com/etemesi254/zune-image/blob/dev/crates/zune-jpeg/README. md#crate-features it says that this "Enables x86 specific instructions"
This should not be an issue according to the README:
https://github.com/etemesi254/zune-image/blob/dev/crates/zune-jpeg/README.md...
""" Note that the x86 features are automatically disabled on platforms that aren't x86 during compile time hence there is no need to disable them explicitly if you are targeting such a platform. """
- x86 requires x86_64-v2, x86_64-v3. Wouldn't that be an issue for
x86_64-v1 when used in an application using this as a dependency?
That is a good question. It looks like there is code to guard against using instructions on CPUs where they are not available:
https://github.com/etemesi254/zune-image/blob/dev/crates/zune-core/src/optio...
This should work under most circumstances. The only way this could break (as far as I can tell) would be if 1) zune-jpeg is built without "std" support 2) with "x86" feature enabled *and* 3) the feature in question is available in the environment where the code is compiled but *not* on the machine the code gets executed on.
https://bugzilla.redhat.com/show_bug.cgi?id=2292522
wojnilowicz lukasz.wojnilowicz@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #6 from wojnilowicz lukasz.wojnilowicz@gmail.com --- Package was generated with rust2rpm, simplifying the review.
✅ package builds and installs without errors on rawhide ✅ test suite is run and all unit tests pass ✅ latest version of the crate is packaged ✅ license matches upstream specification and is acceptable for Fedora ✅ license files are included with %license in %files ✅ package complies with Rust Packaging Guidelines
Package APPROVED.
===
Recommended post-import rust-sig tasks:
- set up package on release-monitoring.org: project: $crate homepage: https://crates.io/crates/$crate backend: crates.io version scheme: semantic version filter: alpha;beta;rc;pre distro: Fedora Package: rust-$crate
- add rust-sig with "commit" access as package co-maintainer (should happen automatically)
- set bugzilla assignee overrides to @rust-sig (optional)
- track package in koschei for all built branches (should happen automatically once rust-sig is co-maintainer)
===
Thanks for the explanations. They seem reasonable to me. What about this?
(In reply to Fabio Valentini from comment #5)
(In reply to wojnilowicz from comment #3)
- Shouldn't the following subpackage:
%package -n %{name}+x86-devel Summary: %{summary} BuildArch: noarch
be somehow restricted to x86_64, and not be noarch?
No, this is not possible. The "-devel" package must be noarch and be available on all architectures, otherwise you get broken dependencies somewhere.
We even have special machinery in rust2rpm for crates that only compile on specific architectures (the "supported-arches" setting in rust2rpm.toml) because of this.
How is it different from https://bugzilla.redhat.com/show_bug.cgi?id=2276290 ? Is it because "supported-arches" there would be used for the whole crate, because the crate as a whole wouldn't work, while here it would have to be used only for a subpackage of the crate, because only this subpackage makes no sense? I would assume that's impossible to filter it out from the rpm point of view, however it's still doable to package it friction less thanks to the following note:
""" Note that the x86 features are automatically disabled on platforms that aren't x86 during compile time hence there is no need to disable them explicitly if you are targeting such a platform. """
https://bugzilla.redhat.com/show_bug.cgi?id=2292522
--- Comment #7 from Fabio Valentini decathorpe@gmail.com --- Thank you for the thorough review!
Is it because "supported-arches" there would be used for the whole crate, because the crate as a whole wouldn't work, while here it would have to be used only for a subpackage of the crate, because only this subpackage makes no sense? I would assume that's impossible to filter it out from the rpm point of view, however it's still doable to package it friction less thanks to the following note (...)
Yeah. supported-arches only affects which architectures a crate is built on and where tests are run. It doesn't affect the package contents at all. Packages that have "BuildArch: noarch" also aren't filtered out by architecture (they are supposed to be architecture-independent after all), so there's not much that can be done here.
https://bugzilla.redhat.com/show_bug.cgi?id=2292522
Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |POST
--- Comment #8 from Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org --- The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-zune-jpeg
https://bugzilla.redhat.com/show_bug.cgi?id=2292522
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Fixed In Version| |rust-zune-jpeg-0.4.11-1.fc4 | |1 Resolution|--- |RAWHIDE Status|POST |CLOSED Last Closed| |2024-07-07 18:01:32
--- Comment #9 from Fabio Valentini decathorpe@gmail.com --- Imported and built: https://bodhi.fedoraproject.org/updates/FEDORA-2024-70b523927a
package-review@lists.fedoraproject.org