Bug 2181022 - Review Request: rust-device_tree - Reads and parses Linux device tree images
Summary: Review Request: rust-device_tree - Reads and parses Linux device tree images
Keywords:
Status: CLOSED NOTABUG
Alias: None
Product: Fedora
Classification: Fedora
Component: Package Review
Version: rawhide
Hardware: All
OS: Linux
medium
medium
Target Milestone: ---
Assignee: Fabio Valentini
QA Contact: Fedora Extras Quality Assurance
URL: https://crates.io/crates/device_tree
Whiteboard:
Depends On:
Blocks: 2181039
TreeView+ depends on / blocked
 
Reported: 2023-03-22 21:32 UTC by fedora.dm0
Modified: 2023-03-27 16:55 UTC (History)
2 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2023-03-27 16:48:40 UTC
Type: ---
Embargoed:
decathorpe: fedora-review?


Attachments (Terms of Use)

Description fedora.dm0 2023-03-22 21:32:11 UTC
Spec URL: https://github.com/dm0-/copr-firecracker/raw/ea04a8d1c6d303e5cf61e62b077fe4b6b0200d5f/rust-device_tree.spec
SRPM URL: https://github.com/dm0-/copr-firecracker/raw/ea04a8d1c6d303e5cf61e62b077fe4b6b0200d5f/rust-device_tree-1.1.0-1.fc37.src.rpm
Description: Reads and parses Linux device tree images.
Fedora Account System Username: dm0

This is a build dependency of Firecracker.  The spec is automatically generated.  The upstream crate is missing license files but hasn't had commits in five years.

Comment 1 Jakub Kadlčík 2023-03-22 21:37:33 UTC
Copr build:
https://copr.fedorainfracloud.org/coprs/build/5696232
(succeeded)

Review template:
https://download.copr.fedorainfracloud.org/results/@fedora-review/fedora-review-2181022-rust-device_tree/fedora-rawhide-x86_64/05696232-rust-device_tree/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.

Comment 2 Fabio Valentini 2023-03-23 14:11:15 UTC
Package looks good, with only one problem:
There's no license text included with published sources, but the MIT license requires this.

There's also no license text in the upstream project:
https://github.com/mbr/device_tree-rs

I already posted more details here:
https://bugzilla.redhat.com/show_bug.cgi?id=2181020#c2

Comment 3 fedora.dm0 2023-03-23 14:49:38 UTC
I noted in the original comment that the project hasn't been active for five years.  Is it enough to just submit a bug about the license?  Should the SRPM add the MIT license file itself if upstream doesn't respond?

Comment 4 Fabio Valentini 2023-03-23 14:53:27 UTC
This is unfortunate, but it doesn't absolve us from abiding by the projects (intended?) license terms.

Since the project looks abandoned, the "best effort" solution that would let us move forward is probably
- submitting a PR to the project to add the missing license file, and
- include the file from the PR in the Fedora package.

Comment 5 fedora.dm0 2023-03-23 15:32:48 UTC
The actual use of this crate just seems to be for running tests on aarch64, so another option could be to skip the tests and drop it if that's easier.  I can try a PR first, though.

Comment 6 Fabio Valentini 2023-03-23 15:42:40 UTC
Yes, if this crate is only needed to run some tests, dropping it and the tests that require it would be another valid option.

Comment 7 fedora.dm0 2023-03-24 14:30:09 UTC
Looking a bit closer at this, it seems the version of this crate on crates.io was never pushed to a Git repo.  It is generating a bunch of deprecation warnings, too.  Given these issues, I think I'd prefer to drop it.  I added this patch to disable building its two tests, so if this looks okay, we can close this request: https://github.com/dm0-/copr-firecracker/blob/616ea4e89c89ed937204000e18cd1070e2f18166/firecracker-1.3.1-remove-device_tree.patch

Comment 8 Fabio Valentini 2023-03-27 16:23:25 UTC
This is not doing what you think it is:

`#[cfg(not(test))]`

This means "include this code only if we're not building tests" (i.e. the code will not end up in test binaries, but in the actual library).

You need to remove the tests entirely. I don't think there's something like an `#[cfg(false)]` attribute.

Other than that, yes, I agree, this crate looks fishy, and dropping it is probably wise.
Feel free to close this ticket as CLOSED/NOTABUG.

Comment 9 fedora.dm0 2023-03-27 16:48:40 UTC
I used not(test) because the functions are in a #[test] module, so I think it's an impossible condition (as close as I could get to #[cfg(false)]).

Comment 10 Fabio Valentini 2023-03-27 16:55:23 UTC
Ah, that's true ... this situation just leads to code that's "unreachable" by the compiler:

#[cfg(test)]
mod tests {
    #[cfg(not(test))]
    fn test() {}
}

It's a bit of a hack though :)


Note You need to log in before you can comment on or make changes to this bug.