Bug 2165224 - Review Request: rust-fallible_collections - Crate which adds fallible allocation api to std collections
Summary: Review Request: rust-fallible_collections - Crate which adds fallible allocat...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Fabio Valentini
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2023-01-28 16:02 UTC by blinxen
Modified: 2023-02-12 20:55 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2023-02-12 20:55:07 UTC
Type: ---
Embargoed:
decathorpe: fedora-review+


Attachments (Terms of Use)

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.


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