[v1] cirrus.yml: Cover SPICE in the MSYS2 job

[PATCH] cirrus.yml: Cover SPICE in the MSYS2 job

Posted by Philippe Mathieu-Daudé 1 year, 1 month ago

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]

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
See https://cirrus-ci.com/task/6189732569284608?logs=main#L891

Based-on: <20230322075256.2043812-1-marcandre.lureau@redhat.com>
---
 .cirrus.yml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.cirrus.yml b/.cirrus.yml
index 5fb00da73d..eb1640814d 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -41,6 +41,7 @@ windows_msys2_task:
       mingw-w64-x86_64-curl
       mingw-w64-x86_64-gnutls
       mingw-w64-x86_64-libnfs
+      mingw-w64-x86_64-spice
     "
     CHERE_INVOKING: 1
   msys2_cache:
-- 
2.38.1

Re: [PATCH] cirrus.yml: Cover SPICE in the MSYS2 job

Posted by Thomas Huth 1 year, 1 month ago

On 22/03/2023 12.05, 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:

Well, hardly anybody is looking at the output on cirrus-ci.com, so this 
patch here likely would not have prevented the merging of this bug... for 
that, you rather have to add it to .gitlab-ci.d/windows.yml instead.

But anyway, the patch here is still a good idea, too, so:

Reviewed-by: Thomas Huth <thuth@redhat.com>


>    [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]
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> See https://cirrus-ci.com/task/6189732569284608?logs=main#L891
> 
> Based-on: <20230322075256.2043812-1-marcandre.lureau@redhat.com>
> ---
>   .cirrus.yml | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/.cirrus.yml b/.cirrus.yml
> index 5fb00da73d..eb1640814d 100644
> --- a/.cirrus.yml
> +++ b/.cirrus.yml
> @@ -41,6 +41,7 @@ windows_msys2_task:
>         mingw-w64-x86_64-curl
>         mingw-w64-x86_64-gnutls
>         mingw-w64-x86_64-libnfs
> +      mingw-w64-x86_64-spice
>       "
>       CHERE_INVOKING: 1
>     msys2_cache:

Re: [PATCH] cirrus.yml: Cover SPICE in the MSYS2 job

Posted by Philippe Mathieu-Daudé 1 year, 1 month ago

On 22/3/23 12:30, Thomas Huth wrote:
> On 22/03/2023 12.05, 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:
> 
> Well, hardly anybody is looking at the output on cirrus-ci.com, so this 
> patch here likely would not have prevented the merging of this bug... 
> for that, you rather have to add it to .gitlab-ci.d/windows.yml instead.

It seems we have 2 jobs doing the same testing (even same version,
msys2-base-x86_64-20220603.sfx.exe). Does it make sense to maintain
both? At least with GitLab we notice a failure.

Re: [PATCH] cirrus.yml: Cover SPICE in the MSYS2 job

Posted by Thomas Huth 1 year, 1 month ago

On 22/03/2023 13.13, Philippe Mathieu-Daudé wrote:
> On 22/3/23 12:30, Thomas Huth wrote:
>> On 22/03/2023 12.05, 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:
>>
>> Well, hardly anybody is looking at the output on cirrus-ci.com, so this 
>> patch here likely would not have prevented the merging of this bug... for 
>> that, you rather have to add it to .gitlab-ci.d/windows.yml instead.
> 
> It seems we have 2 jobs doing the same testing (even same version,
> msys2-base-x86_64-20220603.sfx.exe). Does it make sense to maintain
> both? At least with GitLab we notice a failure.

We can test more things (targets) in the Cirrus-CI job since the build 
machines are way more powerful there ... but yes, since people rarely look 
at this output, it's maybe better to disable them and focus on the gitlab-CI 
jobs instead.

  Thomas

Re: [PATCH] cirrus.yml: Cover SPICE in the MSYS2 job

Posted by Daniel P. Berrangé 1 year, 1 month ago

On Wed, Mar 22, 2023 at 01:13:21PM +0100, Philippe Mathieu-Daudé wrote:
> On 22/3/23 12:30, Thomas Huth wrote:
> > On 22/03/2023 12.05, 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:
> > 
> > Well, hardly anybody is looking at the output on cirrus-ci.com, so this
> > patch here likely would not have prevented the merging of this bug...
> > for that, you rather have to add it to .gitlab-ci.d/windows.yml instead.
> 
> It seems we have 2 jobs doing the same testing (even same version,
> msys2-base-x86_64-20220603.sfx.exe). Does it make sense to maintain
> both? At least with GitLab we notice a failure.

IMHO having the .cirrus.yml file can probably be considered actively
harmful.  Cirrus CI has a concurrency limit of 2 jobs, and when we
trigger Cirrus jobs from GitLab CII using cirrus-run, we need them
to completely in < 1 hour.

If we have extra jobs being triggered by the .cirrus.yml file when
gitlab.com repo is mirrored to github.com, then we're liable to
cause timeouts of the jobs we do care about from cirrus-run.

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