Bug 2312901 - Review Request: rust-blosc2-rs - Bindings to C Blosc2
Summary: Review Request: rust-blosc2-rs - Bindings to C Blosc2
Keywords:
Status: CLOSED ERRATA
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Cristian Le
QA Contact: Fedora Extras Quality Assurance
URL: https://crates.io/crates/blosc2-rs
Whiteboard:
Depends On:
Blocks: 2277635
TreeView+ depends on / blocked
 
Reported: 2024-09-17 18:27 UTC by Ben Beasley
Modified: 2024-10-15 00:16 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2024-10-04 11:47:43 UTC
Type: ---
Embargoed:
fedora: fedora-review+


Attachments (Terms of Use)
Configuration file for rust2rpm (1.77 KB, text/plain)
2024-09-17 18:29 UTC, Ben Beasley
no flags Details
Updated configuration for rust2rpm (1.96 KB, text/plain)
2024-09-18 14:37 UTC, Ben Beasley
no flags Details
The .spec file difference from Copr build 8028549 to 8031348 (1.81 KB, patch)
2024-09-18 14:39 UTC, Fedora Review Service
no flags Details | Diff
Updated configuration for rust2rpm (2.35 KB, text/plain)
2024-10-03 14:09 UTC, Ben Beasley
no flags Details
The .spec file difference from Copr build 8091951 to 8105760 (950 bytes, patch)
2024-10-03 14:17 UTC, Fedora Review Service
no flags Details | Diff

Description Ben Beasley 2024-09-17 18:27:54 UTC
Spec URL: https://music.fedorapeople.org/rust-blosc2-rs.spec
SRPM URL: https://music.fedorapeople.org/rust-blosc2-rs-0.3.0-1.fc40.src.rpm
Description: Bindings to C Blosc2.
Fedora Account System Username: music

Comment 1 Ben Beasley 2024-09-17 18:29:35 UTC
Created attachment 2047284 [details]
Configuration file for rust2rpm

Comment 2 Fedora Review Service 2024-09-17 18:38:02 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8028549
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2312901-rust-blosc2-rs/fedora-rawhide-x86_64/08028549-rust-blosc2-rs/fedora-review/review.txt

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.

Comment 3 Ben Beasley 2024-09-18 14:36:39 UTC
Updated to the latest upstream release, which exposes some C blosc2 CMake configuration options as features. I’ve hidden those that are disabled in the system blosc2 (so allowing other packages to depend on the corresponding features would be somewhere between confusing and wrong).

New Spec URL: https://music.fedorapeople.org/20240918/rust-blosc2-rs.spec
New SRPM URL: https://music.fedorapeople.org/20240918/rust-blosc2-rs-0.3.1-1.fc40.src.rpm

Comment 4 Ben Beasley 2024-09-18 14:37:03 UTC
Created attachment 2047403 [details]
Updated configuration for rust2rpm

Comment 5 Fedora Review Service 2024-09-18 14:39:37 UTC
Created attachment 2047405 [details]
The .spec file difference from Copr build 8028549 to 8031348

Comment 6 Fedora Review Service 2024-09-18 14:39:39 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8031348
(failed)

Build log:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2312901-rust-blosc2-rs/fedora-rawhide-x86_64/08031348-rust-blosc2-rs/builder-live.log.gz

Please make sure the package builds successfully at least for Fedora Rawhide.

- If the build failed for unrelated reasons (e.g. temporary network
  unavailability), please ignore it.
- If the build failed because of missing BuildRequires, please make sure they
  are listed in the "Depends On" field


---
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.

Comment 7 Ben Beasley 2024-09-18 15:38:51 UTC
The automated build failed because https://src.fedoraproject.org/rpms/rust-blosc2-sys/pull-request/3 is not yet in a Rawhide compose. Until then, this can be reviewed with fedora-review -b 2312901 --mock-options=--enablerepo=local.

Comment 8 Ben Beasley 2024-09-30 11:18:49 UTC
[fedora-review-service-build]

Comment 9 Fedora Review Service 2024-09-30 11:28:20 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8091951
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2312901-rust-blosc2-rs/fedora-rawhide-x86_64/08091951-rust-blosc2-rs/fedora-review/review.txt

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.

Comment 10 Cristian Le 2024-10-02 01:17:37 UTC
The `data` folder can be removed, couldn't it? The Copying file indicates it is for benchmarks, and it would be nice to not have to update the license metadata because of those files. Other than that, probably `environment.yml` can also be excluded from installation. I guess you've already discussed with Fabio about the crate version decoupling for this?

Comment 11 Ben Beasley 2024-10-03 13:26:47 UTC
Thanks for looking at this!

(In reply to Cristian Le from comment #10)
> The `data` folder can be removed, couldn't it? The Copying file indicates it
> is for benchmarks, and it would be nice to not have to update the license
> metadata because of those files.

Ugh, I’m not sure how I missed that. The benchmark data are dubiously licensed even for the source RPMs. I sent a PR upstream to python-cramjam for this, https://github.com/milesgranger/cramjam/issues/178. I’ll need to do something similar here, and (until it’s merged and released), package from a “filtered” version of the crate archive to avoid including the dubious files in source RPMs.

> Other than that, probably `environment.yml`
> can also be excluded from installation.

Sure, it’s a tiny file that’s doing no harm, but it’s indeed unnecessary. I would be inclined not to bother patching downstream for it, but it’s worth excluding it in the PR I send upstream for the benchmark data.

> I guess you've already discussed
> with Fabio about the crate version decoupling for this?

I assume you are talking about this?

  # * Do not test that the blosc2-sys bidings were generated against the exact
  #   version of c-blosc2 that upstream expects; we always use the system blosc2,
  #   whatever that may be.
  %cargo_test -- -- --exact --skip tests::test_get_version_string

I think it did come up explicitly in this case, but in general, loosening bounds on library versions in -sys crates is really our only option. If we are going to build against system libraries, we can’t be pinning them to an exact patch-release. If we really needed an exact patch-release, that would be adequate justification for bundling. In almost all cases these strict dependencies are a matter of the crate authors focusing primarily on static linking with bundled libraries and expecting to control their dependency versions. If an update is ABI-compatible for C programs, it’s very unlikely that it would break a -sys crate. This is even more true in cases like this where I will maintain the -sys crate but not the system library, so an exact-version pin could be broken by a compatible update at any time. Still, I think expecting an exact library version is generally too brittle even when both packages share maintainers.

Comment 12 Cristian Le 2024-10-03 13:53:17 UTC
(In reply to Ben Beasley from comment #11)
> > I guess you've already discussed
> > with Fabio about the crate version decoupling for this?
> 
> I assume you are talking about this?
> 
>   # * Do not test that the blosc2-sys bidings were generated against the
> exact
>   #   version of c-blosc2 that upstream expects; we always use the system
> blosc2,
>   #   whatever that may be.
>   %cargo_test -- -- --exact --skip tests::test_get_version_string

I'm actually referring to the difference between `0.3.0+2.15.1` on the crate (and maybe in the dependents?) and `0.3.0` in the rpm. I don't know how rust dependency checker is handling that.

Comment 13 Ben Beasley 2024-10-03 14:03:56 UTC
(In reply to Cristian Le from comment #12)
> I'm actually referring to the difference between `0.3.0+2.15.1` on the crate
> (and maybe in the dependents?) and `0.3.0` in the rpm. I don't know how rust
> dependency checker is handling that.

The metadata version field is automatically stripped by the Rust/cargo packaging scripts, both in the RPM package version and in generated RPM dependencies. This is why we have rust-blosc2-sys-0.3.1-1.fc42 instead of rust-blosc2-sys-0.3.1+X.Y.Z-1.fc42, and why that package satisfies the dependency from this crate. Fabio could surely explain if you have more detailed questions about where and how that happens, but it’s automatic and expected, and I’m not doing anything special to make it happen.

Comment 15 Ben Beasley 2024-10-03 14:09:10 UTC
Created attachment 2050234 [details]
Updated configuration for rust2rpm

After generating the spec file with this rust2rpm.toml, we must change the Source line to:

Source:         %{crate}-%{upstream_version}-clean.crate

to use the “filtered” crate source, and we must remove:

%license %{crate_instdir}/data/COPYING

since the spec file was generated from the “unfiltered” crate.

Comment 16 Fedora Review Service 2024-10-03 14:17:47 UTC
Created attachment 2050235 [details]
The .spec file difference from Copr build 8091951 to 8105760

Comment 17 Fedora Review Service 2024-10-03 14:17:49 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/8105760
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2312901-rust-blosc2-rs/fedora-rawhide-x86_64/08105760-rust-blosc2-rs/fedora-review/review.txt

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.

Comment 18 Cristian Le 2024-10-04 09:43:45 UTC
LGTM. PR for cleaning the crate is already merged, but probably will take some time for it to land.

(using Fabio's review template)

===

Package was generated with rust2rpm, simplifying the review.

✅❌❓🫤

✅ package contains only permissible content
✅ package builds and installs without errors on rawhide
✅ test suite is run and all unit tests pass (except for known s390x failures that are being tracked)
✅ 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 (*NOT* pre-release) 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)

Comment 19 Ben Beasley 2024-10-04 11:08:49 UTC
Thank you for the review (and for doing it thoroughly enough to catch the test data issues)!

https://release-monitoring.org/project/374763/

I’ll take a look at your pending reviews and see if I can get a few of them done.

Comment 20 Fedora Admin user for bugzilla script actions 2024-10-04 11:09:35 UTC
The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-blosc2-rs

Comment 21 Fedora Update System 2024-10-04 11:42:52 UTC
FEDORA-2024-72a7d856fb (rust-blosc2-rs-0.3.1-1.fc42) has been submitted as an update to Fedora 42.
https://bodhi.fedoraproject.org/updates/FEDORA-2024-72a7d856fb

Comment 22 Fedora Update System 2024-10-04 11:47:43 UTC
FEDORA-2024-72a7d856fb (rust-blosc2-rs-0.3.1-1.fc42) has been pushed to the Fedora 42 stable repository.
If problem still persists, please make note of it in this bug report.

Comment 23 Fedora Update System 2024-10-07 00:55:53 UTC
FEDORA-2024-008effb705 (rust-blosc2-rs-0.3.1-1.fc41) has been submitted as an update to Fedora 41.
https://bodhi.fedoraproject.org/updates/FEDORA-2024-008effb705

Comment 24 Fedora Update System 2024-10-08 01:45:11 UTC
FEDORA-2024-008effb705 has been pushed to the Fedora 41 testing repository.
Soon you'll be able to install the update with the following command:
`sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2024-008effb705 \*`
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2024-008effb705

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

Comment 25 Fedora Update System 2024-10-15 00:16:46 UTC
FEDORA-2024-008effb705 (rust-blosc2-rs-0.3.1-1.fc41) has been pushed to the Fedora 41 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.