On Thu, Feb 07, 2019 at 04:21:34PM +0100, Andrea Bolognani wrote:
On Tue, 2019-02-05 at 17:53 +0000, Daniel P. Berrangé wrote:
[...]
> With the Debian 9 distro, this supports arm64, armel, armhf, mips,
> mipsel, mips64el, ppc64el, s390x, which are all the official archs
> that Debian maintainers currently build libvirt for. With Debian Sid,
> this is extended to include i386.
Is i386 really not supported as a cross compilation target on Debian
9? It seems like it would be such a low hanging fruit...
Yes, the i386 gcc simply doesn't exist in 9. No idea why
[...]
> +++ b/guests/host_vars/libvirt-debian-9/docker.yml
> @@ -1,2 +1,59 @@
> ---
> docker_base: debian:9
> +cross_build:
I don't think this belongs in docker.yml, since there's nothing
specific to Docker here: we could just as well use this information
to build a virtual machine in which to perform cross builds.
I would create a new fact file, eg. cross-build.yml, for this.
Ok. In practice this split doesn't seem to actually do anything
since we load all files & take the union of their contents.
> + blacklist:
> + # Doesn't properly resolve from foreign arch package
> + # to the native package of augeas-lenses
I cannot really understand what you're trying to say with this
comment, can you try to reword it please?
dpkg fails to resolve the deps. It wants augeas-lenses:s390x
when it should be satisfied with augeas-lenses:$NATIVE. You
can't install both augeas-lenses as they conflict on files.
> + - libnetcf-dev
> + # Fails to resolve deps from foreign arch
> + - wireshark-dev
> + # dtrace tool doesn't know how to cross-compile
> + - systemtap-sdt-dev
For these and all other blacklisted packages, you should list the
name of the mapping (eg. netcf) rather than the concrete Debian
package name (eg. libnetcf-dev). Then of course you'll have to act
on the blacklist earlier, before mapping is performed.
I don't find that desirable. This is a debian specific config
file, and it is clearer to list the actual debian packages we
have trouble with, as that's what apt-get is complaining about
when it fails. Using the generic mapping obscures what is
going on.
> + arches:
> + arm64:
> + gcc-pkg-prefix: crossbuild-essential-arm64
This is not a prefix, and the package is not GCC :)
I would use 'cross_gcc', which incidentally is also the name of the
variable you use in the Python script to store this value. (More on
this later, though.)
Actually this entire variable can be eliminated, as we can derive
it from the target name.
> + target-prefix: aarch64-linux-gnu
Same here, you later use 'cross_prefix' in the script but also store
it as 'TARGET' in the environment... I'd suggest 'cross_target'.
Using "cross_" is redundant as this already inside a parent
"cross-build"
block.
[...]
> + mipsel:
> + gcc-pkg-prefix: gcc-mipsel-linux-gnu
crossbuild-essential-mipsel is available in Debian 9, so you should
use that instead.
Actually, I've eliminated all usage of the crossbuild-essential-*
packages. These needlessly pull in the C++ compiler too.
> docker_base: debian:sid
> +cross_build:
> + blacklist:
> + # Doesn't properly resolve from foreign arch package
> + # to the native package of augeas-lenses
> + - libnetcf-dev
> + # Fails to resolve deps from foreign arch
> + - wireshark-dev
> + # dtrace tool doesn't know how to cross-compile
> + - systemtap-sdt-dev
> + # appears to be dropped from the 'sid' tree
> + - sheepdog
So it has! And it's apparently not going to be in the next Ubuntu
release, either.
What's missing that we've not seen this problem already ? Is it
just because we don't regularlyl rebuild the CI hosts / images ?
> + if cross_arch and pkgname[-4:] ==
"-dev":
> + if pkgname not in cross_pkgs:
> + cross_pkgs.append(pkgname + ":" + cross_arch)
I can see how sticking with the Debian-style architecture names
makes this bit trivial, but I'm still entirely convinced we should
use the same architecture names as libvirt does.
Especially once we'll have integrated this into libvirt's own
build
system, the difference in names would introduce needless friction,
as the target audience (libvirt developers) is certainly more
familiar with the architecture names used in that project than
Debian's.
Using the libvirt arch names adds no value. There's no friction as
there is no context in which we're using the libvirt arch names &
need a mapping to the cross build names. These image configs are
distro specific and using the distro's native arch is directly
relevant to the build results.
> + ENV TARGET "%(cross_prefix)s"
> + ENV CONFIGURE_OPTS "--host=%(cross_prefix)s
--target=%(cross_prefix)s"
> + ENV PKG_CONFIG_LIBDIR
"/usr/lib/%(cross_prefix)s/pkgconfig"
As you mention in a previous commit message, each ENV statement
will create and additional layer, and it's considered best practice
to have as few of those as possible.
The difference is the usage context. The ENV statement is intended
to provide environment variables to runtime users of the image. The
previous ENV usage was merely used internally to the docker build
and was polluting the runtime env where it was not required.
These ENV statements are intended for direct usage in the docker
images at runtime, so you can launch the image with docker and
simply type "$TARGET-gcc" or "./configure $CONFIGURE_OPTS" and
have it do the right thing.
Another reason why I don't like this is that it's different
from
what we already do for the "cross compilation" target we already
support, MinGW: in that case, we have a single image with pristine
environment, and we inject the additional variables only when
actually running the build. In my opinion, that's a saner way to
tweak the build, since the interaction is very explicit and doesn't
require any knowledge of other components that might even be
hosted, as is the case here, in a separate git repository!
The mingw images are slightly different in that the Fedora mingw
RPMs have invented a "mingw32-configure" program that sets all
these env variables already. If that program didn't exist, then
it would be desirable to have the same env vars set for the mingw
images too.
Additionally, and I only thought about this now :) did we consider
the possibility of providing a single Debian image with all cross
compilation packages installed, rather than one image per target
architecture? Assuming that's feasible, it would also fit very
nicely with how we have a single Fedora image that we use for both
MinGW variants...
Creating a single image would mean a significantly larger download
image size when you only care about testing a single architecture.
Because of the way image layer sharing works, having lots of
separate images ensures you can have a minimal download size for
a single image, while not giving duplication if you need to download
all images.
We ought to have a separate Fedora image for each MinGW target for
this same reason in fact.
Regards,
Daniel
--
|:
https://berrange.com -o-
https://www.flickr.com/photos/dberrange :|
|:
https://libvirt.org -o-
https://fstop138.berrange.com :|
|:
https://entangle-photo.org -o-
https://www.instagram.com/dberrange :|