Re: [PATCH 1/9] automation: Use custom build jobs when extra config options are needed

[Michal Orzel <michal.orzel@amd.com>]

Hi Stefano,

On 22/09/2022 23:40, Stefano Stabellini wrote:
> 
> 
> On Thu, 22 Sep 2022, Michal Orzel wrote:
>> Currently, all the arm64 defconfig build jobs, regardless of the
>> container used, end up building Xen with the extra config options
>> specified in the main build script (e.g. CONFIG_EXPERT,
>> CONFIG_STATIC_MEMORY). Because these options are only needed for
>> specific test jobs, the current behavior of the CI is incorrect
>> as we add the extra options to all the defconfig builds. This means
>> that on arm64 there is not a single job performing proper defconfig build.
>>
>> To fix this issue, add custom build jobs each time there is a need for
>> building Xen with additional config options. Introduce EXTRA_XEN_CONFIG
>> variable to be used by these jobs to store the required options. This
>> variable will be then read by the main build script to modify the .config
>> file. This will also help users to understand what is needed to run specific
>> test.
>>
>> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
>> ---
>> This patch could actually be consider to be taken for 4.17 release.
>> The reason why is because currently our CI for arm64 does not even
>> peform clean defconfig build which is quite crucial target to be tested.
>> Performing builds always with EXPERT and UNSUPPORTED is not something so
>> beneficial for release tests. This is up to the release manager.
> 
> + Henry
> 
> I agree this should go in 4.17, so that gitlab-ci can test non-DEBUG
> builds on ARM.
> 
Do you mean the whole series should go in?
I'm ok with that, even though I only marked this patch as the one that should go in
as it can be seen as a fix. But I can also see the benefits of merging the whole series.

> 
>> ---
>>  automation/gitlab-ci/build.yaml | 15 +++++++++++++++
>>  automation/gitlab-ci/test.yaml  |  4 ++--
>>  automation/scripts/build        |  8 ++------
>>  3 files changed, 19 insertions(+), 8 deletions(-)
>>
>> diff --git a/automation/gitlab-ci/build.yaml b/automation/gitlab-ci/build.yaml
>> index 720ce6e07ba0..a39ed72aac6d 100644
>> --- a/automation/gitlab-ci/build.yaml
>> +++ b/automation/gitlab-ci/build.yaml
>> @@ -566,6 +566,21 @@ alpine-3.12-gcc-debug-arm64:
>>    variables:
>>      CONTAINER: alpine:3.12-arm64v8
>>
>> +alpine-3.12-gcc-arm64-staticmem:
>> +  extends: .gcc-arm64-build
>> +  variables:
>> +    CONTAINER: alpine:3.12-arm64v8
>> +    EXTRA_XEN_CONFIG: |
> 
> Why the "|" ?
> 
> I was trying to look for its documentation in the gitlab yaml docs but
> couldn't find it.
> 
By default gitlab variables are one liners so that they can store one key:value pair.
If you want to define a variable storing multiple values (in this case we want to
store multi-line string because of .config format) you need to use |. You can check [1].

> 
>> +      CONFIG_EXPERT=y
>> +      CONFIG_UNSUPPORTED=y
>> +      CONFIG_STATIC_MEMORY=y
>> +
>> +alpine-3.12-gcc-arm64-boot-cpupools:
>> +  extends: .gcc-arm64-build
>> +  variables:
>> +    CONTAINER: alpine:3.12-arm64v8
>> +    EXTRA_XEN_CONFIG: |
>> +      CONFIG_BOOT_TIME_CPUPOOLS=y
>>
>>  ## Test artifacts common
>>
>> diff --git a/automation/gitlab-ci/test.yaml b/automation/gitlab-ci/test.yaml
>> index d899b3bdbf7a..4f96e6e322de 100644
>> --- a/automation/gitlab-ci/test.yaml
>> +++ b/automation/gitlab-ci/test.yaml
>> @@ -88,7 +88,7 @@ qemu-smoke-arm64-gcc-staticmem:
>>    script:
>>      - ./automation/scripts/qemu-smoke-arm64.sh static-mem 2>&1 | tee qemu-smoke-arm64.log
>>    needs:
>> -    - alpine-3.12-gcc-arm64
>> +    - alpine-3.12-gcc-arm64-staticmem
>>      - alpine-3.12-arm64-rootfs-export
>>      - kernel-5.19-arm64-export
>>      - qemu-system-aarch64-6.0.0-arm64-export
>> @@ -107,7 +107,7 @@ qemu-smoke-arm64-gcc-boot-cpupools:
>>    script:
>>      - ./automation/scripts/qemu-smoke-arm64.sh boot-cpupools 2>&1 | tee qemu-smoke-arm64.log
>>    needs:
>> -    - alpine-3.12-gcc-arm64
>> +    - alpine-3.12-gcc-arm64-boot-cpupools
>>      - alpine-3.12-arm64-rootfs-export
>>      - kernel-5.19-arm64-export
>>      - qemu-system-aarch64-6.0.0-arm64-export
>> diff --git a/automation/scripts/build b/automation/scripts/build
>> index 2f15ab3198e6..7d441cedb4ae 100755
>> --- a/automation/scripts/build
>> +++ b/automation/scripts/build
>> @@ -15,12 +15,8 @@ if [[ "${RANDCONFIG}" == "y" ]]; then
>>      make -j$(nproc) -C xen KCONFIG_ALLCONFIG=tools/kconfig/allrandom.config randconfig
>>      hypervisor_only="y"
>>  else
>> -    if [[ "${XEN_TARGET_ARCH}" = "arm64" ]]; then
>> -        echo "
>> -CONFIG_EXPERT=y
>> -CONFIG_UNSUPPORTED=y
>> -CONFIG_STATIC_MEMORY=y
>> -CONFIG_BOOT_TIME_CPUPOOLS=y" > xen/.config
>> +    if [ -n "${EXTRA_XEN_CONFIG}" ]; then
> 
> NIT: for uniformity with rest of the file use
> 
>   if [[ -n "${EXTRA_XEN_CONFIG}" ]]; then
> 
Ok, will do in v2.

> 
>> +        echo "${EXTRA_XEN_CONFIG}" > xen/.config
>>          make -j$(nproc) -C xen olddefconfig
>>      else
>>          make -j$(nproc) -C xen defconfig
>> --
>> 2.25.1
>>

~Michal

[1] https://docs.gitlab.com/ee/ci/variables/#store-multiple-values-in-one-variable