[PATCH v2 0/2] ci: Remove cirrus-ci & cover SPICE in MSYS2 at gitlab-ci

[PATCH v2 0/2] ci: Remove cirrus-ci & cover SPICE in MSYS2 at gitlab-ci

[Philippe Mathieu-Daudé]

See individual patch descriptions.

Based-on: <20230322075256.2043812-1-marcandre.lureau@redhat.com>
Supersedes: <20230322110521.59949-1-philmd@linaro.org>

Philippe Mathieu-Daudé (2):
  gitlab-ci: Cover SPICE in the MSYS2 job
  cirrus-ci: Remove MSYS2 jobs duplicated with gitlab-ci

 MAINTAINERS              |   3 +-
 .cirrus.yml              | 111 ---------------------------------------
 .gitlab-ci.d/windows.yml |   2 +
 3 files changed, 3 insertions(+), 113 deletions(-)
 delete mode 100644 .cirrus.yml

-- 
2.38.1

[PATCH v2 1/2] gitlab-ci: Cover SPICE in the MSYS2 job

[Philippe Mathieu-Daudé]

Include the mingw-w64-x86_64-spice package so SPICE is covered:

  C compiler for the host machine: cc -m64 -mcx16 (gcc 12.2.0 "cc (Rev10, Built by MSYS2 project) 12.2.0")
  ...
  Run-time dependency spice-protocol found: YES 0.14.4
  Run-time dependency spice-server found: YES 0.15.1

In particular this would have helped catching the build issue
reported as https://gitlab.com/qemu-project/qemu/-/issues/1553:

  [1851/5253] Compiling C object libcommon.fa.p/ui_spice-core.c.obj
  FAILED: libcommon.fa.p/ui_spice-core.c.obj
  ../ui/spice-core.c: In function 'watch_remove':
  ../ui/spice-core.c:152:5: error: implicit declaration of function 'qemu_close_to_socket' [-Werror=implicit-function-declaration]
  152 |     qemu_close_to_socket(watch->fd);
      |     ^~~~~~~~~~~~~~~~~~~~
  ../ui/spice-core.c:152:5: error: nested extern declaration of 'qemu_close_to_socket' [-Werror=nested-externs]

Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
See https://gitlab.com/philmd/qemu/-/jobs/3980693656#L2378
---
 .gitlab-ci.d/windows.yml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/.gitlab-ci.d/windows.yml b/.gitlab-ci.d/windows.yml
index 87235e43b4..472bacd2e2 100644
--- a/.gitlab-ci.d/windows.yml
+++ b/.gitlab-ci.d/windows.yml
@@ -59,6 +59,7 @@ msys2-64bit:
       mingw-w64-x86_64-SDL2
       mingw-w64-x86_64-SDL2_image
       mingw-w64-x86_64-snappy
+      mingw-w64-x86_64-spice
       mingw-w64-x86_64-usbredir
       mingw-w64-x86_64-zstd "
   - $env:CHERE_INVOKING = 'yes'  # Preserve the current working directory
@@ -108,6 +109,7 @@ msys2-32bit:
       mingw-w64-i686-SDL2
       mingw-w64-i686-SDL2_image
       mingw-w64-i686-snappy
+      mingw-w64-i686-spice
       mingw-w64-i686-usbredir
       mingw-w64-i686-zstd "
   - $env:CHERE_INVOKING = 'yes'  # Preserve the current working directory
-- 
2.38.1

Re: [PATCH v2 1/2] gitlab-ci: Cover SPICE in the MSYS2 job

[Daniel P. Berrangé]

On Wed, Mar 22, 2023 at 02:57:20PM +0100, Philippe Mathieu-Daudé wrote:
> Include the mingw-w64-x86_64-spice package so SPICE is covered:
> 
>   C compiler for the host machine: cc -m64 -mcx16 (gcc 12.2.0 "cc (Rev10, Built by MSYS2 project) 12.2.0")
>   ...
>   Run-time dependency spice-protocol found: YES 0.14.4
>   Run-time dependency spice-server found: YES 0.15.1
> 
> In particular this would have helped catching the build issue
> reported as https://gitlab.com/qemu-project/qemu/-/issues/1553:
> 
>   [1851/5253] Compiling C object libcommon.fa.p/ui_spice-core.c.obj
>   FAILED: libcommon.fa.p/ui_spice-core.c.obj
>   ../ui/spice-core.c: In function 'watch_remove':
>   ../ui/spice-core.c:152:5: error: implicit declaration of function 'qemu_close_to_socket' [-Werror=implicit-function-declaration]
>   152 |     qemu_close_to_socket(watch->fd);
>       |     ^~~~~~~~~~~~~~~~~~~~
>   ../ui/spice-core.c:152:5: error: nested extern declaration of 'qemu_close_to_socket' [-Werror=nested-externs]
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> See https://gitlab.com/philmd/qemu/-/jobs/3980693656#L2378
> ---
>  .gitlab-ci.d/windows.yml | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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

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

[Philippe Mathieu-Daudé]

- Various developers are reluctant to git Cirrus-CI the permissions
  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.

- 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/MAINTAINERS b/MAINTAINERS
index 9b56ccdd92..34b50b267c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3818,8 +3818,7 @@ W: https://cirrus-ci.com/github/qemu/qemu
 Windows Hosted Continuous Integration
 M: Yonggang Luo <luoyonggang@gmail.com>
 S: Maintained
-F: .cirrus.yml
-W: https://cirrus-ci.com/github/qemu/qemu
+F: .gitlab-ci.d/windows.yml
 
 Guest Test Compilation Support
 M: Alex Bennée <alex.bennee@linaro.org>
diff --git a/.cirrus.yml b/.cirrus.yml
deleted file mode 100644
index 5fb00da73d..0000000000
--- a/.cirrus.yml
+++ /dev/null
@@ -1,111 +0,0 @@
-env:
-  CIRRUS_CLONE_DEPTH: 1
-
-windows_msys2_task:
-  timeout_in: 90m
-  windows_container:
-    image: cirrusci/windowsservercore:2019
-    os_version: 2019
-    cpu: 8
-    memory: 8G
-  env:
-    CIRRUS_SHELL: powershell
-    MSYS: winsymlinks:native
-    MSYSTEM: MINGW64
-    MSYS2_URL: https://github.com/msys2/msys2-installer/releases/download/2022-06-03/msys2-base-x86_64-20220603.sfx.exe
-    MSYS2_FINGERPRINT: 0
-    MSYS2_PACKAGES: "
-      diffutils git grep make pkg-config sed
-      mingw-w64-x86_64-python
-      mingw-w64-x86_64-python-sphinx
-      mingw-w64-x86_64-toolchain
-      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
-      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
-    "
-    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"
-    - C:\tools\msys64\usr\bin\bash.exe -lc "make -j8"
-    - exit $LastExitCode
-  test_script:
-    - C:\tools\msys64\usr\bin\bash.exe -lc "cd build && make V=1 check"
-    - exit $LastExitCode
-- 
2.38.1

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

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

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

[Daniel P. Berrangé]

On Thu, Mar 23, 2023 at 09:37:40AM +0100, Thomas Huth wrote:
> 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.

IMHO, we should have the same deps present in all CI areas. If we
then need to skip docs because of speed lets pass --disable-docs
so that it is explicit that we're skipping them, rather than having
to infer the intention from the missing deps.

My hope would be that we can ultimately make the package listing huere
be auto-generated by lcitool too. It likely only needs a few naming
tweaks here & there for packages to get it working. At that point we
would need to control disablement via  configure flags.

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

Ok, yes, better to list exactly what we want.

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

Ok, all makes sense.

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

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

Re: [PATCH v2 0/2] ci: Remove cirrus-ci & cover SPICE in MSYS2 at gitlab-ci

[Richard Henderson]

On 3/22/23 06:57, Philippe Mathieu-Daudé wrote:
> See individual patch descriptions.
> 
> Based-on:<20230322075256.2043812-1-marcandre.lureau@redhat.com>
> Supersedes:<20230322110521.59949-1-philmd@linaro.org>
> 
> Philippe Mathieu-Daudé (2):
>    gitlab-ci: Cover SPICE in the MSYS2 job
>    cirrus-ci: Remove MSYS2 jobs duplicated with gitlab-ci

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~