Re: [PATCH 5/9] tests/docker: add container registry to configure and use

["Alex Bennée" <alex.bennee@linaro.org>]

Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> writes:

> On 9/10/26 11:15 AM, Alex Bennée wrote:
>> Previously we had the default baked into Makefile.include but as the
>> meson container logic will also benefit from it we update the configure
>> machinery.
>> For now we allow direct make invocations to override but for the
>> normal case it is in one place now.
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>   configure                     | 8 ++++++++
>>   meson_options.txt             | 3 +++
>>   tests/docker/Makefile.include | 4 +---
>>   tests/tcg/meson.build         | 6 ++++++
>>   4 files changed, 18 insertions(+), 3 deletions(-)
>> diff --git a/configure b/configure
>> index aaa41fe47aa..7d6b11ddd35 100755
>> --- a/configure
>> +++ b/configure
>> @@ -172,6 +172,7 @@ fi
>>   # some defaults, based on the host environment
>>     # default parameters
>> +container_registry="registry.gitlab.com/qemu-project/qemu"
>>   cpu=""
>>   cross_compile="no"
>>   cross_prefix=""
>> @@ -728,6 +729,9 @@ for opt do
>>     ;;
>>     --container-command=*) meson_option_add -Dcontainer_command="$optarg"
>>     ;;
>> +  --container-registry=*) container_registry="$optarg"
>> +                          meson_option_add -Dcontainer_registry="$optarg"
>> +  ;;
>>     --rust-target-triple=*) rust_target_triple="$optarg"
>>     ;;
>>     --gdb=*) meson_option_add -Dgdb="$optarg"
>> @@ -862,6 +866,7 @@ Advanced options (experts only):
>>     --cpu=CPU                Build for host CPU [$cpu]
>>     --disable-containers     don't use containers for cross-building
>>     --container-command=CMD  which container command to use [autodetect]
>> +  --container-registry=URL which container registry to cache from [$container_registry]
>>     --gdb=GDB-path           gdb to use for gdbstub tests [autodetect]
>>     --wasm64-32bit-address-limit Restrict wasm64 address space to 32-bit (default
>>                                  is to use the whole 64-bit range).
>> @@ -1318,6 +1323,9 @@ echo "GENISOIMAGE=$genisoimage" >> $config_host_mak
>>   echo "MESON=$meson" >> $config_host_mak
>>   echo "NINJA=$ninja" >> $config_host_mak
>>   echo "EXESUF=$EXESUF" >> $config_host_mak
>> +if test -n "$container_registry"; then
>> +  echo "DOCKER_DEFAULT_REGISTRY=$container_registry" >> $config_host_mak
>> +fi
>>   if test "$default_targets" = "yes"; then
>>     echo "CONFIG_DEFAULT_TARGETS=y" >> $config_host_mak
>>   fi
>> diff --git a/meson_options.txt b/meson_options.txt
>> index 292625af08a..030231492e1 100644
>> --- a/meson_options.txt
>> +++ b/meson_options.txt
>> @@ -391,6 +391,9 @@ option('containers', type: 'boolean', value: true,
>>          description: 'use containers to cross compile tcg tests')
>>   option('container_command', type: 'string',
>>          description: 'command to build/run containers')
>> +option('container_registry', type: 'string',
>> +       value: 'registry.gitlab.com/qemu-project/qemu',
>> +       description: 'container registry to cache from')
>>     option('tcg_tests_cross_cc_aarch64', type: 'string',
>>          description: 'cc for aarch64 tcg tests')
>> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
>> index 0adddb6a5c6..74bf471f171 100644
>> --- a/tests/docker/Makefile.include
>> +++ b/tests/docker/Makefile.include
>> @@ -11,9 +11,7 @@ USER = $(if $(NOUSER),,$(shell id -un))
>>   UID = $(if $(NOUSER),,$(shell id -u))
>>     DOCKER_FILES_DIR := $(SRC_PATH)/tests/docker/dockerfiles
>> -ifeq ($(HOST_ARCH),x86_64)
>> -DOCKER_DEFAULT_REGISTRY := registry.gitlab.com/qemu-project/qemu
>> -endif
>> +DOCKER_DEFAULT_REGISTRY ?= registry.gitlab.com/qemu-project/qemu
>>   DOCKER_REGISTRY := $(if $(REGISTRY),$(REGISTRY),$(DOCKER_DEFAULT_REGISTRY))
>>
>
> Couldn't we read the default from config_host_mak generated by
> configure instead?
>
>>   CONTAINER_COMMAND ?= $(shell $(SRC_PATH)/tests/docker/docker.py probe)
>> diff --git a/tests/tcg/meson.build b/tests/tcg/meson.build
>> index 814f2f8cd8c..22157a47c96 100644
>> --- a/tests/tcg/meson.build
>> +++ b/tests/tcg/meson.build
>> @@ -101,6 +101,9 @@ if get_option('containers')
>>     endif
>>   endif
>>   tcg_tests_summary += {'container command': container_command}
>> +if get_option('container_registry') != ''
>> +  tcg_tests_summary += {'container registry': get_option('container_registry')}
>> +endif
>>     # plugins come first, as we need to build the list
>>   test_plugins = {}
>> @@ -257,6 +260,9 @@ foreach target, plan: tcg_tests
>>       if cc_dockerfile not in image_targets and has_docker
>>         cmd = [docker_wrapper, 'build', '-f', dockerfile, '-t', 'qemu/' + cc_dockerfile,
>>                '--add-current-user']
>> +      if get_option('container_registry') != ''
>> +        cmd += ['--registry', get_option('container_registry')]
>> +      endif
>>         t = custom_target(image_name, command: cmd,
>>                           build_by_default: false,
>>                           output: 'no_output_' + image_name)
>
> Do user benefit from this, or is it mostly for CI need?

The aim is for faster image builds because the CI registry should always
be up to date. Most of the time the images don't change.

>
> The effect is to use --cache-from $registry, which I'm not sure is
> what we want by default, given all the "caching issues" we had
> previously with dockerfiles already.
> If that's for CI, maybe it could simply set with --container-command
> in our CI only.
>
> Regards,
> Pierrick

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro