[Bug 2181020] New: Review Request: rust-cargo_toml - Cargo.toml struct definitions for parsing with Serde

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(a)fedoraproject.org
          Reporter: fedora.dm0(a)gmail.com
        QA Contact: extras-qa(a)fedoraproject.org
                CC: package-review(a)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....
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


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2181020

[Bug 2181020] Review Request: rust-cargo_toml - Cargo.toml struct definitions for parsing with Serde

https://bugzilla.redhat.com/show_bug.cgi?id=2181020

Jakub Kadlčík <jkadlcik(a)redhat.com&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                URL|                            |https://crates.io/crates/ca
                   |                            |rgo_toml



--- Comment #1 from Jakub Kadlčík <jkadlcik(a)redhat.com&gt; ---
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5696212
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-...

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.


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2181020

[Bug 2181020] Review Request: rust-cargo_toml - Cargo.toml struct definitions for parsing with Serde

https://bugzilla.redhat.com/show_bug.cgi?id=2181020

Fabio Valentini <decathorpe(a)gmail.com&gt; changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |decathorpe(a)gmail.com
           Doc Type|---                         |If docs needed, set a value
             Status|NEW                         |ASSIGNED
              Flags|                            |fedora-review?
           Assignee|nobody(a)fedoraproject.org    |decathorpe(a)gmail.com



--- Comment #2 from Fabio Valentini <decathorpe(a)gmail.com&gt; ---
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.


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
https://bugzilla.redhat.com/show_bug.cgi?id=2181020

[Bug 2181020] Review Request: rust-cargo_toml - Cargo.toml struct definitions for parsing with Serde

https://bugzilla.redhat.com/show_bug.cgi?id=2181020



--- Comment #3 from fedora.dm0(a)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/firecr...


-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
https://bugzilla.redhat.com/show_bug.cgi?id=2181020

[Bug 2181020] Review Request: rust-cargo_toml - Cargo.toml struct definitions for parsing with Serde

https://bugzilla.redhat.com/show_bug.cgi?id=2181020



--- Comment #4 from Fabio Valentini <decathorpe(a)gmail.com&gt; ---
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.


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2181020

[Bug 2181020] Review Request: rust-cargo_toml - Cargo.toml struct definitions for parsing with Serde

https://bugzilla.redhat.com/show_bug.cgi?id=2181020



--- Comment #5 from fedora.dm0(a)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/firecrac...


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2181020

[Bug 2181020] Review Request: rust-cargo_toml - Cargo.toml struct definitions for parsing with Serde

https://bugzilla.redhat.com/show_bug.cgi?id=2181020



--- Comment #6 from Fabio Valentini <decathorpe(a)gmail.com&gt; ---
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.


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2181020

[Bug 2181020] Review Request: rust-cargo_toml - Cargo.toml struct definitions for parsing with Serde

https://bugzilla.redhat.com/show_bug.cgi?id=2181020



--- Comment #7 from fedora.dm0(a)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/f459fc7c0ec178ca1ecfb3795eb...


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2181020

[Bug 2181020] Review Request: rust-cargo_toml - Cargo.toml struct definitions for parsing with Serde

https://bugzilla.redhat.com/show_bug.cgi?id=2181020



--- Comment #8 from Fabio Valentini <decathorpe(a)gmail.com&gt; ---
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.


-- 
You are receiving this mail because:
You are always notified about changes to this product and component
You are on the CC list for the bug.
https://bugzilla.redhat.com/show_bug.cgi?id=2181020

[Bug 2181020] Review Request: rust-cargo_toml - Cargo.toml struct definitions for parsing with Serde

https://bugzilla.redhat.com/show_bug.cgi?id=2181020

fedora.dm0(a)gmail.com changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |CLOSED
         Resolution|---                         |NOTABUG
        Last Closed|                            |2023-03-27 16:49:25




-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
https://bugzilla.redhat.com/show_bug.cgi?id=2181020