diff mbox series

[v2,5/5] gitlab-ci.d: Build with --enable-fdt=system by default

Message ID 20230207201447.566661-6-thuth@redhat.com
State New
Headers show
Series Shorten the runtime of some gitlab-CI shared runner jobs | expand

Commit Message

Thomas Huth Feb. 7, 2023, 8:14 p.m. UTC 
By using --enable-fdt=system we can make sure that the configure
script does not try to check out the "dtc" submodule. This should
help to safe some precious CI minutes in the long run.

While we're at it, also drop some now-redundant --enable-slirp
and --enable-capstone statements. These used to have the "=system"
suffix in the past, too, which has been dropped when the their
corresponding submodules had been removed. Since these features
are auto-enabled anyway now (since the containers have the right
libraries installed), we do not need the explicit --enable-...
statements anymore.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .gitlab-ci.d/buildtest-template.yml  | 2 +-
 .gitlab-ci.d/buildtest.yml           | 9 +++------
 .gitlab-ci.d/crossbuild-template.yml | 5 +++--
 .gitlab-ci.d/crossbuilds.yml         | 2 ++
 .gitlab-ci.d/windows.yml             | 7 +++++--
 5 files changed, 14 insertions(+), 11 deletions(-)

Comments

Daniel P. Berrangé Feb. 23, 2023, 2:02 p.m. UTC | #1 
On Tue, Feb 07, 2023 at 09:14:47PM +0100, Thomas Huth wrote:
> By using --enable-fdt=system we can make sure that the configure
> script does not try to check out the "dtc" submodule. This should
> help to safe some precious CI minutes in the long run.

If our containers have the system dtc installed, I'm pretty
surprised that configure is choosing to use dtc submodule.
I thought we won't touch the submodule at all if system dtc
was sufficiently new.

IOW, do we have a logic bug in configure making it incorrectly
use dtc submodules ?

> While we're at it, also drop some now-redundant --enable-slirp
> and --enable-capstone statements. These used to have the "=system"
> suffix in the past, too, which has been dropped when the their
> corresponding submodules had been removed. Since these features
> are auto-enabled anyway now (since the containers have the right
> libraries installed), we do not need the explicit --enable-...
> statements anymore.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  .gitlab-ci.d/buildtest-template.yml  | 2 +-
>  .gitlab-ci.d/buildtest.yml           | 9 +++------
>  .gitlab-ci.d/crossbuild-template.yml | 5 +++--
>  .gitlab-ci.d/crossbuilds.yml         | 2 ++
>  .gitlab-ci.d/windows.yml             | 7 +++++--
>  5 files changed, 14 insertions(+), 11 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


since it at least does what it claims, even if it the configure
logic is questionable.


With regards,
Daniel
Thomas Huth Feb. 23, 2023, 3:24 p.m. UTC | #2 
On 23/02/2023 15.02, Daniel P. Berrangé wrote:
> On Tue, Feb 07, 2023 at 09:14:47PM +0100, Thomas Huth wrote:
>> By using --enable-fdt=system we can make sure that the configure
>> script does not try to check out the "dtc" submodule. This should
>> help to safe some precious CI minutes in the long run.
> 
> If our containers have the system dtc installed, I'm pretty
> surprised that configure is choosing to use dtc submodule.
> I thought we won't touch the submodule at all if system dtc
> was sufficiently new.
> 
> IOW, do we have a logic bug in configure making it incorrectly
> use dtc submodules ?

Yes, it sounds weird at the first glance, but it's really this way (look for 
the "Simpler to always update submodule, even if not needed" comment in the 
configure script): The problem is that the initial submodule handling is 
done in configure already, so you have to know the needed submodules there 
already. But the check for usability of libfdt is only done in meson.build, 
so you already need to have the submodule available there in case the 
system's libfdt is not usable.

It could maybe cleaned up somehow, but OTOH, I'm still hoping that we can 
rid of the dtc submodule in the near future, so it's maybe not worth the 
effort to spend too much time with this right now.

  Thomas
diff mbox series

Patch

diff --git a/.gitlab-ci.d/buildtest-template.yml b/.gitlab-ci.d/buildtest-template.yml
index 4a922d9c33..cb96b55c3f 100644
--- a/.gitlab-ci.d/buildtest-template.yml
+++ b/.gitlab-ci.d/buildtest-template.yml
@@ -11,7 +11,7 @@ 
       fi
     - mkdir build
     - cd build
-    - ../configure --enable-werror --disable-docs
+    - ../configure --enable-werror --disable-docs --enable-fdt=system
           ${LD_JOBS:+--meson=git} ${TARGETS:+--target-list="$TARGETS"}
           $CONFIGURE_ARGS ||
       { cat config.log meson-logs/meson-log.txt && exit 1; }
diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 8697c61072..d903c42798 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -41,7 +41,7 @@  build-system-ubuntu:
     job: amd64-ubuntu2004-container
   variables:
     IMAGE: ubuntu2004
-    CONFIGURE_ARGS: --enable-docs --enable-fdt=system --enable-capstone
+    CONFIGURE_ARGS: --enable-docs
     TARGETS: alpha-softmmu cris-softmmu hppa-softmmu
       microblazeel-softmmu mips64el-softmmu
     MAKE_CHECK_ARGS: check-build
@@ -120,7 +120,6 @@  build-system-fedora:
   variables:
     IMAGE: fedora
     CONFIGURE_ARGS: --disable-gcrypt --enable-nettle --enable-docs
-             --enable-fdt=system --enable-slirp --enable-capstone
     TARGETS: tricore-softmmu microblaze-softmmu mips-softmmu
       xtensa-softmmu m68k-softmmu riscv32-softmmu ppc-softmmu sparc64-softmmu
     MAKE_CHECK_ARGS: check-build
@@ -166,9 +165,8 @@  build-system-centos:
     job: amd64-centos8-container
   variables:
     IMAGE: centos8
-    CONFIGURE_ARGS: --disable-nettle --enable-gcrypt --enable-fdt=system
+    CONFIGURE_ARGS: --disable-nettle --enable-gcrypt --enable-vfio-user-server
       --enable-modules --enable-trace-backends=dtrace --enable-docs
-      --enable-vfio-user-server
     TARGETS: ppc64-softmmu or1k-softmmu s390x-softmmu
       x86_64-softmmu rx-softmmu sh4-softmmu nios2-softmmu
     MAKE_CHECK_ARGS: check-build
@@ -201,7 +199,6 @@  build-system-opensuse:
     job: amd64-opensuse-leap-container
   variables:
     IMAGE: opensuse-leap
-    CONFIGURE_ARGS: --enable-fdt=system
     TARGETS: s390x-softmmu x86_64-softmmu aarch64-softmmu
     MAKE_CHECK_ARGS: check-build
   artifacts:
@@ -464,7 +461,7 @@  tsan-build:
   variables:
     IMAGE: ubuntu2004
     CONFIGURE_ARGS: --enable-tsan --cc=clang-10 --cxx=clang++-10
-          --enable-trace-backends=ust --enable-fdt=system --disable-slirp
+          --enable-trace-backends=ust --disable-slirp
     TARGETS: x86_64-softmmu ppc64-softmmu riscv64-softmmu x86_64-linux-user
     MAKE_CHECK_ARGS: bench V=1
 
diff --git a/.gitlab-ci.d/crossbuild-template.yml b/.gitlab-ci.d/crossbuild-template.yml
index 6d709628f1..d07989e3b0 100644
--- a/.gitlab-ci.d/crossbuild-template.yml
+++ b/.gitlab-ci.d/crossbuild-template.yml
@@ -6,8 +6,9 @@ 
   script:
     - mkdir build
     - cd build
-    - ../configure --enable-werror --disable-docs $QEMU_CONFIGURE_OPTS
-        --disable-user --target-list-exclude="arm-softmmu cris-softmmu
+    - ../configure --enable-werror --disable-docs --enable-fdt=system
+        --disable-user $QEMU_CONFIGURE_OPTS $EXTRA_CONFIGURE_OPTS
+        --target-list-exclude="arm-softmmu cris-softmmu
           i386-softmmu microblaze-softmmu mips-softmmu mipsel-softmmu
           mips64-softmmu ppc-softmmu riscv32-softmmu sh4-softmmu
           sparc-softmmu xtensa-softmmu $CROSS_SKIP_TARGETS"
diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
index 57637c5127..101416080c 100644
--- a/.gitlab-ci.d/crossbuilds.yml
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -175,6 +175,7 @@  cross-win32-system:
     job: win32-fedora-cross-container
   variables:
     IMAGE: fedora-win32-cross
+    EXTRA_CONFIGURE_OPTS: --enable-fdt=internal
     CROSS_SKIP_TARGETS: alpha-softmmu avr-softmmu hppa-softmmu m68k-softmmu
                         microblazeel-softmmu mips64el-softmmu nios2-softmmu
   artifacts:
@@ -187,6 +188,7 @@  cross-win64-system:
     job: win64-fedora-cross-container
   variables:
     IMAGE: fedora-win64-cross
+    EXTRA_CONFIGURE_OPTS: --enable-fdt=internal
     CROSS_SKIP_TARGETS: alpha-softmmu avr-softmmu hppa-softmmu
                         m68k-softmmu microblazeel-softmmu nios2-softmmu
                         or1k-softmmu rx-softmmu sh4eb-softmmu sparc64-softmmu
diff --git a/.gitlab-ci.d/windows.yml b/.gitlab-ci.d/windows.yml
index cf445b77f6..87235e43b4 100644
--- a/.gitlab-ci.d/windows.yml
+++ b/.gitlab-ci.d/windows.yml
@@ -38,6 +38,7 @@  msys2-64bit:
       mingw-w64-x86_64-capstone
       mingw-w64-x86_64-curl
       mingw-w64-x86_64-cyrus-sasl
+      mingw-w64-x86_64-dtc
       mingw-w64-x86_64-gcc
       mingw-w64-x86_64-glib2
       mingw-w64-x86_64-gnutls
@@ -71,7 +72,7 @@  msys2-64bit:
   # for the msys2 64-bit job, due to the build could not complete within
   # the project timeout.
   - ..\msys64\usr\bin\bash -lc '../configure --target-list=x86_64-softmmu
-      --without-default-devices'
+      --without-default-devices --enable-fdt=system'
   - ..\msys64\usr\bin\bash -lc 'make'
   # qTests don't run successfully with "--without-default-devices",
   # so let's exclude the qtests from CI for now.
@@ -86,6 +87,7 @@  msys2-32bit:
       mingw-w64-i686-capstone
       mingw-w64-i686-curl
       mingw-w64-i686-cyrus-sasl
+      mingw-w64-i686-dtc
       mingw-w64-i686-gcc
       mingw-w64-i686-glib2
       mingw-w64-i686-gnutls
@@ -113,7 +115,8 @@  msys2-32bit:
   - $env:MSYS = 'winsymlinks:native' # Enable native Windows symlink
   - mkdir output
   - cd output
-  - ..\msys64\usr\bin\bash -lc '../configure --target-list=ppc64-softmmu'
+  - ..\msys64\usr\bin\bash -lc '../configure --target-list=ppc64-softmmu
+                                --enable-fdt=system'
   - ..\msys64\usr\bin\bash -lc 'make'
   - ..\msys64\usr\bin\bash -lc 'make check MTESTARGS=\"--no-suite qtest\" ||
                                 { cat meson-logs/testlog.txt; exit 1; }'