[libvirt] [jenkins-ci PATCH v5 5/5] lcitool: support generating cross compiler dockerfiles

Daniel P. Berrangé posted 5 patches 6 years, 11 months ago
There is a newer version of this series
[libvirt] [jenkins-ci PATCH v5 5/5] lcitool: support generating cross compiler dockerfiles
Posted by Daniel P. Berrangé 6 years, 11 months ago
Debian's filesystem layout has a nice advantage over Fedora which is
that it can install non-native RPMs in the main root filesystem. It is
thus possible to prepare an x86_64 filesystem containing -dev packages
for a foreign architecture, along with a GCC cross compiler.

QEMU has used this technique to facilitate developer build testing of
non-x86 architectures, since few people have access to physical
hardware for most of these architectures. For the same reason it would
be helpful to libvirt developers.

This patch extends the 'dockerfile' command to 'lcitool' so that it
accepts a '-x $ARCH' argument.

  $ lcitool dockerfile -x s390x libvirt-debian-9 libvirt

This is only valid when using a 'deb' based distro.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 guests/host_vars/libvirt-debian-9/main.yml   | 44 +++++++++++++
 guests/host_vars/libvirt-debian-sid/main.yml | 45 ++++++++++++++
 guests/lcitool                               | 65 +++++++++++++++++++-
 guests/vars/mappings.yml                     | 59 ++++++++++++++++++
 4 files changed, 211 insertions(+), 2 deletions(-)

diff --git a/guests/host_vars/libvirt-debian-9/main.yml b/guests/host_vars/libvirt-debian-9/main.yml
index ec7e6b4..3bf4ae1 100644
--- a/guests/host_vars/libvirt-debian-9/main.yml
+++ b/guests/host_vars/libvirt-debian-9/main.yml
@@ -21,3 +21,47 @@ os_name: 'Debian'
 os_version: '9'
 
 ansible_python_interpreter: /usr/bin/python3
+
+arches:
+  aarch64:
+    package_arch: arm64
+    abi: aarch64-linux-gnu
+    cross_gcc: gcc-aarch64-linux-gnu
+  armv6l:
+    package_arch: armel
+    abi: arm-linux-gnueabi
+    cross_gcc: gcc-arm-linux-gnueabi
+  armv7l:
+    package_arch: armhf
+    abi: arm-linux-gnueabihf
+    cross_gcc: gcc-arm-linux-gnueabihf
+  i686:
+    package_arch: i386
+    abi: i386-linux-gnu
+  mips:
+    package_arch: mips
+    abi: mips-linux-gnu
+    cross_gcc: gcc-mips-linux-gnu
+  mipsel:
+    package_arch: mipsel
+    abi: mipsel-linux-gnu
+    cross_gcc: gcc-mipsel-linux-gnu
+  mips64:
+    package_arch: mips64
+    abi: mips64-linux-gnu
+    cross_gcc: gcc-mips64-linux-gnu
+  mips64el:
+    package_arch: mips64el
+    abi: mips64el-linux-gnu
+    cross_gcc: gcc-mips64el-linux-gnu
+  ppc64el:
+    package_arch: ppc64el
+    abi: ppc64el-linux-gnu
+    cross_gcc: gcc-ppc64el-linux-gnu
+  s390x:
+    package_arch: s390x
+    abi: s390x-linux-gnu
+    cross_gcc: gcc-s390x-linux-gnu
+  x86_64:
+    package_arch: amd64
+    abi: x86_64-linux-gnu
diff --git a/guests/host_vars/libvirt-debian-sid/main.yml b/guests/host_vars/libvirt-debian-sid/main.yml
index 1c7a29b..b14a564 100644
--- a/guests/host_vars/libvirt-debian-sid/main.yml
+++ b/guests/host_vars/libvirt-debian-sid/main.yml
@@ -21,3 +21,48 @@ os_name: 'Debian'
 os_version: 'Sid'
 
 ansible_python_interpreter: /usr/bin/python3
+
+arches:
+  aarch64:
+    package_arch: arm64
+    abi: aarch64-linux-gnu
+    cross_gcc: gcc-aarch64-linux-gnu
+  armv6l:
+    package_arch: armel
+    abi: arm-linux-gnueabi
+    cross_gcc: gcc-arm-linux-gnueabi
+  armv7l:
+    package_arch: armhf
+    abi: arm-linux-gnueabihf
+    cross_gcc: gcc-arm-linux-gnueabihf
+  i686:
+    package_arch: i386
+    abi: i686-linux-gnu
+    cross_gcc: gcc-i686-linux-gnu
+  mips:
+    package_arch: mips
+    abi: mips-linux-gnu
+    cross_gcc: gcc-mips-linux-gnu
+  mipsel:
+    package_arch: mipsel
+    abi: mipsel-linux-gnu
+    cross_gcc: gcc-mipsel-linux-gnu
+  mips64:
+    package_arch: mips64
+    abi: mips64-linux-gnu
+    cross_gcc: gcc-mips64-linux-gnu
+  mips64el:
+    package_arch: mips64el
+    abi: mips64el-linux-gnu
+    cross_gcc: gcc-mips64el-linux-gnu
+  ppc64el:
+    package_arch: ppc64el
+    abi: ppc64el-linux-gnu
+    cross_gcc: gcc-ppc64el-linux-gnu
+  s390x:
+    package_arch: s390x
+    abi: s390x-linux-gnu
+    cross_gcc: gcc-s390x-linux-gnu
+  x86_64:
+    package_arch: amd64
+    abi: x86_64-linux-gnu
diff --git a/guests/lcitool b/guests/lcitool
index 263ab0d..0fb30a5 100755
--- a/guests/lcitool
+++ b/guests/lcitool
@@ -367,6 +367,10 @@ class Application:
 
         add_hosts_arg(dockerfileparser)
         add_projects_arg(dockerfileparser)
+        dockerfileparser.add_argument(
+            "-x", "--cross-arch",
+            help="cross compiler architecture",
+        )
 
     def _execute_playbook(self, playbook, hosts, projects, git_revision):
         base = Util.get_base()
@@ -537,10 +541,23 @@ class Application:
         os_name = facts["os_name"]
         os_version = facts["os_version"]
         os_full = os_name + os_version
+        cross_facts = None
 
         if package_format not in ["deb", "rpm"]:
             raise Error("Host {} doesn't support Dockerfiles".format(host))
 
+        if args.cross_arch is not None:
+            if "arches" not in facts:
+                raise Error("Non x86_64 arches not supported for this host")
+            if args.cross_arch not in facts["arches"]:
+                raise Error("Arch {} not supported for this host".format(
+                    args.cross_arch))
+            if "cross_gcc" not in facts["arches"][args.cross_arch]:
+                raise Error("Arch {} cross compiler not supported for this host".format
+                            (args.cross_arch))
+
+            cross_build_facts = facts["arches"][args.cross_arch]
+
         projects = self._projects.expand_pattern(args.projects)
         for project in projects:
             if project not in facts["projects"]:
@@ -552,25 +569,50 @@ class Application:
                 )
 
         pkgs = {}
+        cross_pkgs = {}
         base_keys = ["default", package_format, os_name, os_full]
-        keys = base_keys + [k + "-" + self._native_arch for k in base_keys]
+        cross_keys = []
+        if args.cross_arch:
+            keys = base_keys + [k + "-" + args.cross_arch for k in base_keys]
+            cross_keys = [k + "-cross-arch" for k in base_keys]
+        else:
+            keys = base_keys + [k + "-" + self._native_arch for k in base_keys]
+
         # We need to add the base project manually here: the standard
         # machinery hides it because it's an implementation detail
         for project in projects + ["base"]:
             for package in self._projects.get_packages(project):
+                policy = "native"
+                for key in cross_keys:
+                    if key in mappings[package]:
+                        policy = mappings[package][key]
+                if policy not in ["native", "foreign", "skip"]:
+                    raise Error("Unexpected policy {} for {}",
+                                policy, package)
+
                 for key in keys:
                     if key in mappings[package]:
                         pkgs[package] = mappings[package][key]
 
                 if package not in pkgs:
                     continue
-                if pkgs[package] is None:
+                if policy == "foreign" and pkgs[package] is not None:
+                    cross_pkgs[package] = pkgs[package]
+                if pkgs[package] is None or policy in ["skip", "foreign"]:
                     del pkgs[package]
 
         print("FROM {}".format(facts["docker_base"]))
 
         varmap = {}
         varmap["pkgs"] = " \\\n            ".join(sorted(set(pkgs.values())))
+        if args.cross_arch:
+            if package_format != "deb":
+                raise Error("Cannot install foreign {} packages".format(package_format))
+            varmap["cross_arch"] = cross_build_facts["package_arch"]
+            pkg_names = [p + ":" + cross_build_facts["package_arch"] for p in cross_pkgs.values()]
+            pkg_names.append(cross_build_facts["cross_gcc"])
+            varmap["cross_pkgs"] = " \\\n            ".join(sorted(set(pkg_names)))
+            varmap["cross_target"] = cross_build_facts["abi"]
         if package_format == "deb":
             sys.stdout.write(textwrap.dedent("""
                 RUN export DEBIAN_FRONTEND=noninteractive && \\
@@ -581,6 +623,25 @@ class Application:
                     apt-get autoremove -y && \\
                     apt-get autoclean -y
             """).format(**varmap))
+            if args.cross_arch:
+                # Intentionally a separate RUN command from the above
+                # so that the common packages of all cross-built images
+                # share a docker image layer.
+                sys.stdout.write(textwrap.dedent("""
+                    RUN export DEBIAN_FRONTEND=noninteractive && \\
+                        dpkg --add-architecture {cross_arch} && \\
+                        apt-get update && \\
+                        apt-get dist-upgrade -y && \\
+                        apt-get install --no-install-recommends -y \\
+                                {cross_pkgs} && \\
+                        apt-get autoremove -y && \\
+                        apt-get autoclean -y
+
+                    ENV TARGET "{cross_target}"
+                    ENV CONFIGURE_OPTS "--host={cross_target} \\
+                                        --target={cross_target}"
+                    ENV PKG_CONFIG_LIBDIR "/usr/lib/{cross_target}/pkgconfig"
+                """).format(**varmap))
         elif package_format == "rpm":
             if os_name == "Fedora" and os_version == "Rawhide":
                 sys.stdout.write(textwrap.dedent("""
diff --git a/guests/vars/mappings.yml b/guests/vars/mappings.yml
index ea46ce4..ebf7e60 100644
--- a/guests/vars/mappings.yml
+++ b/guests/vars/mappings.yml
@@ -57,11 +57,15 @@
 #    deb-x86_64: libxen-dev
 #    Fedora-x86_64: xen-devel
 #
+# In parallel with this 'cross-arch: native|foreign|skip' entries can
+# used to indicate the policy when setting up a cross-architecture
+# build environment. If omitted 'native' is assumed
 
 mappings:
 
   apparmor:
     deb: libapparmor-dev
+    deb-cross-arch: foreign
 
   augeas:
     default: augeas
@@ -80,6 +84,7 @@ mappings:
 
   avahi:
     deb: libavahi-client-dev
+    deb-cross-arch: foreign
     pkg: avahi
     rpm: avahi-devel
 
@@ -106,6 +111,7 @@ mappings:
 
   cyrus-sasl:
     deb: libsasl2-dev
+    deb-cross-arch: foreign
     pkg: cyrus-sasl
     rpm: cyrus-sasl-devel
 
@@ -116,6 +122,7 @@ mappings:
 
   device-mapper:
     deb: libdevmapper-dev
+    deb-cross-arch: foreign
     rpm: device-mapper-devel
 
   dnsmasq:
@@ -124,6 +131,7 @@ mappings:
 
   dtrace:
     deb: systemtap-sdt-dev
+    deb-cross-arch: skip
     rpm: systemtap-sdt-devel
 
   dwarves:
@@ -144,6 +152,7 @@ mappings:
 
   fuse:
     deb: libfuse-dev
+    deb-cross-arch: foreign
     pkg: fusefs-libs
     rpm: fuse-devel
 
@@ -159,19 +168,23 @@ mappings:
 
   glib2:
     deb: libglib2.0-dev
+    deb-cross-arch: foreign
     pkg: glib
     rpm: glib2-devel
 
   glibc:
     deb: libc6-dev
+    deb-cross-arch: foreign
     rpm: glibc-devel
 
   glibc-static:
     deb: libc6-dev
+    deb-cross-arch: foreign
     rpm: glibc-static
 
   glusterfs:
     deb: libglusterfs-dev
+    deb-cross-arch: foreign
     rpm: glusterfs-api-devel
     Debian8: glusterfs-common
     Debian9: glusterfs-common
@@ -183,6 +196,7 @@ mappings:
 
   gnutls:
     deb: libgnutls28-dev
+    deb-cross-arch: foreign
     pkg: gnutls
     rpm: gnutls-devel
 
@@ -192,11 +206,13 @@ mappings:
 
   gobject-introspection:
     deb: libgirepository1.0-dev
+    deb-cross-arch: foreign
     pkg: gobject-introspection
     rpm: gobject-introspection-devel
 
   gtk3:
     deb: libgtk-3-dev
+    deb-cross-arch: foreign
     pkg: gtk3
     rpm: gtk3-devel
 
@@ -211,6 +227,7 @@ mappings:
 
   gtk-vnc2:
     deb: libgtk-vnc-2.0-dev
+    deb-cross-arch: foreign
     pkg: gtk-vnc
     rpm: gtk-vnc2-devel
 
@@ -243,32 +260,39 @@ mappings:
 
   json-glib:
     deb: libjson-glib-dev
+    deb-cross-arch: foreign
     pkg: json-glib
     rpm: json-glib-devel
 
   libacl:
     deb: libacl1-dev
+    deb-cross-arch: foreign
     rpm: libacl-devel
 
   libarchive:
     deb: libarchive-dev
+    deb-cross-arch: foreign
     pkg: libarchive
     rpm: libarchive-devel
 
   libattr:
     deb: libattr1-dev
+    deb-cross-arch: foreign
     rpm: libattr-devel
 
   libaudit:
     deb: libaudit-dev
+    deb-cross-arch: foreign
     rpm: audit-libs-devel
 
   libblkid:
     deb: libblkid-dev
+    deb-cross-arch: foreign
     rpm: libblkid-devel
 
   libcap-ng:
     deb: libcap-ng-dev
+    deb-cross-arch: foreign
     rpm: libcap-ng-devel
 
   libcmpiutil:
@@ -276,67 +300,81 @@ mappings:
 
   libconfig:
     deb: libconfig-dev
+    deb-cross-arch: foreign
     pkg: libconfig
     rpm: libconfig-devel
 
   libcurl:
     deb: libcurl4-gnutls-dev
+    deb-cross-arch: foreign
     pkg: curl
     rpm: libcurl-devel
 
   libdbus:
     deb: libdbus-1-dev
+    deb-cross-arch: foreign
     pkg: dbus
     rpm: dbus-devel
 
   libgovirt:
     rpm: libgovirt-devel
     Debian: libgovirt-dev
+    Debian-cross-arch: foreign
     Debian8:
 
   libiscsi:
     deb: libiscsi-dev
+    deb-cross-arch: foreign
     rpm: libiscsi-devel
 
   libnl3:
     deb: libnl-3-dev
+    deb-cross-arch: foreign
     rpm: libnl3-devel
 
   libnlroute3:
     deb: libnl-route-3-dev
+    deb-cross-arch: foreign
     rpm: libnl3-devel
 
   libnuma:
     deb: libnuma-dev
+    deb-cross-arch: foreign
     deb-armv6l:
     deb-armv7l:
     rpm: numactl-devel
 
   libparted:
     deb: libparted-dev
+    deb-cross-arch: foreign
     rpm: parted-devel
 
   libpcap:
     deb: libpcap0.8-dev
+    deb-cross-arch: foreign
     pkg: libpcap
     rpm: libpcap-devel
 
   libpciaccess:
     deb: libpciaccess-dev
+    deb-cross-arch: foreign
     pkg: libpciaccess
     rpm: libpciaccess-devel
 
   librbd:
     deb: librbd-dev
+    deb-cross-arch: foreign
     Fedora: librbd-devel
     CentOS7: librbd1-devel
 
   libselinux:
     deb: libselinux1-dev
+    deb-cross-arch: foreign
     rpm: libselinux-devel
 
   libsoup:
     deb: libsoup2.4-dev
+    deb-cross-arch: foreign
     pkg: libsoup
     rpm: libsoup-devel
 
@@ -344,15 +382,18 @@ mappings:
     pkg: libssh
     rpm: libssh-devel
     Debian: libssh-gcrypt-dev
+    Debian-cross-arch: foreign
     Ubuntu: libssh-dev
 
   libssh2:
     deb: libssh2-1-dev
+    deb-cross-arch: foreign
     pkg: libssh2
     rpm: libssh2-devel
 
   libtirpc:
     deb: libtirpc-dev
+    deb-cross-arch: foreign
     rpm: libtirpc-devel
 
   libtool:
@@ -364,20 +405,24 @@ mappings:
 
   libudev:
     deb: libudev-dev
+    deb-cross-arch: foreign
     rpm: libudev-devel
 
   libuuid:
     deb: uuid-dev
+    deb-cross-arch: foreign
     pkg: e2fsprogs-libuuid
     rpm: libuuid-devel
 
   libxml2:
     deb: libxml2-dev
+    deb-cross-arch: foreign
     pkg: libxml2
     rpm: libxml2-devel
 
   libxslt:
     deb: libxslt1-dev
+    deb-cross-arch: foreign
     pkg: libxslt
     rpm: libxslt-devel
 
@@ -554,6 +599,7 @@ mappings:
 
   netcf:
     deb: libnetcf-dev
+    deb-cross-arch: skip
     rpm: netcf-devel
 
   numad:
@@ -714,6 +760,7 @@ mappings:
 
   python2-devel:
     deb: python-dev
+    deb-cross-arch: foreign
     pkg: python2
     rpm: python2-devel
 
@@ -738,6 +785,7 @@ mappings:
 
   python3-devel:
     deb: python3-dev
+    deb-cross-arch: foreign
     pkg: python3
     Fedora: python3-devel
 
@@ -783,6 +831,7 @@ mappings:
 
   readline:
     deb: libreadline-dev
+    deb-cross-arch: foreign
     pkg: readline
     rpm: readline-devel
 
@@ -797,6 +846,7 @@ mappings:
 
   sanlock:
     deb: libsanlock-dev
+    deb-cross-arch: foreign
     rpm: sanlock-devel
 
   screen:
@@ -818,6 +868,7 @@ mappings:
 
   spice-gtk3:
     deb: libspice-client-gtk-3.0-dev
+    deb-cross-arch: foreign
     pkg: spice-gtk
     rpm: spice-gtk3-devel
 
@@ -849,10 +900,12 @@ mappings:
 
   wireshark:
     deb: wireshark-dev
+    deb-cross-arch: skip
     Fedora: wireshark-devel
     Debian8:
 
   xen:
+    deb-cross-arch: foreign
     deb-x86_64: libxen-dev
     deb-armv7l: libxen-dev
     deb-aarch64: libxen-dev
@@ -860,6 +913,7 @@ mappings:
 
   xfsprogs:
     deb: xfslibs-dev
+    deb-cross-arch: foreign
     rpm: xfsprogs-devel
 
   xmllint:
@@ -872,14 +926,17 @@ mappings:
 
   xz:
     deb: liblzma-dev
+    deb-cross-arch: foreign
     rpm: xz-devel
 
   xz-static:
     deb: liblzma-dev
+    deb-cross-arch: foreign
     Fedora: xz-static
 
   yajl:
     deb: libyajl-dev
+    deb-cross-arch: foreign
     pkg: yajl
     rpm: yajl-devel
 
@@ -890,8 +947,10 @@ mappings:
 
   zlib:
     deb: zlib1g-dev
+    deb-cross-arch: foreign
     rpm: zlib-devel
 
   zlib-static:
     deb: zlib1g-dev
+    deb-cross-arch: foreign
     rpm: zlib-static
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v5 5/5] lcitool: support generating cross compiler dockerfiles
Posted by Andrea Bolognani 6 years, 11 months ago
On Tue, 2019-02-26 at 11:00 +0000, Daniel P. Berrangé wrote:
> Debian's filesystem layout has a nice advantage over Fedora which is
> that it can install non-native RPMs

s/RPMs/packages/ ;)

[...]
>  guests/host_vars/libvirt-debian-9/main.yml   | 44 +++++++++++++
>  guests/host_vars/libvirt-debian-sid/main.yml | 45 ++++++++++++++
>  guests/lcitool                               | 65 +++++++++++++++++++-
>  guests/vars/mappings.yml                     | 59 ++++++++++++++++++
>  4 files changed, 211 insertions(+), 2 deletions(-)

It would be great if the changes to code and inventory were in
separate commits. It should be trivial to split: just add the new
information to the inventory first, and code relying on them being
present last.

[...]
> +++ b/guests/host_vars/libvirt-debian-9/main.yml
> @@ -21,3 +21,47 @@ os_name: 'Debian'
>  os_version: '9'
>  
>  ansible_python_interpreter: /usr/bin/python3
> +
> +arches:
> +  aarch64:
> +    package_arch: arm64
> +    abi: aarch64-linux-gnu
> +    cross_gcc: gcc-aarch64-linux-gnu

We previously agreed to store this information in cross-build.yml
rather than main.yml, and that's what it looked like in v3... Can
you please move it back there?

I also questioned a few respins back whether we need to have the
Debian 9 configuration at all. For MinGW builds we use Fedora Rawhide
as the base, so using Debian Sid for cross-builds would make sense,
especially since it supports more target architectures: we are only
going to build a single set of buildenv-cross-* container images
anyway...

[...]
> +++ b/guests/host_vars/libvirt-debian-sid/main.yml
> @@ -21,3 +21,48 @@ os_name: 'Debian'
>  os_version: 'Sid'
>  
>  ansible_python_interpreter: /usr/bin/python3
> +
> +arches:
> +  aarch64:
> +    package_arch: arm64
> +    abi: aarch64-linux-gnu
> +    cross_gcc: gcc-aarch64-linux-gnu

Pretty much all this information is static: the mapping between
libvirt/kernel architecture name and Debian architecture name is
well defined and can't really change, and the same should apply to
the corresponding ABI names, so I would personally just implement
the mapping in Python, right next to the function we already have
introduced in the previous commit to fetch the native architecture
of the machine lcitool is running on.

As for the cross compiler, we can obtain the package name by
prepending "gcc-" to the ABI name, so once again storing it in the
inventory is unnecessary.

[...]
> +  i686:
> +    package_arch: i386
> +    abi: i686-linux-gnu
> +    cross_gcc: gcc-i686-linux-gnu

The value of 'abi' doesn't look right: based on eg.

  https://packages.debian.org/sid/i386/libglib2.0-dev/filelist

it should be 'i386-linux-gnu' instead.

The value of 'cross_gcc', though, is unfortunately correct, which
means that in this case we can't automatically figure out the name
of the cross-compiler package from the ABI like I described above.
Bummer :(

That said, storing this much redundant information because of a
single exception seems wasteful. Perhaps lcitool could behave the
following way:

  * figure out 'package_arch' and 'abi' based on 'cross_arch' as
    specified on the command line, using a static mapping function
    implemented in Python;

  * if the facts for the host contain arches[arch]["cross_gcc"],
    then use that value; otherwise, construct 'cross_gcc' by
    prepending "gcc-" to 'abi' - this will take care of the ugly
    exception above;

  * if we only have Debian Sid to take into account, then we can
    simply block attempts to "cross-compile" to the native
    architecture, but if such special-casing is not considered
    appropriate we can reuse the same logic mentioned above by
    having an empty value for arches["x86_64"]["cross_gcc"]. Note
    though that the check against the native architecture is
    probably what we want, because it's more accurate in general:
    gcc-x86-64-linux-gnu might not be available on x86_64, but it
    is on ppc64le and aarch64...

[...]
> +  mips64:
> +    package_arch: mips64
> +    abi: mips64-linux-gnu
> +    cross_gcc: gcc-mips64-linux-gnu

Debian doesn't have a 'mips64' architecture.

> +  mips64el:
> +    package_arch: mips64el
> +    abi: mips64el-linux-gnu

This should be

  abi: mips64el-linux-gnuabi64

> +  ppc64el:
> +    package_arch: ppc64el
> +    abi: ppc64el-linux-gnu

This should be

  abi: powerpc64le-linux-gnu

[...]
> +++ b/guests/lcitool
> @@ -367,6 +367,10 @@ class Application:
>  
>          add_hosts_arg(dockerfileparser)
>          add_projects_arg(dockerfileparser)
> +        dockerfileparser.add_argument(
> +            "-x", "--cross-arch",
> +            help="cross compiler architecture",
> +        )

You added the add_gitrev_arg() helper for --git-revision despite the
fact that it's only used by a single action, so it would make sense
to have add_crossarch_arg() here for consistency.

[...]
> @@ -537,10 +541,23 @@ class Application:
>          os_name = facts["os_name"]
>          os_version = facts["os_version"]
>          os_full = os_name + os_version
> +        cross_facts = None

You initialize 'cross_facts' here...

[...]
> +        if args.cross_arch is not None:
> +            if "arches" not in facts:
> +                raise Error("Non x86_64 arches not supported for this host")
> +            if args.cross_arch not in facts["arches"]:
> +                raise Error("Arch {} not supported for this host".format(
> +                    args.cross_arch))
> +            if "cross_gcc" not in facts["arches"][args.cross_arch]:
> +                raise Error("Arch {} cross compiler not supported for this host".format
> +                            (args.cross_arch))
> +
> +            cross_build_facts = facts["arches"][args.cross_arch]

... but then set 'cross_build_facts' and use the latter from here
on. And this is why dynamic languages are so much fun! O:-)

I'd stick with 'cross_facts' for the variable name, since we already
have 'pkgs'/'cross_pkgs' and 'keys'/'cross_keys'.

[...]
>          # We need to add the base project manually here: the standard
>          # machinery hides it because it's an implementation detail
>          for project in projects + ["base"]:
>              for package in self._projects.get_packages(project):
> +                policy = "native"

I'd call this variable 'cross_policy' for consistency with the other
variables used in the cross-build context. More comment on the
functionality it's connected to further down.

[...]
> @@ -80,6 +84,7 @@ mappings:
>  
>    avahi:
>      deb: libavahi-client-dev
> +    deb-cross-arch: foreign
>      pkg: avahi
>      rpm: avahi-devel

So, I'll come right out and say that I don't love the idea of storing
this information in the mappings file. That said, doing so allows us
to reuse a lot of existing infrastructure and I don't really have a
better alternative to offer at the moment, so let's just do it this
way :)

We should, however, apply the same changes as for the architecture
information and put the new part first; moreover, I would suggest
using something like 'cross-policy' instead of 'cross-arch', because
the latter is not very descriptive.

One last point is about the possible values: 'skip' and 'native' are
perfectly intuitive, but I feel like 'foreign' is not that great a
name, and it also already has a meaning in the context of Debian's
Multi-Arch implementation which might muddy the waters for people.
I think 'cross' would be a better value.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v5 5/5] lcitool: support generating cross compiler dockerfiles
Posted by Daniel P. Berrangé 6 years, 11 months ago
On Tue, Feb 26, 2019 at 07:04:16PM +0100, Andrea Bolognani wrote:
> On Tue, 2019-02-26 at 11:00 +0000, Daniel P. Berrangé wrote:
> > Debian's filesystem layout has a nice advantage over Fedora which is
> > that it can install non-native RPMs
> 
> s/RPMs/packages/ ;)
> 
> [...]
> >  guests/host_vars/libvirt-debian-9/main.yml   | 44 +++++++++++++
> >  guests/host_vars/libvirt-debian-sid/main.yml | 45 ++++++++++++++
> >  guests/lcitool                               | 65 +++++++++++++++++++-
> >  guests/vars/mappings.yml                     | 59 ++++++++++++++++++
> >  4 files changed, 211 insertions(+), 2 deletions(-)
> 
> It would be great if the changes to code and inventory were in
> separate commits. It should be trivial to split: just add the new
> information to the inventory first, and code relying on them being
> present last.
> 
> [...]
> > +++ b/guests/host_vars/libvirt-debian-9/main.yml
> > @@ -21,3 +21,47 @@ os_name: 'Debian'
> >  os_version: '9'
> >  
> >  ansible_python_interpreter: /usr/bin/python3
> > +
> > +arches:
> > +  aarch64:
> > +    package_arch: arm64
> > +    abi: aarch64-linux-gnu
> > +    cross_gcc: gcc-aarch64-linux-gnu
> 
> We previously agreed to store this information in cross-build.yml
> rather than main.yml, and that's what it looked like in v3... Can
> you please move it back there?

I moved it back here as the pacakge_arch / abi fields are  not
specific to cross builds. They're general information about the
distro / toolchain...

but this all goes away with a static mapping in the code instead
so it doesn't matter.

> I also questioned a few respins back whether we need to have the
> Debian 9 configuration at all. For MinGW builds we use Fedora Rawhide
> as the base, so using Debian Sid for cross-builds would make sense,
> especially since it supports more target architectures: we are only
> going to build a single set of buildenv-cross-* container images
> anyway...

What we do with MinGW right now is not a good example to follow.
There are enough differences betweeen software versions that I
see failures building on stable Fedora that don't appear on
rawhide & vica-versa. This is particularly causing me pain for
virt-viewer release builds as the msi build process is frequently
failing due to changed DLL versions. So when I add MSI builds to
the CI, I'll want MinGW packages across mulitple distro versions.

The other problem is that both rawhide & sid are rolling unstable
distros which break. Right now I can't even test the cross arch
builds on Sid because some recent update has broken the ability
to install the cross compiler toolchain at all. So only supporting
the unstable Sid is not a good idea. 

> > +  i686:
> > +    package_arch: i386
> > +    abi: i686-linux-gnu
> > +    cross_gcc: gcc-i686-linux-gnu
> 
> The value of 'abi' doesn't look right: based on eg.
> 
>   https://packages.debian.org/sid/i386/libglib2.0-dev/filelist
> 
> it should be 'i386-linux-gnu' instead.

The 'abi' is what is prefixed on the toolchain binaries,
and so for this purpose i686-linux-gnu is actually correct:

  https://packages.debian.org/sid/amd64/gcc-i686-linux-gnu/filelist

> The value of 'cross_gcc', though, is unfortunately correct, which
> means that in this case we can't automatically figure out the name
> of the cross-compiler package from the ABI like I described above.
> Bummer :(

Actually we can.

> [...]
> > +++ b/guests/lcitool
> > @@ -367,6 +367,10 @@ class Application:
> >  
> >          add_hosts_arg(dockerfileparser)
> >          add_projects_arg(dockerfileparser)
> > +        dockerfileparser.add_argument(
> > +            "-x", "--cross-arch",
> > +            help="cross compiler architecture",
> > +        )
> 
> You added the add_gitrev_arg() helper for --git-revision despite the
> fact that it's only used by a single action, so it would make sense
> to have add_crossarch_arg() here for consistency.
> 
> [...]
> > @@ -537,10 +541,23 @@ class Application:
> >          os_name = facts["os_name"]
> >          os_version = facts["os_version"]
> >          os_full = os_name + os_version
> > +        cross_facts = None
> 
> You initialize 'cross_facts' here...
> 
> [...]
> > +        if args.cross_arch is not None:
> > +            if "arches" not in facts:
> > +                raise Error("Non x86_64 arches not supported for this host")
> > +            if args.cross_arch not in facts["arches"]:
> > +                raise Error("Arch {} not supported for this host".format(
> > +                    args.cross_arch))
> > +            if "cross_gcc" not in facts["arches"][args.cross_arch]:
> > +                raise Error("Arch {} cross compiler not supported for this host".format
> > +                            (args.cross_arch))
> > +
> > +            cross_build_facts = facts["arches"][args.cross_arch]
> 
> ... but then set 'cross_build_facts' and use the latter from here
> on. And this is why dynamic languages are so much fun! O:-)
> 
> I'd stick with 'cross_facts' for the variable name, since we already
> have 'pkgs'/'cross_pkgs' and 'keys'/'cross_keys'.

This dict goes away if we use a static mapping table.


> [...]
> > @@ -80,6 +84,7 @@ mappings:
> >  
> >    avahi:
> >      deb: libavahi-client-dev
> > +    deb-cross-arch: foreign
> >      pkg: avahi
> >      rpm: avahi-devel
> 
> So, I'll come right out and say that I don't love the idea of storing
> this information in the mappings file. That said, doing so allows us
> to reuse a lot of existing infrastructure and I don't really have a
> better alternative to offer at the moment, so let's just do it this
> way :)

The compelling reason for using mappings.xml is that it provides a
single source of information for packages. This way when adding new
packages to the list, it is more likely we'll remember to add the
right cross arch info at the same time.

> One last point is about the possible values: 'skip' and 'native' are
> perfectly intuitive, but I feel like 'foreign' is not that great a
> name, and it also already has a meaning in the context of Debian's
> Multi-Arch implementation which might muddy the waters for people.
> I think 'cross' would be a better value.

"foreign" is the logical inverse of "native", so I think it is
actually the right word to use here. "cross" is misleading as
we're not installing a cross-built package, we're installing
the native built package from the foreign architecture.


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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v5 5/5] lcitool: support generating cross compiler dockerfiles
Posted by Andrea Bolognani 6 years, 11 months ago
On Thu, 2019-02-28 at 15:52 +0000, Daniel P. Berrangé wrote:
> On Tue, Feb 26, 2019 at 07:04:16PM +0100, Andrea Bolognani wrote:
[...]
> > > +++ b/guests/host_vars/libvirt-debian-9/main.yml
> > > @@ -21,3 +21,47 @@ os_name: 'Debian'
> > >  os_version: '9'
> > >  
> > >  ansible_python_interpreter: /usr/bin/python3
> > > +
> > > +arches:
> > > +  aarch64:
> > > +    package_arch: arm64
> > > +    abi: aarch64-linux-gnu
> > > +    cross_gcc: gcc-aarch64-linux-gnu
> > 
> > We previously agreed to store this information in cross-build.yml
> > rather than main.yml, and that's what it looked like in v3... Can
> > you please move it back there?
> 
> I moved it back here as the pacakge_arch / abi fields are  not
> specific to cross builds. They're general information about the
> distro / toolchain...
> 
> but this all goes away with a static mapping in the code instead
> so it doesn't matter.

Okay, fair enough. For whatever local override we might need though,
eg. to disable cross-building for i386 on Debian 9, we still want to
use a separate facts file.

> > I also questioned a few respins back whether we need to have the
> > Debian 9 configuration at all. For MinGW builds we use Fedora Rawhide
> > as the base, so using Debian Sid for cross-builds would make sense,
> > especially since it supports more target architectures: we are only
> > going to build a single set of buildenv-cross-* container images
> > anyway...
> 
> What we do with MinGW right now is not a good example to follow.
> There are enough differences betweeen software versions that I
> see failures building on stable Fedora that don't appear on
> rawhide & vica-versa. This is particularly causing me pain for
> virt-viewer release builds as the msi build process is frequently
> failing due to changed DLL versions. So when I add MSI builds to
> the CI, I'll want MinGW packages across mulitple distro versions.
> 
> The other problem is that both rawhide & sid are rolling unstable
> distros which break. Right now I can't even test the cross arch
> builds on Sid because some recent update has broken the ability
> to install the cross compiler toolchain at all. So only supporting
> the unstable Sid is not a good idea.

Alright, makes sense.

> > > +  i686:
> > > +    package_arch: i386
> > > +    abi: i686-linux-gnu
> > > +    cross_gcc: gcc-i686-linux-gnu
> > 
> > The value of 'abi' doesn't look right: based on eg.
> > 
> >   https://packages.debian.org/sid/i386/libglib2.0-dev/filelist
> > 
> > it should be 'i386-linux-gnu' instead.
> 
> The 'abi' is what is prefixed on the toolchain binaries,
> and so for this purpose i686-linux-gnu is actually correct:
> 
>   https://packages.debian.org/sid/amd64/gcc-i686-linux-gnu/filelist

Okay, so I guess you need to have cross_target=i686-linux-gnu for

  ENV CONFIGURE_OPTS "--host={cross_target} \
                      --target={cross_target}"

to work; however, that will cause issues for

  ENV PKG_CONFIG_LIBDIR "/usr/lib/{cross_target}/pkgconfig"

because as shown above paths look like

  /usr/lib/i386-linux-gnu/pkgconfig/glib-2.0.pc

and so on. What an inconsistent mess! Does it mean we need to
carry around both? :(


... Actually, it doesn't look like it: I just tried cross-building
for i686 (on sid/x86_64) by simply passing

  --host=i686-linux-gnu --target=i686-linux-gnu

and the build system was able to locate all necessary dependencies
despite not having set $PKG_CONFIG_LIBDIR in the environment!

This seems to be thanks to i686-linux-gnu-pkg-config, which is a
symlink to /usr/share/pkg-config-crosswrapper, being picked up and
invoked instead of the native one.


tl;dr It looks like we can generate 'cross_gcc' programmatically
      and we can even get rid of one ENV statement! Neat :)

[...]
> > One last point is about the possible values: 'skip' and 'native' are
> > perfectly intuitive, but I feel like 'foreign' is not that great a
> > name, and it also already has a meaning in the context of Debian's
> > Multi-Arch implementation which might muddy the waters for people.
> > I think 'cross' would be a better value.
> 
> "foreign" is the logical inverse of "native", so I think it is
> actually the right word to use here. "cross" is misleading as
> we're not installing a cross-built package, we're installing
> the native built package from the foreign architecture.

Sure, let's keep it that way then.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v5 5/5] lcitool: support generating cross compiler dockerfiles
Posted by Daniel P. Berrangé 6 years, 11 months ago
On Thu, Feb 28, 2019 at 06:46:41PM +0100, Andrea Bolognani wrote:
> On Thu, 2019-02-28 at 15:52 +0000, Daniel P. Berrangé wrote:
> > On Tue, Feb 26, 2019 at 07:04:16PM +0100, Andrea Bolognani wrote:
> [...]

> > > > +  i686:
> > > > +    package_arch: i386
> > > > +    abi: i686-linux-gnu
> > > > +    cross_gcc: gcc-i686-linux-gnu
> > > 
> > > The value of 'abi' doesn't look right: based on eg.
> > > 
> > >   https://packages.debian.org/sid/i386/libglib2.0-dev/filelist
> > > 
> > > it should be 'i386-linux-gnu' instead.
> > 
> > The 'abi' is what is prefixed on the toolchain binaries,
> > and so for this purpose i686-linux-gnu is actually correct:
> > 
> >   https://packages.debian.org/sid/amd64/gcc-i686-linux-gnu/filelist
> 
> Okay, so I guess you need to have cross_target=i686-linux-gnu for
> 
>   ENV CONFIGURE_OPTS "--host={cross_target} \
>                       --target={cross_target}"
> 
> to work; however, that will cause issues for
> 
>   ENV PKG_CONFIG_LIBDIR "/usr/lib/{cross_target}/pkgconfig"
> 
> because as shown above paths look like
> 
>   /usr/lib/i386-linux-gnu/pkgconfig/glib-2.0.pc
> 
> and so on. What an inconsistent mess! Does it mean we need to
> carry around both? :(
> 
> 
> ... Actually, it doesn't look like it: I just tried cross-building
> for i686 (on sid/x86_64) by simply passing
> 
>   --host=i686-linux-gnu --target=i686-linux-gnu
> 
> and the build system was able to locate all necessary dependencies
> despite not having set $PKG_CONFIG_LIBDIR in the environment!
> 
> This seems to be thanks to i686-linux-gnu-pkg-config, which is a
> symlink to /usr/share/pkg-config-crosswrapper, being picked up and
> invoked instead of the native one.

Interesting. I'll have to check this again. In my previous versions
of this series, pkgconfig was mistakenly finding the native .pc
files, and PKG_CONFIG_LIBDIR was needed to fix it. So that can't have
been using the i686-linux-gnu-pkg-config binary before. I'll try and
see what changed..

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v5 5/5] lcitool: support generating cross compiler dockerfiles
Posted by Andrea Bolognani 6 years, 11 months ago
On Thu, 2019-02-28 at 18:03 +0000, Daniel P. Berrangé wrote:
> On Thu, Feb 28, 2019 at 06:46:41PM +0100, Andrea Bolognani wrote:
> > Okay, so I guess you need to have cross_target=i686-linux-gnu for
> > 
> >   ENV CONFIGURE_OPTS "--host={cross_target} \
> >                       --target={cross_target}"
> > 
> > to work; however, that will cause issues for
> > 
> >   ENV PKG_CONFIG_LIBDIR "/usr/lib/{cross_target}/pkgconfig"
> > 
> > because as shown above paths look like
> > 
> >   /usr/lib/i386-linux-gnu/pkgconfig/glib-2.0.pc
> > 
> > and so on. What an inconsistent mess! Does it mean we need to
> > carry around both? :(
> > 
> > 
> > ... Actually, it doesn't look like it: I just tried cross-building
> > for i686 (on sid/x86_64) by simply passing
> > 
> >   --host=i686-linux-gnu --target=i686-linux-gnu
> > 
> > and the build system was able to locate all necessary dependencies
> > despite not having set $PKG_CONFIG_LIBDIR in the environment!
> > 
> > This seems to be thanks to i686-linux-gnu-pkg-config, which is a
> > symlink to /usr/share/pkg-config-crosswrapper, being picked up and
> > invoked instead of the native one.
> 
> Interesting. I'll have to check this again. In my previous versions
> of this series, pkgconfig was mistakenly finding the native .pc
> files, and PKG_CONFIG_LIBDIR was needed to fix it. So that can't have
> been using the i686-linux-gnu-pkg-config binary before. I'll try and
> see what changed..

Unfortunately that only seems to apply to Debian Sid: the version
of pkg-config in Debian 9 is quite a bit older and apparently not
as smart.

So we need to have $PKG_CONFIG_LIBDIR in the environment after all,
and also deal with the mess described above somehow :(

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [jenkins-ci PATCH v5 5/5] lcitool: support generating cross compiler dockerfiles
Posted by Daniel P. Berrangé 6 years, 11 months ago
On Fri, Mar 01, 2019 at 05:19:29PM +0100, Andrea Bolognani wrote:
> On Thu, 2019-02-28 at 18:03 +0000, Daniel P. Berrangé wrote:
> > On Thu, Feb 28, 2019 at 06:46:41PM +0100, Andrea Bolognani wrote:
> > > Okay, so I guess you need to have cross_target=i686-linux-gnu for
> > > 
> > >   ENV CONFIGURE_OPTS "--host={cross_target} \
> > >                       --target={cross_target}"
> > > 
> > > to work; however, that will cause issues for
> > > 
> > >   ENV PKG_CONFIG_LIBDIR "/usr/lib/{cross_target}/pkgconfig"
> > > 
> > > because as shown above paths look like
> > > 
> > >   /usr/lib/i386-linux-gnu/pkgconfig/glib-2.0.pc
> > > 
> > > and so on. What an inconsistent mess! Does it mean we need to
> > > carry around both? :(
> > > 
> > > 
> > > ... Actually, it doesn't look like it: I just tried cross-building
> > > for i686 (on sid/x86_64) by simply passing
> > > 
> > >   --host=i686-linux-gnu --target=i686-linux-gnu
> > > 
> > > and the build system was able to locate all necessary dependencies
> > > despite not having set $PKG_CONFIG_LIBDIR in the environment!
> > > 
> > > This seems to be thanks to i686-linux-gnu-pkg-config, which is a
> > > symlink to /usr/share/pkg-config-crosswrapper, being picked up and
> > > invoked instead of the native one.
> > 
> > Interesting. I'll have to check this again. In my previous versions
> > of this series, pkgconfig was mistakenly finding the native .pc
> > files, and PKG_CONFIG_LIBDIR was needed to fix it. So that can't have
> > been using the i686-linux-gnu-pkg-config binary before. I'll try and
> > see what changed..
> 
> Unfortunately that only seems to apply to Debian Sid: the version
> of pkg-config in Debian 9 is quite a bit older and apparently not
> as smart.

Ah ha, that explains it - I only tested that Deb9 originally.

> So we need to have $PKG_CONFIG_LIBDIR in the environment after all,
> and also deal with the mess described above somehow :(

Yeah, I'll think about how to deal with that mess

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 :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list