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

[Thomas Huth]

On 22/03/2023 19.37, Daniel P. Berrangé wrote:
> On Wed, Mar 22, 2023 at 02:57:21PM +0100, Philippe Mathieu-Daudé wrote:
...
> > 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

I think that's fine. The gitlab CI Windows jobs are very slow and ran into 
timeout issues in the past already, so we certainly don't want to waste our 
time there with building the documentation.

> > -      mingw-w64-x86_64-toolchain
>
> This also isn't listed

Seems to be a "group" package:

 https://packages.msys2.org/groups/mingw-w64-x86_64-toolchain

It includes other languages like Fortran and Ada ... I think we don't want 
that in the gitlab-CI job.

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

I think jemalloc is very niche these days for building QEMU, especially on 
Windows, so I'd rather not use it there.

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

dtc for avoiding to recompile the submodule, gcc and pkgconf as replacement 
for the toolchain group package.

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

Well, different configs also help to test different things... but in this 
case, I think it's not enough to justify keeping the cirrus-ci.yml file.

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

The gitlab job struggles with timeout issues when you add more stuff to it ...

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

Yes, that's the main point! cirrus-ci.yml even does not provide e-mail 
notification when things go wrong, so most issues that could be discovered 
here just go completely unnoticed.

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

Much, much 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

The -j2 has been removed in commit 5f2992fe1ec5594e5c since we had some 
weird failures with it, likely due to memory constraints in those 
containers. After removing -j2, the jobs were reliable
again.

> 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

You can have a try, but from my experience, it will be very hard to increase 
the test coverage of those jobs without hitting timeout issues again.

 Thomas