Bug 2181035

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

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2181035-rust-vm-allocator/fedora-rawhide-x86_64/05696284-rust-vm-allocator/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 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.

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/4eaba5322d6dea29c8d41e4470a1ef1f67d5f495

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.

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?

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

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.

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/LicensingGuidelines/#_license_text

Upstream has confirmed the exact license text, so I updated the spec/SRPM to add it.

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

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

I'd prefer to not have a generic source file name here since the license text references "the rust-vmm authors" specific to this crate.  Giving it a crate-specific file name will avoid clashing when multiple SRPMs are extracted in SOURCES.  (This was a concern in the old days of packaging, maybe nobody cares anymore with everything containerized and buildrooted.)

FEDORA-2023-2ffdc3af5f has been submitted as an update to Fedora 39. https://bodhi.fedoraproject.org/updates/FEDORA-2023-2ffdc3af5f

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.

FEDORA-2023-02106a6c13 has been submitted as an update to Fedora 38. https://bodhi.fedoraproject.org/updates/FEDORA-2023-02106a6c13

FEDORA-2023-a4085b6295 has been submitted as an update to Fedora 37. https://bodhi.fedoraproject.org/updates/FEDORA-2023-a4085b6295

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.

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.

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.

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.