https://bugzilla.redhat.com/show_bug.cgi?id=2181020
Bug ID: 2181020 Summary: Review Request: rust-cargo_toml - Cargo.toml struct definitions for parsing with Serde 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-cargo_toml.spec SRPM URL: https://github.com/dm0-/copr-firecracker/raw/fedora/rust-cargo_toml-0.13.3-1... Description: Cargo.toml struct definitions for parsing with Serde. Fedora Account System Username: dm0
This is a dependency of the Firecracker test suite, specifically the older 0.13 branch. The spec is automatically generated, except for a minor edit to the description to remove Markdown. The upstream crate is missing license files, so I've opened an issue to request adding them: https://gitlab.com/crates.rs/cargo_toml/-/issues/24
https://bugzilla.redhat.com/show_bug.cgi?id=2181020
Jakub Kadlčík jkadlcik@redhat.com changed:
What |Removed |Added ---------------------------------------------------------------------------- URL| |https://crates.io/crates/ca | |rgo_toml
--- Comment #1 from Jakub Kadlčík jkadlcik@redhat.com --- Copr build: https://copr.fedorainfracloud.org/coprs/build/5696212 (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=2181020
Fabio Valentini decathorpe@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |decathorpe@gmail.com Doc Type|--- |If docs needed, set a value Status|NEW |ASSIGNED Flags| |fedora-review? Assignee|nobody@fedoraproject.org |decathorpe@gmail.com
--- Comment #2 from Fabio Valentini decathorpe@gmail.com --- Package looks good, with two exceptions:
1. Both the MIT and Apache-2.0 licenses require that redistributed sources contain a copy of the license text, but the published crates for the cargo_toml crate do not contain license texts (this is why there's a FIXME in the generated spec file).
Looking at the upstream project, there's also no license files there: https://gitlab.com/crates.rs/cargo_toml
Please report this with the upstream project. They should include license files, otherwise their own project isn't conforming to the license(s) they chose :)
I've already filed similar issues with dozens of other projects ... for example, you can take this one as a template: https://github.com/danieldg/ordered-stream/issues/1
2. The latest version is 0.15.2, but you're packaging 0.13.3. I assume this is intentional (i.e. firecracker depends on ^0.13?)? If that is the case, just confirm this in this bug here. This is enough of a justification to not package the latest version.
https://bugzilla.redhat.com/show_bug.cgi?id=2181020
--- Comment #3 from fedora.dm0@gmail.com --- Do you want me to edit the issue linked in the original comment to indicate that it is a requirement?
Yes, the 0.13 branch is used specifically: https://github.com/firecracker-microvm/firecracker/blob/v1.3.1/src/firecrack...
https://bugzilla.redhat.com/show_bug.cgi?id=2181020
--- Comment #4 from Fabio Valentini decathorpe@gmail.com --- Oh, sorry, I missed that you already filed a ticket. The COPR build service is nice, but it sure adds noise to these tickets ... If the upstream project doesn't respond, please file a PR to include the license files, and include the files from the PR in your package.
Yes, the 0.13 branch is used specifically
Great, thanks, that is fine then.
https://bugzilla.redhat.com/show_bug.cgi?id=2181020
--- Comment #5 from fedora.dm0@gmail.com --- I've made a minor edit to this package to exclude the photo of Tom Anderson. It's not clear to me what the license of the image is. Would that need to removed from the SRPM?
If the crate license and the photo are problems, maybe this crate can be dropped like device_tree? It is only used in this file: https://github.com/firecracker-microvm/firecracker/blob/main/src/firecracker...
https://bugzilla.redhat.com/show_bug.cgi?id=2181020
--- Comment #6 from Fabio Valentini decathorpe@gmail.com --- Hm, true, it is weird to include a photo of a person in the sources ... If it's unclear what the license / rights situation is wrt/ that photo, then yes, it would probably need to be removed from the crate, and the sources re-packaged without it.
But as you mention, it's apparently only used in one test file in firecracker, so dropping the dependency and skipping these tests (which don't look that useful) would be easiest.
https://bugzilla.redhat.com/show_bug.cgi?id=2181020
--- Comment #7 from fedora.dm0@gmail.com --- I've added a patch to remove the crate. I don't know what is the cleanest way to disable integration tests--I tried to use conditional compilation instead of deletion to minimize the patch and reduce chances of conflicts during upgrades. If this looks okay, we can close this request: https://github.com/dm0-/copr-firecracker/blob/f459fc7c0ec178ca1ecfb3795eb4b3...
https://bugzilla.redhat.com/show_bug.cgi?id=2181020
--- Comment #8 from Fabio Valentini decathorpe@gmail.com --- The easiest solution for skipping a file with integration tests is:
$ rm src/firecracker/tests/verify_dependencies.rs
Otherwise, that looks good to me. Feel free to close this ticket as CLOSED/NOTABUG if you no longer need cargo_toml packaged.
https://bugzilla.redhat.com/show_bug.cgi?id=2181020
fedora.dm0@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |CLOSED Resolution|--- |NOTABUG Last Closed| |2023-03-27 16:49:25
package-review@lists.fedoraproject.org