Bug 1981110 (rust-cpufeatures)

Summary: Review Request: rust-cpufeatures - No-std compatible alternative to the is_x86_feature_detected! macro
Product: [Fedora] Fedora Reporter: Robert-André Mauchin 🐧 <zebob.m>
Component: Package ReviewAssignee: Fabio Valentini <decathorpe>
Status: CLOSED ERRATA QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: rawhideCC: decathorpe, package-review
Target Milestone: ---Flags: decathorpe: fedora-review+
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2021-07-27 21:45:42 UTC Type: ---
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1957380    

Description Robert-André Mauchin 🐧 2021-07-11 14:11:15 UTC
Spec URL: https://eclipseo.fedorapeople.org/for-review/rust-cpufeatures.spec
SRPM URL: https://eclipseo.fedorapeople.org/for-review/rust-cpufeatures-0.1.5-1.fc35.src.rpm

Description:
Lightweight and efficient no-std compatible alternative to the is_x86_feature_detected! macro.

Fedora Account System Username: eclipseo

Comment 1 Fabio Valentini 2021-07-19 09:04:21 UTC
Taking this review.

Looks like you'll need to add something like "ExclusiveArch: x86_64 aarch64".
The crate does not compile on other architectures, and should also not be used on other architectures.

Alternatively, you could do something like what was done in the rust-cpuid-bool package for the same situation:
https://src.fedoraproject.org/rpms/rust-cpuid-bool/blob/rawhide/f/rust-cpuid-bool.spec

Additionally, It would be great to use the changelog timestamp format without time and timezone.
rust2rpm >= 18 will do that by default again.

Comment 2 Robert-André Mauchin 🐧 2021-07-19 16:17:18 UTC
(In reply to Fabio Valentini from comment #1)
> Taking this review.
> 
> Looks like you'll need to add something like "ExclusiveArch: x86_64 aarch64".
> The crate does not compile on other architectures, and should also not be
> used on other architectures.
> 
This gonna restrict significantly the reach of dependent packages :(

> Alternatively, you could do something like what was done in the
> rust-cpuid-bool package for the same situation:
> https://src.fedoraproject.org/rpms/rust-cpuid-bool/blob/rawhide/f/rust-cpuid-
> bool.spec
> 
> Additionally, It would be great to use the changelog timestamp format
> without time and timezone.
> rust2rpm >= 18 will do that by default again.

I've reverted itt in my script, but I plqn to move to rpmautospec for all new and updated packages.

Comment 4 Robert-André Mauchin 🐧 2021-07-19 16:22:45 UTC
Thanks for the review!

Comment 5 Fabio Valentini 2021-07-19 16:29:41 UTC
(In reply to Robert-André Mauchin 🐧 from comment #2)
> (In reply to Fabio Valentini from comment #1)
> > Taking this review.
> > 
> > Looks like you'll need to add something like "ExclusiveArch: x86_64 aarch64".
> > The crate does not compile on other architectures, and should also not be
> > used on other architectures.
> > 
> This gonna restrict significantly the reach of dependent packages :(

It won't. Or at least, it shouldn't.
For example, looking at for example sha-1, the cpufeatures dependency is scoped to x86_64 and aarch64, as well,
same as the cpuid-bool dependency was before it in older versions.
If the dependency is not scoped by target architecture in some crates, then that's a bug. :)

> > Alternatively, you could do something like what was done in the
> > rust-cpuid-bool package for the same situation:
> > https://src.fedoraproject.org/rpms/rust-cpuid-bool/blob/rawhide/f/rust-cpuid-
> > bool.spec
> > 
> > Additionally, It would be great to use the changelog timestamp format
> > without time and timezone.
> > rust2rpm >= 18 will do that by default again.
> 
> I've reverted itt in my script, but I plqn to move to rpmautospec for all
> new and updated packages.

Works for me.

Package APPROVED

Comment 6 Gwyn Ciesla 2021-07-19 20:46:10 UTC
(fedscm-admin):  The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-cpufeatures

Comment 7 Fedora Update System 2021-07-27 21:44:46 UTC
FEDORA-2021-9f4d0e6041 has been submitted as an update to Fedora 34. https://bodhi.fedoraproject.org/updates/FEDORA-2021-9f4d0e6041

Comment 8 Fedora Update System 2021-07-27 21:45:42 UTC
FEDORA-2021-2f997f6af2 has been pushed to the Fedora 35 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 9 Fedora Update System 2021-07-28 01:29:31 UTC
FEDORA-2021-9f4d0e6041 has been pushed to the Fedora 34 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf upgrade --enablerepo=updates-testing --advisory=FEDORA-2021-9f4d0e6041`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2021-9f4d0e6041

See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.

Comment 10 Fedora Update System 2021-08-04 03:43:14 UTC
FEDORA-2021-9f4d0e6041 has been pushed to the Fedora 34 stable repository.
If problem still persists, please make note of it in this bug report.