Bug 1981110 (rust-cpufeatures) - Review Request: rust-cpufeatures - No-std compatible alternative to the is_x86_feature_detected! macro
Summary: Review Request: rust-cpufeatures - No-std compatible alternative to the is_x8...
Keywords:
Status: CLOSED ERRATA
Alias: rust-cpufeatures
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Fabio Valentini
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1957380
TreeView+ depends on / blocked
 
Reported: 2021-07-11 14:11 UTC by Robert-André Mauchin 🐧
Modified: 2021-08-04 03:43 UTC (History)
2 users (show)

Fixed In Version:
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2021-07-27 21:45:42 UTC
Type: ---
Embargoed:
decathorpe: fedora-review+


Attachments (Terms of Use)

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.


Note You need to log in before you can comment on or make changes to this bug.