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
