https://bugzilla.redhat.com/show_bug.cgi?id=2238751
--- Comment #11 from Marc-Andre Lureau <marcandre.lureau(a)redhat.com> ---
(In reply to Fabio Valentini from comment #8)
Thanks for the update, and sorry for the wait.
No worries (the QEMU side isn't yet merged upstream, and I am not sure when we
will actually have users)
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.
fixed
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.
ok
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.
It seems they considered it:
https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4082335
I don't see what difference there is between the Chromium LICENSE and BSD-3
though.
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)
"""
fixed, thanks
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
"""
done
(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?
done
7. You could drop the BuildRequires for "make" since it is no longer used.
ok
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
ok
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).
Hmm, I thought it was there initially and you asked me to move it. I realize
now you just asked for an extra empty line, ok.. (confusing)
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.
ok
--
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=rep...