[PATCH 0/3] Improve FreeBSD and macOS jobs in the Cirrus-CI

[PATCH 0/3] Improve FreeBSD and macOS jobs in the Cirrus-CI

[Thomas Huth]

Our configure script does not enable -Werror on macOS and FreeBSD
by default yet. That's bad in the CI, since we might miss compiler
warnings and thus bugs this way. So after fixing a problem in the
configure script in the first patch, we now turn on -Werror here
in the second patch. The third patch is just a cosmetical one, since
Cirrus-CI seems to upgrade all jobs automatically to Cataline these
days.

Thomas Huth (3):
  configure: Fix atomic64 test for --enable-werror on macOS
  cirrus.yml: Compile macOS and FreeBSD with -Werror
  cirrus.yml: Update the macOS jobs to Catalina

 .cirrus.yml | 12 +++++++-----
 configure   | 10 +++++-----
 2 files changed, 12 insertions(+), 10 deletions(-)

-- 
2.18.1

[PATCH 1/3] configure: Fix atomic64 test for --enable-werror on macOS

[Thomas Huth]

When using --enable-werror for the macOS builders in the Cirrus-CI,
the atomic64 test is currently failing, and config.log shows a bunch
of error messages like this:

 config-temp/qemu-conf.c:6:7: error: implicit declaration of function
 '__atomic_load_8' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
  y = __atomic_load_8(&x, 0);
      ^
 config-temp/qemu-conf.c:6:7: error: this function declaration is not a
 prototype [-Werror,-Wstrict-prototypes]

Seems like these __atomic_*_8 functions are available in one of the
libraries there, so that the test links and passes there when not
using --enable-werror. But there does not seem to be a valid prototype
for them in any of the header files, so that the test fails when using
--enable-werror.

Fix it by using the "official" built-in functions instead (see e.g.
https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html).
We are not using the *_8 variants in QEMU anyway.

Suggested-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 configure | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/configure b/configure
index 4bd80ed507..9eaf501f50 100755
--- a/configure
+++ b/configure
@@ -5919,11 +5919,11 @@ int main(void)
 {
   uint64_t x = 0, y = 0;
 #ifdef __ATOMIC_RELAXED
-  y = __atomic_load_8(&x, 0);
-  __atomic_store_8(&x, y, 0);
-  __atomic_compare_exchange_8(&x, &y, x, 0, 0, 0);
-  __atomic_exchange_8(&x, y, 0);
-  __atomic_fetch_add_8(&x, y, 0);
+  y = __atomic_load_n(&x, __ATOMIC_RELAXED);
+  __atomic_store_n(&x, y, __ATOMIC_RELAXED);
+  __atomic_compare_exchange_n(&x, &y, x, 0, __ATOMIC_RELAXED, __ATOMIC_RELAXED);
+  __atomic_exchange_n(&x, y, __ATOMIC_RELAXED);
+  __atomic_fetch_add(&x, y, __ATOMIC_RELAXED);
 #else
   typedef char is_host64[sizeof(void *) >= sizeof(uint64_t) ? 1 : -1];
   __sync_lock_test_and_set(&x, y);
-- 
2.18.1

Re: [PATCH 1/3] configure: Fix atomic64 test for --enable-werror on macOS

[Christian Schoenebeck]

[-- Attachment #1: Type: text/plain, Size: 2337 bytes --]

On Freitag, 24. Juli 2020 16:32:18 CEST Thomas Huth wrote:
> When using --enable-werror for the macOS builders in the Cirrus-CI,
> the atomic64 test is currently failing, and config.log shows a bunch
> of error messages like this:
> 
>  config-temp/qemu-conf.c:6:7: error: implicit declaration of function
>  '__atomic_load_8' is invalid in C99
> [-Werror,-Wimplicit-function-declaration] y = __atomic_load_8(&x, 0);
>       ^
>  config-temp/qemu-conf.c:6:7: error: this function declaration is not a
>  prototype [-Werror,-Wstrict-prototypes]
> 
> Seems like these __atomic_*_8 functions are available in one of the
> libraries there, so that the test links and passes there when not
> using --enable-werror. But there does not seem to be a valid prototype
> for them in any of the header files, so that the test fails when using
> --enable-werror.
> 
> Fix it by using the "official" built-in functions instead (see e.g.
> https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html).
> We are not using the *_8 variants in QEMU anyway.
> 
> Suggested-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  configure | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/configure b/configure
> index 4bd80ed507..9eaf501f50 100755
> --- a/configure
> +++ b/configure
> @@ -5919,11 +5919,11 @@ int main(void)
>  {
>    uint64_t x = 0, y = 0;
>  #ifdef __ATOMIC_RELAXED
> -  y = __atomic_load_8(&x, 0);
> -  __atomic_store_8(&x, y, 0);
> -  __atomic_compare_exchange_8(&x, &y, x, 0, 0, 0);
> -  __atomic_exchange_8(&x, y, 0);
> -  __atomic_fetch_add_8(&x, y, 0);
> +  y = __atomic_load_n(&x, __ATOMIC_RELAXED);
> +  __atomic_store_n(&x, y, __ATOMIC_RELAXED);
> +  __atomic_compare_exchange_n(&x, &y, x, 0, __ATOMIC_RELAXED,
> __ATOMIC_RELAXED); +  __atomic_exchange_n(&x, y, __ATOMIC_RELAXED);

Ah right, there is also the __atomic_*_n() variant of these functions. I 
actually had the more generic variants in mind.

But LGTM and yes, it resolves the warnings on macOS, so ...

Reviewed-by: Christian Schoenebeck <qemu_oss@crudebyte.com>

> +  __atomic_fetch_add(&x, y, __ATOMIC_RELAXED);
>  #else
>    typedef char is_host64[sizeof(void *) >= sizeof(uint64_t) ? 1 : -1];
>    __sync_lock_test_and_set(&x, y);

Best regards,
Christian Schoenebeck

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH 1/3] configure: Fix atomic64 test for --enable-werror on macOS

[Alex Bennée]

Thomas Huth <thuth@redhat.com> writes:

> When using --enable-werror for the macOS builders in the Cirrus-CI,
> the atomic64 test is currently failing, and config.log shows a bunch
> of error messages like this:
>
>  config-temp/qemu-conf.c:6:7: error: implicit declaration of function
>  '__atomic_load_8' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
>   y = __atomic_load_8(&x, 0);
>       ^
>  config-temp/qemu-conf.c:6:7: error: this function declaration is not a
>  prototype [-Werror,-Wstrict-prototypes]
>
> Seems like these __atomic_*_8 functions are available in one of the
> libraries there, so that the test links and passes there when not
> using --enable-werror. But there does not seem to be a valid prototype
> for them in any of the header files, so that the test fails when using
> --enable-werror.
>
> Fix it by using the "official" built-in functions instead (see e.g.
> https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html).
> We are not using the *_8 variants in QEMU anyway.
>
> Suggested-by: Christian Schoenebeck <qemu_oss@crudebyte.com>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

This also fixes the failure to set CONFIG_ATOMIC64 for clang (9 and
others) which didn't fail on my box but was certainly missing.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Tested-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée

[PATCH 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Thomas Huth]

Compiler warnings currently go unnoticed in our FreeBSD and macOS builds,
since -Werror is only enabled for Linux and MinGW builds by default. So
let's enable them here now, too.
For macOS, that unfortunately means that we have to disable the vnc-sasl
feature, since this is marked as deprecated in the macOS headers and thus
generates a lot of deprecation warnings.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .cirrus.yml | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index f287d23c5b..bb25c1723b 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -12,7 +12,7 @@ freebsd_12_task:
   script:
     - mkdir build
     - cd build
-    - ../configure || { cat config.log; exit 1; }
+    - ../configure --enable-werror || { cat config.log; exit 1; }
     - gmake -j8
     - gmake V=1 check
 
@@ -24,7 +24,8 @@ macos_task:
   script:
     - mkdir build
     - cd build
-    - ../configure --python=/usr/local/bin/python3 || { cat config.log; exit 1; }
+    - ../configure --python=/usr/local/bin/python3 --disable-vnc-sasl
+        --enable-werror || { cat config.log; exit 1; }
     - gmake -j$(sysctl -n hw.ncpu)
     - gmake check
 
@@ -37,6 +38,7 @@ macos_xcode_task:
   script:
     - mkdir build
     - cd build
-    - ../configure --cc=clang || { cat config.log; exit 1; }
+    - ../configure --cc=clang --disable-vnc-sasl --enable-werror
+        || { cat config.log; exit 1; }
     - gmake -j$(sysctl -n hw.ncpu)
     - gmake check
-- 
2.18.1

Re: [PATCH 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Daniel P. Berrangé]

On Fri, Jul 24, 2020 at 04:32:19PM +0200, Thomas Huth wrote:
> Compiler warnings currently go unnoticed in our FreeBSD and macOS builds,
> since -Werror is only enabled for Linux and MinGW builds by default. So
> let's enable them here now, too.
> For macOS, that unfortunately means that we have to disable the vnc-sasl
> feature, since this is marked as deprecated in the macOS headers and thus
> generates a lot of deprecation warnings.

I wonder if its possible to add

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wdeprecated"

...

#pragma GCC diagnostic pop

to silence just one source file ?


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 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Philippe Mathieu-Daudé]

On 7/24/20 4:46 PM, Daniel P. Berrangé wrote:
> On Fri, Jul 24, 2020 at 04:32:19PM +0200, Thomas Huth wrote:
>> Compiler warnings currently go unnoticed in our FreeBSD and macOS builds,
>> since -Werror is only enabled for Linux and MinGW builds by default. So
>> let's enable them here now, too.
>> For macOS, that unfortunately means that we have to disable the vnc-sasl
>> feature, since this is marked as deprecated in the macOS headers and thus
>> generates a lot of deprecation warnings.
> 
> I wonder if its possible to add
> 
> #pragma GCC diagnostic push
> #pragma GCC diagnostic ignored "-Wdeprecated"
> 
> ...
> 
> #pragma GCC diagnostic pop
> 
> to silence just one source file ?

3 years ago Peter said:

"The awkward part is
 that it has to  be in force at the point where the deprecated
 function is used, not where it's declared. So you can't just wrap
 the #include of the ssl header in pragmas, you'd have to either
 do it at every callsite or else over the whole .c file."

https://www.mail-archive.com/qemu-devel@nongnu.org/msg459264.html

I guess we were expecting the distrib to update the pkg.

> 
> 
> Regards,
> Daniel
> 

Re: [PATCH 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Daniel P. Berrangé]

On Fri, Jul 24, 2020 at 06:46:23PM +0200, Philippe Mathieu-Daudé wrote:
> On 7/24/20 4:46 PM, Daniel P. Berrangé wrote:
> > On Fri, Jul 24, 2020 at 04:32:19PM +0200, Thomas Huth wrote:
> >> Compiler warnings currently go unnoticed in our FreeBSD and macOS builds,
> >> since -Werror is only enabled for Linux and MinGW builds by default. So
> >> let's enable them here now, too.
> >> For macOS, that unfortunately means that we have to disable the vnc-sasl
> >> feature, since this is marked as deprecated in the macOS headers and thus
> >> generates a lot of deprecation warnings.
> > 
> > I wonder if its possible to add
> > 
> > #pragma GCC diagnostic push
> > #pragma GCC diagnostic ignored "-Wdeprecated"
> > 
> > ...
> > 
> > #pragma GCC diagnostic pop
> > 
> > to silence just one source file ?
> 
> 3 years ago Peter said:
> 
> "The awkward part is
>  that it has to  be in force at the point where the deprecated
>  function is used, not where it's declared. So you can't just wrap
>  the #include of the ssl header in pragmas, you'd have to either
>  do it at every callsite or else over the whole .c file."

Nearly all our sasl code is isolated in ui/vnc-auth-sasl.c, so we
can just do pragma push/pop around that entire file.

There's then just two remaining cases in ui/vnc.c which are
easy enough to deal with, or we can move the calls out of vnc.c
into vnc-auth-sasl.c to fully isolate the code

> 
> https://www.mail-archive.com/qemu-devel@nongnu.org/msg459264.html
> 
> I guess we were expecting the distrib to update the pkg.
> 
> > 
> > 
> > Regards,
> > Daniel
> > 
> 

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 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Thomas Huth]

On 24/07/2020 18.49, Daniel P. Berrangé wrote:
> On Fri, Jul 24, 2020 at 06:46:23PM +0200, Philippe Mathieu-Daudé wrote:
>> On 7/24/20 4:46 PM, Daniel P. Berrangé wrote:
>>> On Fri, Jul 24, 2020 at 04:32:19PM +0200, Thomas Huth wrote:
>>>> Compiler warnings currently go unnoticed in our FreeBSD and macOS builds,
>>>> since -Werror is only enabled for Linux and MinGW builds by default. So
>>>> let's enable them here now, too.
>>>> For macOS, that unfortunately means that we have to disable the vnc-sasl
>>>> feature, since this is marked as deprecated in the macOS headers and thus
>>>> generates a lot of deprecation warnings.
>>>
>>> I wonder if its possible to add
>>>
>>> #pragma GCC diagnostic push
>>> #pragma GCC diagnostic ignored "-Wdeprecated"
>>>
>>> ...
>>>
>>> #pragma GCC diagnostic pop
>>>
>>> to silence just one source file ?
>>
>> 3 years ago Peter said:
>>
>> "The awkward part is
>>  that it has to  be in force at the point where the deprecated
>>  function is used, not where it's declared. So you can't just wrap
>>  the #include of the ssl header in pragmas, you'd have to either
>>  do it at every callsite or else over the whole .c file."
> 
> Nearly all our sasl code is isolated in ui/vnc-auth-sasl.c, so we
> can just do pragma push/pop around that entire file.
> 
> There's then just two remaining cases in ui/vnc.c which are
> easy enough to deal with, or we can move the calls out of vnc.c
> into vnc-auth-sasl.c to fully isolate the code

Sounds like it could be done, indeed. But I wonder whether we really
really want to silence the warnings here? We'd hide the information to
the users that sasl is apparently disliked by Apple and might get
removed on macOS in the future.

Maybe we should rather change the "configure" script to disable sasl by
default on macOS unless the user explicitely specified --enable-vnc-sasl ?

 Thomas

Re: [PATCH 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Peter Maydell]

On Mon, 27 Jul 2020 at 06:44, Thomas Huth <thuth@redhat.com> wrote:
> Sounds like it could be done, indeed. But I wonder whether we really
> really want to silence the warnings here? We'd hide the information to
> the users that sasl is apparently disliked by Apple and might get
> removed on macOS in the future.
>
> Maybe we should rather change the "configure" script to disable sasl by
> default on macOS unless the user explicitely specified --enable-vnc-sasl ?

Does the Homebrew packaging of QEMU depend on and use a Homebrew
packaged sasl, or rely on the system sasl ?

thanks
-- PMM

Re: [PATCH 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Thomas Huth]

On 27/07/2020 10.30, Peter Maydell wrote:
> On Mon, 27 Jul 2020 at 06:44, Thomas Huth <thuth@redhat.com> wrote:
>> Sounds like it could be done, indeed. But I wonder whether we really
>> really want to silence the warnings here? We'd hide the information to
>> the users that sasl is apparently disliked by Apple and might get
>> removed on macOS in the future.
>>
>> Maybe we should rather change the "configure" script to disable sasl by
>> default on macOS unless the user explicitely specified --enable-vnc-sasl ?
> 
> Does the Homebrew packaging of QEMU depend on and use a Homebrew
> packaged sasl, or rely on the system sasl ?

I don't have a Mac, but looking at
https://github.com/Homebrew/homebrew-core/blob/master/Formula/qemu.rb it
seems like they don't do anything special with regards to vnc ... so I
guess the answer is "they use system sasl" ?

 Thomas

Re: [PATCH 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Peter Maydell]

On Fri, 24 Jul 2020 at 17:46, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> I guess we were expecting the distrib to update the pkg.

Apple's view is that you shouldn't be using the sasl header
at all but instead their proprietary crypto library APIs, so
I wouldn't expect them to ever ship something without the
deprecation warnings.

thanks
-- PMM

Re: [PATCH 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Christian Schoenebeck]

On Freitag, 24. Juli 2020 18:50:47 CEST Peter Maydell wrote:
> On Fri, 24 Jul 2020 at 17:46, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> > I guess we were expecting the distrib to update the pkg.
> 
> Apple's view is that you shouldn't be using the sasl header
> at all but instead their proprietary crypto library APIs, so
> I wouldn't expect them to ever ship something without the
> deprecation warnings.

AFAIK Apple's reason for this is similar to no longer providing headers for
OpenSSL [1] (or now actually BoringSSL): they cannot guarantee binary
compatibility of these libs beyond individual macOS releases (i.e. without
breaking old clients) and bad things happened [2] in the past for apps which
expected it would.

[1] https://lists.apple.com/archives/macnetworkprog/2015/Jun/msg00025.html
[2] https://lists.andrew.cmu.edu/pipermail/cyrus-sasl/2007-November/001254.html

The common recommendation is: "Ship your macOS app bundled with the preferred
version of these libs."

Best regards,
Christian Schoenebeck

Re: [PATCH 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Daniel P. Berrangé]

On Fri, Jul 24, 2020 at 05:50:47PM +0100, Peter Maydell wrote:
> On Fri, 24 Jul 2020 at 17:46, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> > I guess we were expecting the distrib to update the pkg.
> 
> Apple's view is that you shouldn't be using the sasl header
> at all but instead their proprietary crypto library APIs, so
> I wouldn't expect them to ever ship something without the
> deprecation warnings.

So from pov of our CI, it seems the right answer is to modify the
cirrus.yml to install libsasl2 from homebrew:

  https://formulae.brew.sh/formula-linux/libsasl2

I presume we might need some env variables (CFLAGS/LDFLAGS) to make
configure find this version, in preference to the system version.

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 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Thomas Huth]

On 27/07/2020 12.57, Daniel P. Berrangé wrote:
> On Fri, Jul 24, 2020 at 05:50:47PM +0100, Peter Maydell wrote:
>> On Fri, 24 Jul 2020 at 17:46, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>>> I guess we were expecting the distrib to update the pkg.
>>
>> Apple's view is that you shouldn't be using the sasl header
>> at all but instead their proprietary crypto library APIs, so
>> I wouldn't expect them to ever ship something without the
>> deprecation warnings.
> 
> So from pov of our CI, it seems the right answer is to modify the
> cirrus.yml to install libsasl2 from homebrew:
> 
>   https://formulae.brew.sh/formula-linux/libsasl2

Ok, that one confused me for quite a while, since brew refused to find
it in the macOS jobs on Cirrus-CI. The solution: This is not a macOS
package, but a Linux package! Homebrew is apparently also available for
Linux. There is no libsasl package in homebrew for macOS.

So what to do now? I think introducing a libsasl submodule to QEMU just
for compiling this code on macOS without warnings is also overkill. And
if I got the answers here right, --disable-sasl is also disliked (since
this code likely should still be (compile-)tested on macOS).
So I think I'll go for the same trick as Peter is using for his tests
and use --extra-cflags='-Werror -Wno-error=deprecated-declarations'.

 Thomas

Re: [PATCH 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Daniel P. Berrangé]

On Tue, Jul 28, 2020 at 08:43:29AM +0200, Thomas Huth wrote:
> On 27/07/2020 12.57, Daniel P. Berrangé wrote:
> > On Fri, Jul 24, 2020 at 05:50:47PM +0100, Peter Maydell wrote:
> >> On Fri, 24 Jul 2020 at 17:46, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
> >>> I guess we were expecting the distrib to update the pkg.
> >>
> >> Apple's view is that you shouldn't be using the sasl header
> >> at all but instead their proprietary crypto library APIs, so
> >> I wouldn't expect them to ever ship something without the
> >> deprecation warnings.
> > 
> > So from pov of our CI, it seems the right answer is to modify the
> > cirrus.yml to install libsasl2 from homebrew:
> > 
> >   https://formulae.brew.sh/formula-linux/libsasl2
> 
> Ok, that one confused me for quite a while, since brew refused to find
> it in the macOS jobs on Cirrus-CI. The solution: This is not a macOS
> package, but a Linux package! Homebrew is apparently also available for
> Linux. There is no libsasl package in homebrew for macOS.

Hah, I totally missed that too.

> So what to do now? I think introducing a libsasl submodule to QEMU just
> for compiling this code on macOS without warnings is also overkill. And
> if I got the answers here right, --disable-sasl is also disliked (since
> this code likely should still be (compile-)tested on macOS).
> So I think I'll go for the same trick as Peter is using for his tests
> and use --extra-cflags='-Werror -Wno-error=deprecated-declarations'.

Seems like despite the deprecation, people just continue to use the
system sasl. I guess the extra-cflags  is reasonable for CI, and it
gives end users a warning that they're relyin on a feature of macOS
that's not considered stable....even if it doesn't appear to have
had any actual changes for 10 years.

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 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Peter Maydell]

On Fri, 24 Jul 2020 at 15:32, Thomas Huth <thuth@redhat.com> wrote:
>
> Compiler warnings currently go unnoticed in our FreeBSD and macOS builds,
> since -Werror is only enabled for Linux and MinGW builds by default. So
> let's enable them here now, too.
> For macOS, that unfortunately means that we have to disable the vnc-sasl
> feature, since this is marked as deprecated in the macOS headers and thus
> generates a lot of deprecation warnings.

For my local OSX builds I use
--extra-cflags='-Werror -Wno-error=deprecated-declarations'
to work around the SASL thing.

thanks
-- PMM

Re: [PATCH 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Ed Maste]

On Fri, 24 Jul 2020 at 10:32, Thomas Huth <thuth@redhat.com> wrote:
>
> Compiler warnings currently go unnoticed in our FreeBSD and macOS builds,
> since -Werror is only enabled for Linux and MinGW builds by default. So
> let's enable them here now, too.

Reviewed-by: Ed Maste <emaste@freebsd.org>

for the FreeBSD change; I'm indifferent on which method is used to
address the macos deprecation warnings.

Re: [PATCH 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Christian Schoenebeck]

On Sonntag, 26. Juli 2020 18:14:11 CEST Ed Maste wrote:
> On Fri, 24 Jul 2020 at 10:32, Thomas Huth <thuth@redhat.com> wrote:
> > Compiler warnings currently go unnoticed in our FreeBSD and macOS builds,
> > since -Werror is only enabled for Linux and MinGW builds by default. So
> > let's enable them here now, too.
> 
> Reviewed-by: Ed Maste <emaste@freebsd.org>
> 
> for the FreeBSD change; I'm indifferent on which method is used to
> address the macos deprecation warnings.

Like I pointed out on my response to Peter, it is a bit risky to just blindly 
link against Apple's version of sasl on macOS. Say you compile qemu on a 
certain macOS version, then you run qemu again after an update of macOS 
without recompiling qemu, chances are that you get misbehaviours.

One solution would be bundling the qemu app along with sasl, so qemu would be 
forced to use that bundled sasl version instead of whatever Apple's version of 
sasl is installed on the system.

Another appraoch would be checking the system's sasl version on qemu startup 
by calling either sasl_version() or sasl_version_info() and comparing that 
with the version qemu was compiled with, and erroring out on doubt.

Or obvious last option: simply taking the risk by ignoring this potential 
issue. The SASL headers are still made available by Apple, which suggests that 
they don't break SASL 'too often'.

Your choice. ;-)

Best regards,
Christian Schoenebeck

Re: [PATCH 2/3] cirrus.yml: Compile macOS and FreeBSD with -Werror

[Thomas Huth]

On 26/07/2020 18.14, Ed Maste wrote:
> On Fri, 24 Jul 2020 at 10:32, Thomas Huth <thuth@redhat.com> wrote:
>>
>> Compiler warnings currently go unnoticed in our FreeBSD and macOS builds,
>> since -Werror is only enabled for Linux and MinGW builds by default. So
>> let's enable them here now, too.
> 
> Reviewed-by: Ed Maste <emaste@freebsd.org>
> 
> for the FreeBSD change; I'm indifferent on which method is used to
> address the macos deprecation warnings.

Thanks!

I think I'll split the FreeBSD change from the macOS changes, since
macOS apparently needs some more work... (I'll have a try with Daniel's
suggestion to use libsasl2 from Homebrew there...)

 Thomas

[PATCH 3/3] cirrus.yml: Update the macOS jobs to Catalina

[Thomas Huth]

When looking at the CI jobs on cirrus-ci.com, it seems like the mojave-based
images have been decomissioned a while ago already, since apparently all our
jobs get automatically upgraded to catalina. So let's update our YML script
accordingly to avoid confusion.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .cirrus.yml | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/.cirrus.yml b/.cirrus.yml
index bb25c1723b..b11aac53cb 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -18,7 +18,7 @@ freebsd_12_task:
 
 macos_task:
   osx_instance:
-    image: mojave-base
+    image: catalina-base
   install_script:
     - brew install pkg-config python gnu-sed glib pixman make sdl2 bash
   script:
@@ -32,7 +32,7 @@ macos_task:
 macos_xcode_task:
   osx_instance:
     # this is an alias for the latest Xcode
-    image: mojave-xcode
+    image: catalina-xcode
   install_script:
     - brew install pkg-config gnu-sed glib pixman make sdl2 bash
   script:
-- 
2.18.1

Re: [PATCH 3/3] cirrus.yml: Update the macOS jobs to Catalina

[Ed Maste]

On Fri, 24 Jul 2020 at 10:32, Thomas Huth <thuth@redhat.com> wrote:
>
> When looking at the CI jobs on cirrus-ci.com, it seems like the mojave-based
> images have been decomissioned a while ago already, since apparently all our
> jobs get automatically upgraded to catalina. So let's update our YML script
> accordingly to avoid confusion.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>

Reviewed-by: Ed Maste <emaste@freebsd.org>