https://bugzilla.redhat.com/show_bug.cgi?id=2295793
Bug ID: 2295793 Summary: Review Request: rust-btf-rs - Library for the BPF type format (BTF) Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: atenart@redhat.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://github.com/retis-org/copr-playground/blob/main/rust-btf-rs.spec SRPM URL: https://download.copr.fedorainfracloud.org/results/@retis/playground/fedora-... Build: https://copr.fedorainfracloud.org/coprs/g/retis/playground/build/7671873/
Description:
Library for the BPF type format (BTF).
Fedora Account System Usernames: atenart, pvaler, amorenoz
I’m starting this review request on behalf of Paolo, Adrián and me. We’d like to maintain it as a group. It’s our first package and we’ll need a sponsor. We’re also the upstream maintainers of btf-rs. This is needed for the Retis package (https://bugzilla.redhat.com/show_bug.cgi?id=2295792).
https://bugzilla.redhat.com/show_bug.cgi?id=2295793
Fedora Review Service fedora-review-bot@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- URL| |https://crates.io/crates/bt | |f-rs Keywords| |AutomationTriaged
--- Comment #1 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/7707873 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2295793
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |decathorpe@gmail.com
--- Comment #3 from Fabio Valentini decathorpe@gmail.com --- Please provide URLs to raw files, not HTML pages. Otherwise tooling will not be able to parse this ticket.
https://bugzilla.redhat.com/show_bug.cgi?id=2295793
Antoine Tenart atenart@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Comment|0 |updated
--- Comment #0 has been edited ---
Spec URL: https://raw.githubusercontent.com/retis-org/copr-playground/main/rust-btf-rs... SRPM URL: https://download.copr.fedorainfracloud.org/results/@retis/playground/fedora-... Build: https://copr.fedorainfracloud.org/coprs/g/retis/playground/build/7671873/
Description:
Library for the BPF type format (BTF).
Fedora Account System Usernames: atenart, pvaler, amorenoz
I’m starting this review request on behalf of Paolo, Adrián and me. We’d like to maintain it as a group. It’s our first package and we’ll need a sponsor. We’re also the upstream maintainers of btf-rs. This is needed for the Retis package (https://bugzilla.redhat.com/show_bug.cgi?id=2295792).
https://bugzilla.redhat.com/show_bug.cgi?id=2295793
--- Comment #4 from Antoine Tenart atenart@redhat.com ---
Please provide URLs to raw files, not HTML pages. Otherwise tooling will not be able to parse this ticket.
I updated the spec file link to point to a raw content, assuming this was the issue. If this was about other links let me know.
https://bugzilla.redhat.com/show_bug.cgi?id=2295793
--- Comment #5 from Fabio Valentini decathorpe@gmail.com --- [fedora-review-service-build]
https://bugzilla.redhat.com/show_bug.cgi?id=2295793
--- Comment #6 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/7773627 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2295793
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED Assignee|nobody@fedoraproject.org |decathorpe@gmail.com Flags| |fedora-review?
--- Comment #7 from Fabio Valentini decathorpe@gmail.com --- Two minor issues:
1. It looks like you changed the License tag in the package from LGPL-2.0-or-later to LGPL-2.1-or-later. The latter matches the license text included in the package.
Is this an actual issue in the crate? i.e. is LGPL-2.1-or-later the intended license (I assume so)? If this is the case, please file an issue with the upstream project about fixing the license expression in the crate metadata.
Additionally, to have automated tools for collecting license information pick this fix up, you will need to use "rust2rpm -p" to patch Cargo.toml for this change too - just editing the License tag in the generated spec file is not enough in this case.
2. You dropped the subpackage corresponding to the "test_runtime" feature. Why? It has no additional dependencies, so I see no reason to hide this feature on the RPM metadata side.
https://bugzilla.redhat.com/show_bug.cgi?id=2295793
--- Comment #8 from Antoine Tenart atenart@redhat.com --- (In reply to Fabio Valentini from comment #7)
Two minor issues:
- It looks like you changed the License tag in the package from
LGPL-2.0-or-later to LGPL-2.1-or-later. The latter matches the license text included in the package.
Is this an actual issue in the crate? i.e. is LGPL-2.1-or-later the intended license (I assume so)? If this is the case, please file an issue with the upstream project about fixing the license expression in the crate metadata.
This is a known issue and I opened a PR some time ago, https://github.com/retis-org/btf-rs/pull/26. I pinged the other maintainers and it is merged now.
Additionally, to have automated tools for collecting license information pick this fix up, you will need to use "rust2rpm -p" to patch Cargo.toml for this change too - just editing the License tag in the generated spec file is not enough in this case.
OK, so Cargo.toml needs to be up to date for the tooling. Alternatively we can release a new upstream version with that fix included, IMO that would be even better.
- You dropped the subpackage corresponding to the "test_runtime" feature.
Why? It has no additional dependencies, so I see no reason to hide this feature on the RPM metadata side.
It has no additional dependency but is a very specific feature. It enables tests requiring specific privileges (access to /sys/kernel/btf/*) instead of test files provided in the sources. There's really no use case for a consumer of that crate to use such feature (and looking at it again now we might even remove it in the future).
Thanks for the review!
https://bugzilla.redhat.com/show_bug.cgi?id=2295793
--- Comment #9 from Antoine Tenart atenart@redhat.com --- (In reply to Antoine Tenart from comment #8)
(In reply to Fabio Valentini from comment #7)
Two minor issues:
- It looks like you changed the License tag in the package from
LGPL-2.0-or-later to LGPL-2.1-or-later. The latter matches the license text included in the package.
Is this an actual issue in the crate? i.e. is LGPL-2.1-or-later the intended license (I assume so)? If this is the case, please file an issue with the upstream project about fixing the license expression in the crate metadata.
This is a known issue and I opened a PR some time ago, https://github.com/retis-org/btf-rs/pull/26. I pinged the other maintainers and it is merged now.
Additionally, to have automated tools for collecting license information pick this fix up, you will need to use "rust2rpm -p" to patch Cargo.toml for this change too - just editing the License tag in the generated spec file is not enough in this case.
OK, so Cargo.toml needs to be up to date for the tooling. Alternatively we can release a new upstream version with that fix included, IMO that would be even better.
FYI I just released btf-rs v1.1.1, which includes the license fix in Cargo.toml, and updated the spec file.
https://bugzilla.redhat.com/show_bug.cgi?id=2295793
--- Comment #10 from Fabio Valentini decathorpe@gmail.com --- Sorry for the delay. Please post updated Spec *and* SRPM URLs that match (i.e. point to the same version).
You don't need to update Comment 0 (in fact, that makes it harder to tell that you changed something, and trips up automation), you can just add it in a new comment below.
https://bugzilla.redhat.com/show_bug.cgi?id=2295793
--- Comment #11 from Antoine Tenart atenart@redhat.com --- Thanks for the info. Here is the btf-rs 1.1.1-1 build, which includes the license tag fix,
Spec URL: https://raw.githubusercontent.com/retis-org/copr-playground/main/rust-btf-rs... SRPM URL: https://download.copr.fedorainfracloud.org/results/@retis/playground/fedora-... Build: https://copr.fedorainfracloud.org/coprs/g/retis/playground/build/7967763/
https://bugzilla.redhat.com/show_bug.cgi?id=2295793
--- Comment #12 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Created attachment 2045298 --> https://bugzilla.redhat.com/attachment.cgi?id=2045298&action=edit The .spec file difference from Copr build 7773627 to 7974548
https://bugzilla.redhat.com/show_bug.cgi?id=2295793
--- Comment #13 from Fedora Review Service fedora-review-bot@fedoraproject.org --- Copr build: https://copr.fedorainfracloud.org/coprs/build/7974548 (succeeded)
Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-rev...
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.
https://bugzilla.redhat.com/show_bug.cgi?id=2295793
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Flags|fedora-review? |fedora-review+
--- Comment #14 from Fabio Valentini decathorpe@gmail.com --- Looks good to me now, thank you!
===
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 ✅ latest version of the crate is packaged ✅ license matches upstream specification and is acceptable for Fedora ✅ license file is 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 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)
===
Your sponsor should be able to help you with these tasks and with importing the package if you have any questions.
https://bugzilla.redhat.com/show_bug.cgi?id=2295793
Fernando F. Mancera ferferna@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |ferferna@redhat.com
--- Comment #15 from Fernando F. Mancera ferferna@redhat.com --- Hi, I am willing to sponsor the packager. And I am doing a review of the package too.
Check list for rust package:
[X] package contains only permissible content [X] package builds and installs without errors on rawhide [X] test suite is run and all unit tests pass [X] latest version of the crate is packaged [X] license matches upstream specification and is acceptable for Fedora [X] license file is included with %license in %files [X] package complies with Rust Packaging Guidelines
---
I will help moving it forward importing the package and setting the bugzilla assignee and dist-git.
Thank you!
https://bugzilla.redhat.com/show_bug.cgi?id=2295793
Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |RELEASE_PENDING
--- Comment #16 from Fedora Admin user for bugzilla script actions fedora-admin-xmlrpc@fedoraproject.org --- The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-btf-rs
https://bugzilla.redhat.com/show_bug.cgi?id=2295793
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RELEASE_PENDING |MODIFIED
--- Comment #17 from Fedora Update System updates@fedoraproject.org --- FEDORA-2024-08cdcbef96 (rust-btf-rs-1.1.1-1.fc42) has been submitted as an update to Fedora 42. https://bodhi.fedoraproject.org/updates/FEDORA-2024-08cdcbef96
https://bugzilla.redhat.com/show_bug.cgi?id=2295793
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|MODIFIED |CLOSED Resolution|--- |ERRATA Last Closed| |2024-10-15 14:19:27
--- Comment #18 from Fedora Update System updates@fedoraproject.org --- FEDORA-2024-08cdcbef96 (rust-btf-rs-1.1.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.
package-review@lists.fedoraproject.org