[PATCH] build: disable container-based cross compilers by default

[PATCH] build: disable container-based cross compilers by default

[Paolo Bonzini]

Container-based cross compilers have some issues which were overlooked
when they were only used for TCG tests, but are more visible since
firmware builds try to use them:

- Downloading and building containers as part of make adds a
  very long task to the build, unless you are on a fast network.
  Container images can be hundreds of MBs.

- Verbose progress information from the container builds
  is printed on stderr and messes up other output from
  make/ninja

- There seem to be some rough edges around failure too.

So, make container builds opt-in.

Reported-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 .gitlab-ci.d/buildtest.yml                       | 16 ++++++++--------
 .gitlab-ci.d/crossbuilds.yml                     |  2 +-
 .../custom-runners/ubuntu-20.04-s390x.yml        |  2 +-
 .../custom-runners/ubuntu-22.04-aarch64.yml      |  2 +-
 configure                                        |  4 ++--
 5 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 6c05c46397..41742ae962 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -263,7 +263,7 @@ build-user:
     job: amd64-debian-user-cross-container
   variables:
     IMAGE: debian-all-test-cross
-    CONFIGURE_ARGS: --disable-tools --disable-system
+    CONFIGURE_ARGS: --disable-tools --disable-system --enable-containers
     MAKE_CHECK_ARGS: check-tcg
 
 build-user-static:
@@ -272,7 +272,7 @@ build-user-static:
     job: amd64-debian-user-cross-container
   variables:
     IMAGE: debian-all-test-cross
-    CONFIGURE_ARGS: --disable-tools --disable-system --static
+    CONFIGURE_ARGS: --disable-tools --disable-system --static --enable-containers
     MAKE_CHECK_ARGS: check-tcg
 
 # Because the hexagon cross-compiler takes so long to build we don't rely
@@ -286,7 +286,7 @@ build-user-hexagon:
   variables:
     IMAGE: debian-hexagon-cross
     TARGETS: hexagon-linux-user
-    CONFIGURE_ARGS: --disable-tools --disable-docs --enable-debug-tcg
+    CONFIGURE_ARGS: --disable-tools --disable-docs --enable-debug-tcg --enable-containers
     MAKE_CHECK_ARGS: check-tcg
 
 # Only build the softmmu targets we have check-tcg tests for
@@ -296,7 +296,7 @@ build-some-softmmu:
     job: amd64-debian-user-cross-container
   variables:
     IMAGE: debian-all-test-cross
-    CONFIGURE_ARGS: --disable-tools --enable-debug
+    CONFIGURE_ARGS: --disable-tools --enable-debug --enable-containers
     TARGETS: xtensa-softmmu arm-softmmu aarch64-softmmu alpha-softmmu
     MAKE_CHECK_ARGS: check-tcg
 
@@ -307,7 +307,7 @@ build-tricore-softmmu:
     job: tricore-debian-cross-container
   variables:
     IMAGE: debian-tricore-cross
-    CONFIGURE_ARGS: --disable-tools --disable-fdt --enable-debug
+    CONFIGURE_ARGS: --disable-tools --disable-fdt --enable-debug --enable-containers
     TARGETS: tricore-softmmu
     MAKE_CHECK_ARGS: check-tcg
 
@@ -317,7 +317,7 @@ clang-system:
     job: amd64-fedora-container
   variables:
     IMAGE: fedora
-    CONFIGURE_ARGS: --cc=clang --cxx=clang++
+    CONFIGURE_ARGS: --enable-containers --cc=clang --cxx=clang++
       --extra-cflags=-fsanitize=undefined --extra-cflags=-fno-sanitize-recover=undefined
     TARGETS: alpha-softmmu arm-softmmu m68k-softmmu mips64-softmmu
       ppc-softmmu s390x-softmmu
@@ -329,7 +329,7 @@ clang-user:
     job: amd64-debian-user-cross-container
   variables:
     IMAGE: debian-all-test-cross
-    CONFIGURE_ARGS: --cc=clang --cxx=clang++ --disable-system
+    CONFIGURE_ARGS: --enable-containers --cc=clang --cxx=clang++ --disable-system
       --target-list-exclude=microblazeel-linux-user,aarch64_be-linux-user,i386-linux-user,m68k-linux-user,mipsn32el-linux-user,xtensaeb-linux-user
       --extra-cflags=-fsanitize=undefined --extra-cflags=-fno-sanitize-recover=undefined
     MAKE_CHECK_ARGS: check-unit check-tcg
@@ -523,7 +523,7 @@ build-tci:
     - TARGETS="aarch64 alpha arm hppa m68k microblaze ppc64 s390x x86_64"
     - mkdir build
     - cd build
-    - ../configure --enable-tcg-interpreter
+    - ../configure --enable-containers --enable-tcg-interpreter
         --target-list="$(for tg in $TARGETS; do echo -n ${tg}'-softmmu '; done)" || { cat config.log meson-logs/meson-log.txt && exit 1; }
     - make -j"$JOBS"
     - make tests/qtest/boot-serial-test tests/qtest/cdrom-test tests/qtest/pxe-test
diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
index c4cd96433d..b2519ff2e0 100644
--- a/.gitlab-ci.d/crossbuilds.yml
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -67,7 +67,7 @@ cross-i386-tci:
   variables:
     IMAGE: fedora-i386-cross
     ACCEL: tcg-interpreter
-    EXTRA_CONFIGURE_OPTS: --target-list=i386-softmmu,i386-linux-user,aarch64-softmmu,aarch64-linux-user,ppc-softmmu,ppc-linux-user
+    EXTRA_CONFIGURE_OPTS: --target-list=i386-softmmu,i386-linux-user,aarch64-softmmu,aarch64-linux-user,ppc-softmmu,ppc-linux-user --enable-containers
     MAKE_CHECK_ARGS: check check-tcg
 
 cross-mipsel-system:
diff --git a/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml b/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml
index 0c835939db..24bca3f995 100644
--- a/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml
+++ b/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml
@@ -16,7 +16,7 @@ ubuntu-20.04-s390x-all-linux-static:
  # --disable-glusterfs is needed because there's no static version of those libs in distro supplied packages
  - mkdir build
  - cd build
- - ../configure --enable-debug --static --disable-system --disable-glusterfs --disable-libssh
+ - ../configure --enable-debug --static --disable-system --disable-glusterfs --disable-libssh --enable-containers
    || { cat config.log meson-logs/meson-log.txt; exit 1; }
  - make --output-sync -j`nproc`
  - make --output-sync -j`nproc` check V=1
diff --git a/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch64.yml b/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch64.yml
index ce0b18af6f..db0c919fab 100644
--- a/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch64.yml
+++ b/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch64.yml
@@ -16,7 +16,7 @@ ubuntu-22.04-aarch64-all-linux-static:
  - cd build
  # Disable -static-pie due to build error with system libc:
  # https://bugs.launchpad.net/ubuntu/+source/glibc/+bug/1987438
- - ../configure --enable-debug --static --disable-system --disable-pie
+ - ../configure --enable-debug --static --disable-system --disable-pie --enable-containers
    || { cat config.log meson-logs/meson-log.txt; exit 1; }
  - make --output-sync -j`nproc --ignore=40`
  - make --output-sync -j`nproc --ignore=40` check V=1
diff --git a/configure b/configure
index baa69189f0..6fa158a0d4 100755
--- a/configure
+++ b/configure
@@ -227,7 +227,7 @@ cross_prefix=""
 host_cc="cc"
 stack_protector=""
 safe_stack=""
-use_containers="yes"
+use_containers="no"
 gdb_bin=$(command -v "gdb-multiarch" || command -v "gdb")
 
 if test -e "$source_path/.git"
@@ -1034,7 +1034,7 @@ Advanced options (experts only):
                            ucontext, sigaltstack, windows
   --enable-plugins
                            enable plugins via shared library loading
-  --disable-containers     don't use containers for cross-building
+  --enable-containers      use containers for cross-building
   --gdb=GDB-path           gdb to use for gdbstub tests [$gdb_bin]
 EOF
   meson_options_help
-- 
2.37.3

Re: [PATCH] build: disable container-based cross compilers by default

[Daniel P. Berrangé]

On Wed, Oct 12, 2022 at 11:08:55AM +0200, Paolo Bonzini wrote:
> Container-based cross compilers have some issues which were overlooked
> when they were only used for TCG tests, but are more visible since
> firmware builds try to use them:
> 
> - Downloading and building containers as part of make adds a
>   very long task to the build, unless you are on a fast network.
>   Container images can be hundreds of MBs.
> 
> - Verbose progress information from the container builds
>   is printed on stderr and messes up other output from
>   make/ninja
> 
> - There seem to be some rough edges around failure too.
> 
> So, make container builds opt-in.
> 
> Reported-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  .gitlab-ci.d/buildtest.yml                       | 16 ++++++++--------
>  .gitlab-ci.d/crossbuilds.yml                     |  2 +-
>  .../custom-runners/ubuntu-20.04-s390x.yml        |  2 +-
>  .../custom-runners/ubuntu-22.04-aarch64.yml      |  2 +-
>  configure                                        |  4 ++--
>  5 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> index 6c05c46397..41742ae962 100644
> --- a/.gitlab-ci.d/buildtest.yml
> +++ b/.gitlab-ci.d/buildtest.yml
> @@ -263,7 +263,7 @@ build-user:
>      job: amd64-debian-user-cross-container
>    variables:
>      IMAGE: debian-all-test-cross
> -    CONFIGURE_ARGS: --disable-tools --disable-system
> +    CONFIGURE_ARGS: --disable-tools --disable-system --enable-containers
>      MAKE_CHECK_ARGS: check-tcg

Are you sure these jobs wer using containers in the first place ?

A standard gitlab CI environment isn't able to use normal docker,
and the build jobs aren't configured with the docker-in-docker
service. So I'd be surprised if any were using the container
logic.

I guess we auto-detect if it works, so silently skip them, but
its probably misleading to add --enable-containers to an env
we don't expect to use them.


> diff --git a/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml b/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml
> index 0c835939db..24bca3f995 100644
> --- a/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml
> +++ b/.gitlab-ci.d/custom-runners/ubuntu-20.04-s390x.yml
> @@ -16,7 +16,7 @@ ubuntu-20.04-s390x-all-linux-static:
>   # --disable-glusterfs is needed because there's no static version of those libs in distro supplied packages
>   - mkdir build
>   - cd build
> - - ../configure --enable-debug --static --disable-system --disable-glusterfs --disable-libssh
> + - ../configure --enable-debug --static --disable-system --disable-glusterfs --disable-libssh --enable-containers
>     || { cat config.log meson-logs/meson-log.txt; exit 1; }
>   - make --output-sync -j`nproc`
>   - make --output-sync -j`nproc` check V=1
> diff --git a/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch64.yml b/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch64.yml
> index ce0b18af6f..db0c919fab 100644
> --- a/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch64.yml
> +++ b/.gitlab-ci.d/custom-runners/ubuntu-22.04-aarch64.yml
> @@ -16,7 +16,7 @@ ubuntu-22.04-aarch64-all-linux-static:
>   - cd build
>   # Disable -static-pie due to build error with system libc:
>   # https://bugs.launchpad.net/ubuntu/+source/glibc/+bug/1987438
> - - ../configure --enable-debug --static --disable-system --disable-pie
> + - ../configure --enable-debug --static --disable-system --disable-pie --enable-containers
>     || { cat config.log meson-logs/meson-log.txt; exit 1; }
>   - make --output-sync -j`nproc --ignore=40`
>   - make --output-sync -j`nproc --ignore=40` check V=1


These changes are likely ok, as the custom runners are bare metal
so can use containers normally, provided docker/podman is installed.

> diff --git a/configure b/configure
> index baa69189f0..6fa158a0d4 100755
> --- a/configure
> +++ b/configure
> @@ -227,7 +227,7 @@ cross_prefix=""
>  host_cc="cc"
>  stack_protector=""
>  safe_stack=""
> -use_containers="yes"
> +use_containers="no"
>  gdb_bin=$(command -v "gdb-multiarch" || command -v "gdb")
>  
>  if test -e "$source_path/.git"
> @@ -1034,7 +1034,7 @@ Advanced options (experts only):
>                             ucontext, sigaltstack, windows
>    --enable-plugins
>                             enable plugins via shared library loading
> -  --disable-containers     don't use containers for cross-building
> +  --enable-containers      use containers for cross-building
>    --gdb=GDB-path           gdb to use for gdbstub tests [$gdb_bin]
>  EOF
>    meson_options_help
> -- 
> 2.37.3
> 

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

Re: [PATCH] build: disable container-based cross compilers by default

[Alex Bennée]

Paolo Bonzini <pbonzini@redhat.com> writes:

> Container-based cross compilers have some issues which were overlooked
> when they were only used for TCG tests, but are more visible since
> firmware builds try to use them:

We seem to have dropped our gating somewhere. Previously if a user did
not have docker or podman on their system none of the container stuff
would run.

-- 
Alex Bennée

Re: [PATCH] build: disable container-based cross compilers by default

[Paolo Bonzini]

On 10/12/22 14:17, Alex Bennée wrote:
>> Container-based cross compilers have some issues which were overlooked
>> when they were only used for TCG tests, but are more visible since
>> firmware builds try to use them:
> We seem to have dropped our gating somewhere. Previously if a user did
> not have docker or podman on their system none of the container stuff
> would run.

It's still there:

container="no"
if test $use_containers = "yes"; then
     case $($python "$source_path"/tests/docker/docker.py probe) in
         *docker) container=docker ;;
         podman) container=podman ;;
         no) container=no ;;
     esac
     if test "$container" != "no"; then
         docker_py="$python $source_path/tests/docker/docker.py --engine $container"
     fi
fi

I think what's happening is that podman is there but there's no support
for rootless containers, so "podman run" fails.

Paolo

Re: [PATCH] build: disable container-based cross compilers by default

[Alex Bennée]

Paolo Bonzini <pbonzini@redhat.com> writes:

> On 10/12/22 14:17, Alex Bennée wrote:
>>> Container-based cross compilers have some issues which were overlooked
>>> when they were only used for TCG tests, but are more visible since
>>> firmware builds try to use them:
>> We seem to have dropped our gating somewhere. Previously if a user did
>> not have docker or podman on their system none of the container stuff
>> would run.
>
> It's still there:
>
> container="no"
> if test $use_containers = "yes"; then
>     case $($python "$source_path"/tests/docker/docker.py probe) in
>         *docker) container=docker ;;
>         podman) container=podman ;;
>         no) container=no ;;
>     esac
>     if test "$container" != "no"; then
>         docker_py="$python $source_path/tests/docker/docker.py --engine $container"
>     fi
> fi
>
> I think what's happening is that podman is there but there's no support
> for rootless containers, so "podman run" fails.

Ahh so we could improve our probe code then? I'm afraid I don't have
much personal testing coverage for podman stuff - I thought rootless
support was the main reason Fedora had transitioned to it.

>
> Paolo


-- 
Alex Bennée