Bug 2165224

Summary: Review Request: rust-fallible_collections - Crate which adds fallible allocation api to std collections
Product: [Fedora] Fedora Reporter: blinxen <h-k-81>
Component: Package ReviewAssignee: Fabio Valentini <decathorpe>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: medium Docs Contact:
Priority: medium    
Version: rawhideCC: decathorpe, package-review
Target Milestone: ---Flags: decathorpe: fedora-review+
Target Release: ---   
Hardware: All   
OS: Linux   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2023-02-12 20:55:07 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:

Description blinxen 2023-01-28 16:02:50 UTC
Spec URL: https://download.copr.fedorainfracloud.org/results/blinxen/rust-fallible_collections/fedora-37-x86_64/05344958-rust-fallible_collections/rust-fallible_collections.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/blinxen/rust-fallible_collections/fedora-37-x86_64/05344958-rust-fallible_collections/rust-fallible_collections-0.4.6-1.fc37.src.rpm

Description:

Implement Fallible collections on allocation errors, quite as describe in RFC 2116.
This was used in the turbofish OS hobby project to mitigate
the lack of faillible allocation in rust.

Fedora Account System Username: blinxen

The purpose of this package review is to unretire the fallible_collections crate. See https://pagure.io/releng/issue/11245.

Comment 2 Fabio Valentini 2023-01-28 19:21:58 UTC
Oh, another library that was rewritten from C in Rust. :D

Reading the upstream README, I recommend that use use cargo-c to build the shared library.
In my experience, it is the least painful option for building C-style shared libraries, especially because it handles setting SONAME and generating headers and pkg-config files.

===

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.

===

Once releng has processed the unretirement, please also request an f37 branch for this package. It makes it much easier for Rust SIG members to update things if there's not "holes" (i.e. this package would be present in rawhide and f36, but not f37, which would be weird). I also recommend to re-enable tracking for the package in koschei for the rawhide and f37 branches once they have been created.

Comment 3 Fabio Valentini 2023-01-28 19:24:43 UTC
I forgot to add: We already use cargo-c for building rav1e (an AV1 encoder library).
It might be helpful to look at its spec file to see how to use cargo-c for Fedora RPM builds:
https://src.fedoraproject.org/rpms/rust-rav1e/blob/rawhide/f/rust-rav1e.spec#_543-560

Comment 4 blinxen 2023-01-28 22:47:53 UTC
> I forgot to add: We already use cargo-c for building rav1e (an AV1 encoder library).
> It might be helpful to look at its spec file to see how to use cargo-c for Fedora RPM builds:
> https://src.fedoraproject.org/rpms/rust-rav1e/blob/rawhide/f/rust-rav1e.spec#_543-560

Thanks for the tip!! Much appreciated since I am still new to the packaging scene :D.

Comment 5 Fabio Valentini 2023-02-01 22:37:45 UTC
It appears that build of this crate failed in koji - do you need help with debugging the failure?

Comment 6 blinxen 2023-02-02 08:19:07 UTC
Help would be very much appreciated! I was trying to debug it myself, but I think I don't understand the underlying problem.
I have created the following issue in the upstream repository: https://github.com/vcombey/fallible_collections/issues/35
I documented my findings findings there. The only build that is failing, is the i686 build.

Comment 7 blinxen 2023-02-12 20:55:07 UTC
The package now builds successfully for all targeted branches. Closing this review.