Re: [RFC PATCH 2/2] GitLab CI: crude mapping of PMM's scripts to jobs

[Thomas Huth]

On 03/02/2020 04.23, Cleber Rosa wrote:
> This is a crude and straightforward mapping of Peter's
> "remake-merge-builds" and "pull-buildtest" scripts.
> 
> Some characteristics were removed for simplicity sake (but eventually
> will), including:
>  * number of simultaneous make jobs
>  * make's synchronous output, not needed because of previous point
>  * out-of-tree builds
> 
> This covers the "x86-64 Linux with a variety of different build
> configs"[1].  I've personally tested all of them, and only had
> issues with the "notcg" job[2], but it seems to be a test specific
> issue with the nested KVM I was using.
> 
> [1] - https://wiki.qemu.org/Requirements/GatingCI#Current_Tests
> [2] - https://paste.centos.org/view/1dd43a1c
> 
> Signed-off-by: Cleber Rosa <address@hidden>
> ---
>  .gitlab-ci.yml | 116 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 116 insertions(+)

Thanks for doing this! The patch looks basically fine to me, but some
comments below...

> diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
> index d2c7d2198e..eb4077e2ab 100644
> --- a/.gitlab-ci.yml
> +++ b/.gitlab-ci.yml
> @@ -2,6 +2,8 @@ include:
>    - local: '/.gitlab-ci-edk2.yml'
>  
>  build-system1:
> + rules:
> + - if: '$CI_COMMIT_REF_NAME != "staging"'
>   before_script: &before_scr_apt
>   - apt-get update -qq
>   - apt-get install -y -qq flex bison libglib2.0-dev libpixman-1-dev 
> genisoimage
> @@ -17,6 +19,8 @@ build-system1:
>   - make -j2 check
>  
>  build-system2:
> + rules:
> + - if: '$CI_COMMIT_REF_NAME != "staging"'
>   before_script:
>    *before_scr_apt
>   script:
> @@ -31,6 +35,8 @@ build-system2:
>   - make -j2 check
>  
>  build-disabled:
> + rules:
> + - if: '$CI_COMMIT_REF_NAME != "staging"'
>   before_script:
>    *before_scr_apt
>   script:
> @@ -47,6 +53,8 @@ build-disabled:
>   - make -j2 check-qtest SPEED=slow
>  
>  build-tcg-disabled:
> + rules:
> + - if: '$CI_COMMIT_REF_NAME != "staging"'
>   before_script:
>    *before_scr_apt
>   script:
> @@ -67,6 +75,8 @@ build-tcg-disabled:
>              248 250 254 255 256
>  
>  build-user:
> + rules:
> + - if: '$CI_COMMIT_REF_NAME != "staging"'
>   before_script:
>    *before_scr_apt
>   script:
> @@ -78,6 +88,8 @@ build-user:
>   - make run-tcg-tests-i386-linux-user run-tcg-tests-x86_64-linux-user
>  
>  build-clang:
> + rules:
> + - if: '$CI_COMMIT_REF_NAME != "staging"'
>   before_script:
>    *before_scr_apt
>   script:
> @@ -92,6 +104,8 @@ build-clang:
>   - make -j2 check
>  
>  build-tci:
> + rules:
> + - if: '$CI_COMMIT_REF_NAME != "staging"'
>   before_script:
>    *before_scr_apt
>   script:

Question to Peter/Alex/Stefan/Howevermergespullreqsinthefuture:

Should the above jobs really be skipped for pull requests, or would it
be ok to include them there, too? (in the latter case, the above changes
could just be dropped)

> @@ -111,3 +125,105 @@ build-tci:
>   - QTEST_QEMU_BINARY="x86_64-softmmu/qemu-system-x86_64" 
> ./tests/qtest/pxe-test
>   - QTEST_QEMU_BINARY="s390x-softmmu/qemu-system-s390x"
>     ./tests/qtest/pxe-test -m slow
> +
> +ubuntu-18.04.3-x86_64-notools:
> + tags:
> + - ubuntu_18.04.3
> + - x86_64
> + rules:
> + - if: '$CI_COMMIT_REF_NAME == "staging"'
> + script:
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/remake-merge-builds#n22

Line 22 currently seems to be disabled, so I think you could drop this job?

> + - ./configure --target-list=arm-softmmu --disable-tools --disable-libssh
> + # There is no make / make check in the "pull-buildtest" script for this.
> + # Question: should it at least be built? Or dropped?
> + - make
> +
> +ubuntu-18.04.3-x86_64-all-linux-static:
> + tags:
> + - ubuntu_18.04.3
> + - x86_64
> + rules:
> + - if: '$CI_COMMIT_REF_NAME == "staging"'
> + script:
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/remake-merge-builds#n25
> + - ./configure --enable-debug --static --disable-system --disable-glusterfs 
> --disable-libssh
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/pull-buildtest#n36
> + - make
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/pull-buildtest#n45
> + - make check V=1
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/pull-buildtest#n48
> + - make check-tcg V=1
> +
> +ubuntu-18.04.3-x86_64-all:
> + tags:
> + - ubuntu_18.04.3
> + - x86_64
> + rules:
> + - if: '$CI_COMMIT_REF_NAME == "staging"'
> + script:
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/remake-merge-builds#n26
> + - ./configure --disable-libssh
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/pull-buildtest#n28
> + - make
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/pull-buildtest#n37
> + - make check V=1
> +
> +ubuntu-18.04.3-x86_64-alldbg:
> + tags:
> + - ubuntu_18.04.3
> + - x86_64
> + rules:
> + - if: '$CI_COMMIT_REF_NAME == "staging"'
> + script:
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/remake-merge-builds#n27
> + - ./configure --disable-libssh
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/pull-buildtest#n27
> + - make clean
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/pull-buildtest#n29
> + - make
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/pull-buildtest#n37
> + - make check V=1
> +
> +ubuntu-18.04.3-x86_64-clang:
> + tags:
> + - ubuntu_18.04.3
> + - x86_64
> + rules:
> + - if: '$CI_COMMIT_REF_NAME == "staging"'
> + script:
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/remake-merge-builds#n31
> + - ./configure --disable-libssh --cc=clang --cxx=clang++ --enable-gtk 
> --extra-cflags='-fsanitize=undefined -fno-sanitize=shift-base -Werror'
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/pull-buildtest#n33
> + - make
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/pull-buildtest#n39
> + - make check V=1
> +
> +ubuntu-18.04.3-x86_64-tci:
> + tags:
> + - ubuntu_18.04.3
> + - x86_64
> + rules:
> + - if: '$CI_COMMIT_REF_NAME == "staging"'
> + script:
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/remake-merge-builds#n33
> + - ./configure --disable-libssh --enable-debug --enable-tcg-interpreter
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/pull-buildtest#n34
> + - make

I think "make check" should now work with TCI, too, there was a fix for
it some months ago (commit 2f160e0f9797c7522bfd0d09218d0c9340a5137c).
At least it's worth a try to enable it...

Alternatively, I think we don't urgently need to introduce this job -
there is already "build-tci" in gitlab-ci.yml which should cover
--enable-tcg-interpreter already and does some testing, too.

> +
> +ubuntu-18.04.3-x86_64-notcg:
> + tags:
> + - ubuntu_18.04.3
> + - x86_64
> + rules:
> + - if: '$CI_COMMIT_REF_NAME == "staging"'
> + script:
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/remake-merge-builds#n35
> + - ./configure --disable-libssh --enable-debug --disable-tcg
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/pull-buildtest#n35
> + - make
> + # 
> https://git.linaro.org/people/peter.maydell/misc-scripts.git/tree/pull-buildtest#n39
> + # Question: check is disabled on the original script, because the machine
> + # is said to be running VirtualBox.  Should this be dropped or should the
> + # machine be tweaked or substituted?
> + - make check V=1

Without TCG, you definitely need a host that can do KVM for running make
check.
Question for Peter: Would it be ok to drop this job and simply always
use the "build-tcg-disabled" job that is already available in
.gitlab-ci.yml ?

 Thomas