https://bugzilla.redhat.com/show_bug.cgi?id=1590425
Christophe de Dinechin <dinechin(a)redhat.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Flags|needinfo?(dinechin(a)redhat.c |
|om) |
--- Comment #11 from Christophe de Dinechin <dinechin(a)redhat.com> ---
(In reply to Cole Robinson from comment #10)
Using the SRPM URL: and Spec URL: format in Comment #3 helps simplify
the
review process, fedora-review tool can just be pointed at the bug. So for
next posting please provide that as well
OK, sorry.
* The kata-runtime binary should be in /usr/bin, which is where upstream
puts it. I don't see any reason to deviate
One arguable reason is that historical docker put its runtime there,
specifically in /usr/libexec/docker/docker-runc-current.
A second one is that I don't like polluting /usr/bin with commands that
are not intended for end-users. But that's just me.
A third reason is lack of manpages and silencing a warning from fedora-review.
The first argument is quite weak, based on other container software:
The moby-engine package has only docker and dockerd in /usr/bin,
and then has:
/usr/libexec/docker/docker-init
/usr/libexec/docker/docker-proxy
but I believe it relies on runc, which is /usr/bin/runc.
Podman is looking for runc all over the place:
# Paths to look for a valid OCI runtime (runc, runv, etc)
[runtimes]
runc = [
"/usr/bin/runc",
"/usr/sbin/runc",
"/usr/local/bin/runc",
"/usr/local/sbin/runc",
"/sbin/runc",
"/bin/runc",
"/usr/lib/cri-o-runc/sbin/runc"
]
crun = [
"/usr/bin/crun",
"/usr/local/bin/crun",
]
So there does not seem to be any settled theory.
I will do the change back to /usr/bin, but reluctantly :-)
* godocs/golicense/files: only a single LICENSE file ends up in the rpm. I
don't think the godocs/golicense stuff is
playing well with the %files section, but I'm not sure. either way, I
don't think we need to put anything more
than the top level LICENSE and README.md into the RPM. The LICENSEs are
all the same apache version, and most of
of the other docs seem fine to just exist in upstream git.
I pasted there the output of go2rpm here. I assume they have a rationale for
this, and I don't want to break their approach unnecessarily. Who knows if
they have some scripts scourging rpms for these macros, etc.
In short, I don't understand the implications, but at least regarding license,
I suspect this may be related to the Tech Talk given by David Fontana
recently, regarding the problem with the historical license compliance model
for Fedora and Red Hat when confronted with languages that have their own
package manager.
To make a long story short, the way I understand it is that historically,
we complied by shipping the source, assuming that the license was in the
source.
For languages such as go or nodejs or Java, this may no longer be the case,
i.e. you may actually pull some additional code (thus some extra licenses)
in binary format without knowing it.
I imagine that this whole license fishing done by go2rpm may be an attempt to
address that problem.
So for now, I would prefer to leave it as is, unless it triggers a warning.
However, I did add a big fat comment explaining where this came from.
We can remove it later if the go folks confirm it is useless.
* The BuildRequires: compiler(go-compiler) is not required, go-rpm-macros
does that for us
I think this is true for go-rpm-macros (part of %gometa)
I don't think this is true for compiler(go-compiler), rpm -qR on src rpm
does not show compiler(go-compiler) anymore if I remove it.
* The comment about a 'rediculous' name from goname is not necessary. IMO
it's self evident why
goname does not apply here, it doesn't need the commentary
Was not obvious to me ;-)
Rephrased the comment to avoid offensive mention of "ridiculous"
Trimmed fedora-review output below, with some comments.
[ ]: Package must own all directories that it creates.
Note: Directories without known owners: /usr/share/defaults
* Yes, seems like this should be a %dir in the %files list. This is
a non-standard FHS directory I believe, but it will take work with
kata upstream to correct it, so it should stay for now IMO>
Moved it to /usr/share/kata-containers/defaults
(just needed to pass a variable to the makefile)
We already have a /usr/share/kata-containers for images.
Which package(s) need a %dir for it? I added one, not sure it's correct.
[ ]: %build honors applicable compiler flags or justifies otherwise.
* We talked about this offline. Use of %gobuild is not easy in this
package because 'make' handles a bunch of other stuff. So distro
go build flags aren't making it into the build. Next release
of go-rpm-macros should have a %gobuildflags macro that will help
here, but it isn't available yet. Christophe, please add a comment
to that effect in the spec, so we don't forget, and optionally
link to the go-rpm-macros commit:
https://pagure.io/go-rpm-macros/c/
67b4fbbbfce0986ac46cd1329bf85a18ea7a43d2?branch=master
I tried this macro (by locally patching the macro files), but I
get errors using it. It expands to:
'BUILDFLAGS=%{gocompilerflags} -tags="rpm_crashtraceback " -ldflags
"-X
github.com/kata-containers/runtime/version=1.8.2 -X
github.com/kata-containers/runtime/version.tag=1.8.2 -B
0x60831d87667855a13661dfaf1b99ee957b4de2a4 -extldflags '\''-Wl,-z,relro
-Wl,--as-needed -Wl,-z,now -specs=/usr/lib/rpm/redhat/redhat-hardened-ld
'\''"
-a -v -x'
The %{gocompilerflags} at the beginning causes problems. Gave up for now.
[ ]: Package is not known to require an ExcludeArch tag.
* ExcludeArch is necessary as dependent kata components do not build on
32 bit archs. This is a known issue that isn't kata-runtimes fault.
Doesn't block this review
There is also an ExcludeArch that comes from %gometa.
[!]: Reviewer should test that the package builds in mock.
* worked fine for me
[!]: Uses parallel make %{?_smp_mflags} macro.
* indeed, spec should use %make_build and %make_install macros in place of
the direct
'make' calls. But if this causes build issues, just drop them and mention
it in
the next submission
Done.
Rpmlint
-------
Checking: kata-runtime-1.8.2-2.fc32.x86_64.rpm
kata-runtime-debuginfo-1.8.2-2.fc32.x86_64.rpm
kata-runtime-debugsource-1.8.2-2.fc32.x86_64.rpm
kata-runtime-1.8.2-2.fc32.src.rpm
kata-runtime.x86_64: W: no-manual-page-for-binary kata-collect-data.sh
4 packages and 0 specfiles checked; 0 errors, 1 warnings.
* This issue can be safely ignored. I think we may not want to ship this
script, or move it elsewhere, but we can figure it out after package import
You'll have more warnings about missing man pages after reverting
the binaries from /usr/libexec to /usr/bin.
--
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