[Bug 2238751] Review Request: rutabaga-gfx-ffi - Handling virtio-gpu protocols

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



--- Comment #8 from Fabio Valentini <decathorpe@xxxxxxxxx> ---
Thanks for the update, and sorry for the wait.
Package looks mostly good to me, there's just a few remaining issues:

1. The "make-snapshot.sh" script is not included in the SRPM. It should be
included as a SOURCE, otherwise it's impossible to re-create the srpm from its
contents.

2. The %cargo_license macro has an "-a" flag (for "--all-features"), which will
make its results include things that aren't actually linked into the binary.
Please drop the "-a" flag here.

3. The crates do not set their license in crate metadata, which trips up the
%cargo_license* macros. It would be great if upstream set `package.license =
"BSD-3-Clause"` for both crates ... but until that's the case, it might be a
good idea to patch that in manually so the macros produce correct / complete
output.

4. The licenses of all statically linked dependencies are listed in
LICENSE.dependencies, but the License tag itself does not reflect them. Looking
at the contents of the LICENSE.dependencies file from the built package, the
License tag in the spec should look something like this:

"""
# rutabaga_gfx: BSD-3-Clause
# crate dependencies:
# - BSD-2-Clause
# - MIT OR Apache-2.0
# - MIT
# - Unlicense OR MIT
License:        BSD-3-Clause AND BSD-2-Clause AND MIT AND (MIT OR Apache-2.0)
AND (Unlicense OR MIT)
"""

5. The patches don't have explanations or justifications, please add a short
comment for each one, like

"""
# drop a Windows-specific dependency
Patch0000:  drop-winapi.patch
# fix filename for installed library and set up symlinks correctly
Patch0001:  fix-libs.patch
# set library SONAME correctly
Patch0002:  set-soname.patch
"""

(I'm assuming that's the reason for the patches, but I'm just guessing, since
they're not documented yet :))
>From what it looks like, the "fix-libs.patch" could be dropped entirely, since
"make" is no longer used to install the files?

7. You could drop the BuildRequires for "make" since it is no longer used.

8. There's no URL to the upstream project in the package metadata.

I would add this instead of only having the url in a comment:
URL: https://chromium.googlesource.com/crosvm/crosvm

9. The placement of the "%generate_buildrequires" scriptlet (between RPM tags
and %description) is very unusual. Please place it between %prep and %build
(since that's the order in which the scriptlets are executed).

10. Please bump the BuildRequires from "rust-packaging >= 21" to
"cargo-rpm-macros".
The "rust-packaging" package no longer exists and is only provided by
"cargo-rpm-macros" for backwards compatibility.


-- 
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=2238751

Report this comment as SPAM: https://bugzilla.redhat.com/enter_bug.cgi?product=Bugzilla&format=report-spam&short_desc=Report%20of%20Bug%202238751%23c8
_______________________________________________
package-review mailing list -- package-review@xxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe send an email to package-review-leave@xxxxxxxxxxxxxxxxxxxxxxx
Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/
List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines
List Archives: https://lists.fedoraproject.org/archives/list/package-review@xxxxxxxxxxxxxxxxxxxxxxx
Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue




[Index of Archives]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite Conditions]     [KDE Users]

  Powered by Linux