Re: [PATCH v2 2/2] cirrus-ci: Remove MSYS2 jobs duplicated with gitlab-ci

[Daniel P . Berrangé]

On Wed, Mar 22, 2023 at 02:57:21PM +0100, Philippe Mathieu-Daudé wrote:
> - Various developers are reluctant to git Cirrus-CI the permissions

                                      s/git/give/

>   requested to access their GitHub account.
> 
> - When we use the cirrus-run script to trigger Cirrus-CI job from
>   GitLab-CI, the GitLab-CI job is restricted to a 1h timeout
>   (often not enough).
> 
> - Although Cirrus-CI VMs are more powerful than GitLab-CI ones,
>   its free plan is limited in 2 concurrent jobs.

Right and these two points combine to create us problems. Pushes to
gitlab get automatically mirrored to github, and thus in turn trigger
CI jobs directly on Cirrus.

These jobs can prevent the next PULL on the staging branch from getting
enough execution time, and make it more likely they'll hit the 1h timeout.
This ultimately reduces the reliability of the Cirrus CI jobs for FreeBSD
and macOS that we do care about from GitLab CI.

> - The GitLab-CI MSYS2 jobs are a 1:1 mapping with the Cirrus-CI ones
>   (modulo the environment caching).
> 
> Reduce the maintenance burden by removing the Cirrus-CI config file,
> keeping the GitLab-CI jobs.
> 
> Update Yonggang Luo's maintenance file list to the new file, which
> use the same environment shell.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  MAINTAINERS |   3 +-
>  .cirrus.yml | 111 ----------------------------------------------------
>  2 files changed, 1 insertion(+), 113 deletions(-)
>  delete mode 100644 .cirrus.yml

> diff --git a/.cirrus.yml b/.cirrus.yml
> deleted file mode 100644
> index 5fb00da73d..0000000000
> --- a/.cirrus.yml
> +++ /dev/null

> -    MSYS2_PACKAGES: "
> -      diffutils git grep make pkg-config sed
> -      mingw-w64-x86_64-python
> -      mingw-w64-x86_64-python-sphinx

This isn't listed in the .gitlab-ci.d/windows.yml file

> -      mingw-w64-x86_64-toolchain

This also isn't listed

> -      mingw-w64-x86_64-SDL2
> -      mingw-w64-x86_64-SDL2_image
> -      mingw-w64-x86_64-gtk3
> -      mingw-w64-x86_64-glib2
> -      mingw-w64-x86_64-ninja
> -      mingw-w64-x86_64-jemalloc

This also isn't listed

> -      mingw-w64-x86_64-lzo2
> -      mingw-w64-x86_64-zstd
> -      mingw-w64-x86_64-libjpeg-turbo
> -      mingw-w64-x86_64-pixman
> -      mingw-w64-x86_64-libgcrypt
> -      mingw-w64-x86_64-libpng
> -      mingw-w64-x86_64-libssh
> -      mingw-w64-x86_64-snappy
> -      mingw-w64-x86_64-libusb
> -      mingw-w64-x86_64-usbredir
> -      mingw-w64-x86_64-libtasn1
> -      mingw-w64-x86_64-nettle
> -      mingw-w64-x86_64-cyrus-sasl
> -      mingw-w64-x86_64-curl
> -      mingw-w64-x86_64-gnutls
> -      mingw-w64-x86_64-libnfs

The  .gitlab-ci.d/windows.yml file meanwhile adds 'dtc' 'gcc'
and 'pkgconf' which are not present here.

This inconsistency is another point in favour of removing this
redundant cirrus config.

> -    "
> -    CHERE_INVOKING: 1
> -  msys2_cache:
> -    folder: C:\tools\archive
> -    reupload_on_changes: false
> -    # These env variables are used to generate fingerprint to trigger the 
> cache procedure
> -    # If wanna to force re-populate msys2, increase MSYS2_FINGERPRINT
> -    fingerprint_script:
> -      - |
> -        echo $env:CIRRUS_TASK_NAME
> -        echo $env:MSYS2_URL
> -        echo $env:MSYS2_FINGERPRINT
> -        echo $env:MSYS2_PACKAGES
> -    populate_script:
> -      - |
> -        md -Force C:\tools\archive\pkg
> -        $start_time = Get-Date
> -        bitsadmin /transfer msys_download /dynamic /download /priority 
> FOREGROUND $env:MSYS2_URL C:\tools\archive\base.exe
> -        Write-Output "Download time taken: 
> $((Get-Date).Subtract($start_time))"
> -        cd C:\tools
> -        C:\tools\archive\base.exe -y
> -        del -Force C:\tools\archive\base.exe
> -        Write-Output "Base install time taken: 
> $((Get-Date).Subtract($start_time))"
> -        $start_time = Get-Date
> -
> -        ((Get-Content -path 
> C:\tools\msys64\etc\\post-install\\07-pacman-key.post -Raw) -replace 
> '--refresh-keys', '--version') | Set-Content -Path 
> C:\tools\msys64\etc\\post-install\\07-pacman-key.post
> -        C:\tools\msys64\usr\bin\bash.exe -lc "sed -i 
> 's/^CheckSpace/#CheckSpace/g' /etc/pacman.conf"
> -        C:\tools\msys64\usr\bin\bash.exe -lc "export"
> -        C:\tools\msys64\usr\bin\pacman.exe --noconfirm -Sy
> -        echo Y | C:\tools\msys64\usr\bin\pacman.exe --noconfirm -Suu 
> --overwrite=*
> -        taskkill /F /FI "MODULES eq msys-2.0.dll"
> -        tasklist
> -        C:\tools\msys64\usr\bin\bash.exe -lc "mv -f /etc/pacman.conf.pacnew 
> /etc/pacman.conf || true"
> -        C:\tools\msys64\usr\bin\bash.exe -lc "pacman --noconfirm -Syuu 
> --overwrite=*"
> -        Write-Output "Core install time taken: 
> $((Get-Date).Subtract($start_time))"
> -        $start_time = Get-Date
> -
> -        C:\tools\msys64\usr\bin\bash.exe -lc "pacman --noconfirm -S --needed 
> $env:MSYS2_PACKAGES"
> -        Write-Output "Package install time taken: 
> $((Get-Date).Subtract($start_time))"
> -        $start_time = Get-Date
> -
> -        del -Force -ErrorAction SilentlyContinue C:\tools\msys64\etc\mtab
> -        del -Force -ErrorAction SilentlyContinue C:\tools\msys64\dev\fd
> -        del -Force -ErrorAction SilentlyContinue C:\tools\msys64\dev\stderr
> -        del -Force -ErrorAction SilentlyContinue C:\tools\msys64\dev\stdin
> -        del -Force -ErrorAction SilentlyContinue C:\tools\msys64\dev\stdout
> -        del -Force -Recurse -ErrorAction SilentlyContinue 
> C:\tools\msys64\var\cache\pacman\pkg
> -        tar cf C:\tools\archive\msys64.tar -C C:\tools\ msys64
> -
> -        Write-Output "Package archive time taken: 
> $((Get-Date).Subtract($start_time))"
> -        del -Force -Recurse -ErrorAction SilentlyContinue c:\tools\msys64 
> -  install_script:
> -    - |
> -      $start_time = Get-Date
> -      cd C:\tools
> -      ls C:\tools\archive\msys64.tar
> -      tar xf C:\tools\archive\msys64.tar
> -      Write-Output "Extract msys2 time taken: 
> $((Get-Date).Subtract($start_time))"
> -  script:
> -    - mkdir build
> -    - cd build
> -    - C:\tools\msys64\usr\bin\bash.exe -lc "../configure --python=python3
> -        
> --target-list-exclude=i386-softmmu,ppc64-softmmu,aarch64-softmmu,mips64-softmmu,mipsel-softmmu,sh4-softmmu"

This excludes a few targets, but the .gitlab-ci.d/windows.yml file
merely allow-lists  x86_64-softmmu only, and also adds
--without-default-devices

IOW the remaining config has less coverage than this one. Of course
if no one ever looks at these results, the better coverage is not
doing anything for us.

> -    - C:\tools\msys64\usr\bin\bash.exe -lc "make -j8"

The .gitlab-ci.d/windows.yml file does not pass '-j8' so presumably
runs slower.

THe gitlab docs indicate the Windows VMs have 2 vCPUs so we ought to
have been using -j2 in the .gitlab-ci.d/windows.yml file IIUC

> -    - exit $LastExitCode
> -  test_script:

> -    - C:\tools\msys64\usr\bin\bash.exe -lc "cd build && make V=1 check"

The .gitlab-ci.d/windows.yml file passes '--no-suite qtest' so appears
to have less test coverage


Broadly I agree with this proposal, but it feels like we might want a
few tweak to the windows.yml file to address some of the inconsistencies



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