https://bugzilla.redhat.com/show_bug.cgi?id=2181035
Bug ID: 2181035 Summary: Review Request: rust-vm-allocator - Helpers for allocating resources needed during the lifetime of a VM Product: Fedora Version: rawhide Hardware: All OS: Linux Status: NEW Component: Package Review Severity: medium Priority: medium Assignee: nobody@fedoraproject.org Reporter: fedora.dm0@gmail.com QA Contact: extras-qa@fedoraproject.org CC: package-review@lists.fedoraproject.org Target Milestone: --- Classification: Fedora
Spec URL: https://github.com/dm0-/copr-firecracker/raw/fedora/rust-vm-allocator.spec SRPM URL: https://github.com/dm0-/copr-firecracker/raw/fedora/rust-vm-allocator-0.1.0-... Description: Helpers for allocating resources needed during the lifetime of a VM. Fedora Account System Username: dm0
This is a dependency of Firecracker. The spec is automatically generated. Note the crate license currently has issues, and upstream is working on it: https://github.com/rust-vmm/vm-allocator/issues/53
https://bugzilla.redhat.com/show_bug.cgi?id=2181035
Jakub Kadlčík jkadlcik@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- URL| |https://crates.io/crates/vm | |-allocator
--- Comment #1 from Jakub Kadlčík jkadlcik@redhat.com --- Copr build: https://copr.fedorainfracloud.org/coprs/build/5696284 (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=2181035
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |decathorpe@gmail.com Assignee|nobody@fedoraproject.org |decathorpe@gmail.com Flags| |fedora-review? Status|NEW |ASSIGNED Doc Type|--- |If docs needed, set a value
--- Comment #2 from Fabio Valentini decathorpe@gmail.com --- Two comments:
1. Please also fix the license in the Cargo.toml metadata. Otherwise it will not be correctly taken into account by tools like the "%cargo_license" RPM macro.
2. Please change the libc ">= 0.2.39" requirement to be either "^0.2.39" or just "0.2.39" (the latter of which are equivalent), and submit this change upstream. It is probably what upstream project intended, but used ">=" by mistake or due to a misunderstanding of how cargo resolves dependencies.
Requirements like ">= 0.2.39" that are semi-open ranges are pretty dangerous and will break at an unknown point in the future. For example, this requirement also matches "0.3.0" or even "42.0.99", all of which are *explicitly API-incompatible* with v0.2.
https://bugzilla.redhat.com/show_bug.cgi?id=2181035
--- Comment #3 from fedora.dm0@gmail.com --- I've updated this package to drop CODEOWNERS and coverage_config* files, to flag additional files as documentation, to fix the libc version, and to move the license edit from the spec to Cargo.toml. The libc change is upstream but not publishes to crates.io: https://github.com/rust-vmm/vm-allocator/commit/4eaba5322d6dea29c8d41e4470a1...
https://bugzilla.redhat.com/show_bug.cgi?id=2181035
--- Comment #4 from Fabio Valentini decathorpe@gmail.com --- Thanks! One minor problem and one bigger problem remain:
1. I'm not sure if marking directories as %doc even works ... I would replace this: "%doc %{crate_instdir}/images" with this: "%doc %{crate_instdir}/images/*"
Marking "src/allocation_engine/DESIGN.md" as %doc isn't really necessary, but it doesn't hurt either.
2. The project's license (both the upstream one and the fixed one) specify the BSD-3-Clause license, but the license text for this license is not included - neither in published crates, nor in the upstream project's repository. This seems like an oversight.
The BSD-3-Clause license also requires that (re)distributed sources contain a copy of the license notice and license text: https://choosealicense.com/licenses/bsd-3-clause/
The 2. issue is a blocker for now ... please notify upstream about the missing license text.
https://bugzilla.redhat.com/show_bug.cgi?id=2181035
--- Comment #5 from fedora.dm0@gmail.com --- The files under the directory get marked as documentation, but I can change it.
I notified upstream at the link in the first comment. Should I add the BSD license text to the SRPM directly?
https://bugzilla.redhat.com/show_bug.cgi?id=2181035
--- Comment #6 from Fabio Valentini decathorpe@gmail.com --- (In reply to fedora.dm0 from comment #5)
The files under the directory get marked as documentation, but I can change it.
Ah, then it works, but it still doesn't make sense for Rust -devel packages :)
I notified upstream at the link in the first comment. Should I add the BSD license text to the SRPM directly?
Sorry, I seem to have skimmed over that. As far as I know, just including the license text without the upstream project's blessing is a no-go (unless it's a last resort). In this case, the project seems to be aware of the problem, so they should decide what the correct license text is.
https://bugzilla.redhat.com/show_bug.cgi?id=2181035
--- Comment #7 from fedora.dm0@gmail.com --- I added comments and dropped docs in the spec and SRPM.
Would it be possible to just declare the package as Apache-2.0, since the BSD license is only for some dual-licensed files? Otherwise, I guess I can pester upstream about it.
https://bugzilla.redhat.com/show_bug.cgi?id=2181035
--- Comment #8 from Fabio Valentini decathorpe@gmail.com --- No, the license of the upstream code must be reflected in the Fedora package: https://docs.fedoraproject.org/en-US/legal/license-field/#_basic_policy
In particular: "The spec file License: field consists of an enumeration of all licenses covering any code or other material contained in the corresponding binary RPM."
The new Legal guidelines are still a bit lacking in some regards, but the general Packaging Guidelines have a section about license files, which applies to this situation: https://docs.fedoraproject.org/en-US/packaging-guidelines/LicensingGuideline...
https://bugzilla.redhat.com/show_bug.cgi?id=2181035
--- Comment #9 from fedora.dm0@gmail.com --- Upstream has confirmed the exact license text, so I updated the spec/SRPM to add it.
https://bugzilla.redhat.com/show_bug.cgi?id=2181035
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |POST Flags|fedora-review? |fedora-review+
--- Comment #10 from Fabio Valentini decathorpe@gmail.com --- Thanks for the update!
You can remove the trailing "#/%{crate}-0.1.0-bsd.txt" URL fragment from the source URL since you're overriding the file name with "cp -p %{SOURCE1} LICENSE-BSD-3-Clause" anyway (and you could even simplify that to "cp -p %{SOURCE1} ." if you drop the URL fragment).
===
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 ("Apache-2.0 AND (Apache-2.0 OR BSD-3-Clause)") 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:
- add @rust-sig with "commit" access as package co-maintainer
- set bugzilla assignee overrides to @rust-sig (optional)
- 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
- track package in koschei for all built branches
https://bugzilla.redhat.com/show_bug.cgi?id=2181035
--- Comment #11 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-vm-allocator
https://bugzilla.redhat.com/show_bug.cgi?id=2181035
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|POST |MODIFIED
--- Comment #13 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-2ffdc3af5f has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2023-2ffdc3af5f
https://bugzilla.redhat.com/show_bug.cgi?id=2181035
Fedora Update System updates@fedoraproject.org changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |ERRATA Status|MODIFIED |CLOSED Last Closed| |2023-03-28 19:27:06
--- Comment #14 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-2ffdc3af5f has been pushed to the Fedora 39 stable repository. If problem still persists, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=2181035
--- Comment #15 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-02106a6c13 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2023-02106a6c13
https://bugzilla.redhat.com/show_bug.cgi?id=2181035
--- Comment #16 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-a4085b6295 has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2023-a4085b6295
https://bugzilla.redhat.com/show_bug.cgi?id=2181035
--- Comment #17 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-02106a6c13 has been pushed to the Fedora 38 testing repository.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-02106a6c13
See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
https://bugzilla.redhat.com/show_bug.cgi?id=2181035
--- Comment #18 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-a4085b6295 has been pushed to the Fedora 37 testing repository. Soon you'll be able to install the update with the following command: `sudo dnf install --enablerepo=updates-testing --refresh --advisory=FEDORA-2023-a4085b6295 *` You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2023-a4085b6295
See also https://fedoraproject.org/wiki/QA:Updates_Testing for more information on how to test updates.
https://bugzilla.redhat.com/show_bug.cgi?id=2181035
--- Comment #19 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-a4085b6295 has been pushed to the Fedora 37 stable repository. If problem still persists, please make note of it in this bug report.
https://bugzilla.redhat.com/show_bug.cgi?id=2181035
--- Comment #20 from Fedora Update System updates@fedoraproject.org --- FEDORA-2023-02106a6c13 has been pushed to the Fedora 38 stable repository. If problem still persists, please make note of it in this bug report.
package-review@lists.fedoraproject.org