Bug 2353001
| Summary: | Review Request: rust-in-toto - A rust implementation of in-toto | ||
|---|---|---|---|
| Product: | [Fedora] Fedora | Reporter: | Jelle van der Waa <jvanderwaa> |
| Component: | Package Review | Assignee: | Fabio Valentini <decathorpe> |
| Status: | CLOSED ERRATA | QA Contact: | Fedora Extras Quality Assurance <extras-qa> |
| Severity: | medium | Docs Contact: | |
| Priority: | medium | ||
| Version: | rawhide | CC: | decathorpe, package-review |
| Target Milestone: | --- | Keywords: | AutomationTriaged |
| Target Release: | --- | Flags: | decathorpe:
fedora-review+
|
| Hardware: | All | ||
| OS: | Linux | ||
| URL: | https://crates.io/crates/in-toto | ||
| Whiteboard: | |||
| Fixed In Version: | Doc Type: | --- | |
| Doc Text: | Story Points: | --- | |
| Clone Of: | Environment: | ||
| Last Closed: | 2025-04-16 15:57:57 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
Jelle van der Waa
2025-03-17 19:34:34 UTC
Copr build: https://copr.fedorainfracloud.org/coprs/build/8778293 (succeeded) Review template: https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2353001-rust-in-toto/fedora-rawhide-x86_64/08778293-rust-in-toto/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. Package looks mostly fine to me, just two suggestions:
- Don't rename the patch file from the name that is generated by rust2rpm.
Some of its functionality relies on the file name being the expected one, and you'd also need to keep making those renaming changes every time there's an update for this crate.
- Consider excluding all the test data from the built package. There's a lot of files and folder structure that's not needed for the functionality of the crate, all of it seems to be subdirectories of "tests/". You should be able to use something like `%exclude %{crate_instdir}/*/` in the %files list of the -devel subpackage for this purpose.
Thanks for the review, it seems I send an old src.rpm initially which didn't use a rust2rpm.toml file. This new version does use it to add BuildRequires for openssl and the test exclude you requested. New src.rpm https://download.copr.fedorainfracloud.org/results/jelly/rebuilderd/fedora-rawhide-aarch64/08827846-rust-in-toto/rust-in-toto-0.4.0-1.fc43.src.rpm New spec file https://download.copr.fedorainfracloud.org/results/jelly/rebuilderd/fedora-rawhide-aarch64/08827846-rust-in-toto/rust-in-toto.spec Ah, I see now that I mis-interpreted something the first time round, by looking at the git diff wrongly.
> # Manually created patch for downstream crate metadata changes
This line is usually kept for Rust packages.
Either way, you need to document the patch in some way, ideally by adding links to upstream PRs (or commits) to bump the two dependencies you adjusted.
Other than that, looks good to me, thanks!
The manual patch for metadata is just bumping derp to be the same latest version as Fedora has. I have found this PR upstream which is merged, I assume that applying this patch is preferred over "rust2rpm --patch"? https://github.com/in-toto/in-toto-rs/pull/103 No, the only supported way to apply patches to Cargo.toml is with "rust2rpm --patch" -
because the final Cargo.toml contents must be known *before* spec file generation.
You can link that upstream PR as documentation though, that would be perfect.
You can put it into the config file like that:
```
[package]
cargo-toml-patch-comments = [
"bump derp and untrusted dependencies: https://github.com/in-toto/in-toto-rs/pull/103",
]
```
This way the comment is added automatically, and also causes rust2rpm to do some more sanity checks when it's run.
Assuming you add a link to this PR to the spec file, package looks good to me, thanks.
===
Package was generated with rust2rpm, simplifying the review.
Patches are reasonable and correspond to changes already upstream but not yet part of a new release.
✅ 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 (*NOT* pre-release filter): alpha;beta;rc;pre
distro: Fedora
Package: rust-$crate
- set bugzilla assignee overrides to @rust-sig (optional)
The Pagure repository was created at https://src.fedoraproject.org/rpms/rust-in-toto FEDORA-2025-831c3d8fe1 (rust-in-toto-0.4.0-1.fc43) has been submitted as an update to Fedora 43. https://bodhi.fedoraproject.org/updates/FEDORA-2025-831c3d8fe1 FEDORA-2025-831c3d8fe1 (rust-in-toto-0.4.0-1.fc43) has been pushed to the Fedora 43 stable repository. If problem still persists, please make note of it in this bug report. |