Bug 2295793

Copr build:
https://copr.fedorainfracloud.org/coprs/build/7707873
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2295793-rust-btf-rs/fedora-rawhide-x86_64/07707873-rust-btf-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.

Blocked FE-NEEDSPONSOR as you mention still requiring a sponsor

Please provide URLs to raw files, not HTML pages. Otherwise tooling will not be able to parse this ticket.

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

[fedora-review-service-build]

Copr build:
https://copr.fedorainfracloud.org/coprs/build/7773627
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2295793-rust-btf-rs/fedora-rawhide-x86_64/07773627-rust-btf-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.

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.

(In reply to Fabio Valentini from comment #7)
> 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.

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.

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

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!

(In reply to Antoine Tenart from comment #8)
> (In reply to Fabio Valentini from comment #7)
> > 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.
> 
> 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.

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.

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.spec
SRPM URL: https://download.copr.fedorainfracloud.org/results/@retis/playground/fedora-rawhide-x86_64/07967763-rust-btf-rs/rust-btf-rs-1.1.1-1.fc42.src.rpm
Build: https://copr.fedorainfracloud.org/coprs/g/retis/playground/build/7967763/

Created attachment 2045298 [details]
The .spec file difference from Copr build 7773627 to 7974548

Copr build:
https://copr.fedorainfracloud.org/coprs/build/7974548
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2295793-rust-btf-rs/fedora-rawhide-x86_64/07974548-rust-btf-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.

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.

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!

The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-btf-rs

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

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.