[PATCH] meson.build: Support ncurses on MacOS

[PATCH] meson.build: Support ncurses on MacOS

[Stefan Weil]

MacOS provides header files for curses 5.7 with support
for wide characters, but requires _XOPEN_SOURCE_EXTENDED=1
to activate that.

By default those old header files are used even if there
is a newer Homebrew installation of ncurses 6.2 available.

Change also the old macro definition of NCURSES_WIDECHAR
and set it to 1 like it is done in newer versions of
curses.h when _XOPEN_SOURCE_EXTENDED=1 is defined.

Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 meson.build | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index d2a9ce91f5..3ca8faa264 100644
--- a/meson.build
+++ b/meson.build
@@ -606,7 +606,11 @@ if have_system and not get_option('curses').disabled()
     endif
   endforeach
   msg = get_option('curses').enabled() ? 'curses library not found' : ''
-  curses_compile_args = ['-DNCURSES_WIDECHAR']
+  if host_machine.system() == 'darwin'
+    curses_compile_args = [ '-D_XOPEN_SOURCE_EXTENDED=1']
+  else
+    curses_compile_args = ['-DNCURSES_WIDECHAR=1']
+  endif
   if curses.found()
     if cc.links(curses_test, args: curses_compile_args, dependencies: [curses])
       curses = declare_dependency(compile_args: curses_compile_args, dependencies: [curses])
-- 
2.30.1 (Apple Git-130)

Re: [PATCH] meson.build: Support ncurses on MacOS

[Brad Smith]

This same problem also applies to OpenBSD as we have the same
version of ncurses with support for wide characters. I have a similar
patch in our QEMU port.

On 6/12/2021 4:03 AM, Stefan Weil wrote:
> MacOS provides header files for curses 5.7 with support
> for wide characters, but requires _XOPEN_SOURCE_EXTENDED=1
> to activate that.
>
> By default those old header files are used even if there
> is a newer Homebrew installation of ncurses 6.2 available.
>
> Change also the old macro definition of NCURSES_WIDECHAR
> and set it to 1 like it is done in newer versions of
> curses.h when _XOPEN_SOURCE_EXTENDED=1 is defined.
>
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Stefan Weil <sw@weilnetz.de>
> ---
>   meson.build | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/meson.build b/meson.build
> index d2a9ce91f5..3ca8faa264 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -606,7 +606,11 @@ if have_system and not get_option('curses').disabled()
>       endif
>     endforeach
>     msg = get_option('curses').enabled() ? 'curses library not found' : ''
> -  curses_compile_args = ['-DNCURSES_WIDECHAR']
> +  if host_machine.system() == 'darwin'
> +    curses_compile_args = [ '-D_XOPEN_SOURCE_EXTENDED=1']
> +  else
> +    curses_compile_args = ['-DNCURSES_WIDECHAR=1']
> +  endif
>     if curses.found()
>       if cc.links(curses_test, args: curses_compile_args, dependencies: [curses])
>         curses = declare_dependency(compile_args: curses_compile_args, dependencies: [curses])

Re: [PATCH] meson.build: Support ncurses on MacOS

[Stefan Weil]

Am 13.06.21 um 03:40 schrieb Brad Smith:

> This same problem also applies to OpenBSD as we have the same
> version of ncurses with support for wide characters. I have a similar
> patch in our QEMU port.


Then we should either extend the conditional statement to handle OpenBSD 
as well, or simply define both macros unconditionally:

     # Newer versions of curses use NCURSES_WIDECHAR.
     # Older versions (e. g. on MacOS, OpenBSD) still require 
_XOPEN_SOURCE_EXTENDED.
     curses_compile_args = ['-DNCURSES_WIDECHAR=1', 
'-D_XOPEN_SOURCE_EXTENDED=1']

Defining only _XOPEN_SOURCE_EXTENDED would also work with old and new 
versions, so that's another option.

Stefan

Re: [PATCH] meson.build: Support ncurses on MacOS

[Philippe Mathieu-Daudé]

On 6/13/21 8:33 AM, Stefan Weil wrote:
> Am 13.06.21 um 03:40 schrieb Brad Smith:
> 
>> This same problem also applies to OpenBSD as we have the same
>> version of ncurses with support for wide characters. I have a similar
>> patch in our QEMU port.
> 
> 
> Then we should either extend the conditional statement to handle OpenBSD
> as well, or simply define both macros unconditionally:
> 
>     # Newer versions of curses use NCURSES_WIDECHAR.
>     # Older versions (e. g. on MacOS, OpenBSD) still require
> _XOPEN_SOURCE_EXTENDED.
>     curses_compile_args = ['-DNCURSES_WIDECHAR=1',
> '-D_XOPEN_SOURCE_EXTENDED=1']
> 
> Defining only _XOPEN_SOURCE_EXTENDED would also work with old and new
> versions, so that's another option.

It is simpler to ask Brad to upstream the OpenBSD patch :)

Re: [PATCH] meson.build: Support ncurses on MacOS

[Brad Smith]

On 6/14/2021 1:45 AM, Philippe Mathieu-Daudé wrote:
> On 6/13/21 8:33 AM, Stefan Weil wrote:
>> Am 13.06.21 um 03:40 schrieb Brad Smith:
>>
>>> This same problem also applies to OpenBSD as we have the same
>>> version of ncurses with support for wide characters. I have a similar
>>> patch in our QEMU port.
>>
>> Then we should either extend the conditional statement to handle OpenBSD
>> as well, or simply define both macros unconditionally:
>>
>>      # Newer versions of curses use NCURSES_WIDECHAR.
>>      # Older versions (e. g. on MacOS, OpenBSD) still require
>> _XOPEN_SOURCE_EXTENDED.
>>      curses_compile_args = ['-DNCURSES_WIDECHAR=1',
>> '-D_XOPEN_SOURCE_EXTENDED=1']
>>
>> Defining only _XOPEN_SOURCE_EXTENDED would also work with old and new
>> versions, so that's another option.
> It is simpler to ask Brad to upstream the OpenBSD patch :)

That doesn't answer his question and that's the part that actually matters.

Re: [PATCH] meson.build: Support ncurses on MacOS

[Stefan Weil]

Am 15.06.21 um 03:53 schrieb Brad Smith:

> On 6/14/2021 1:45 AM, Philippe Mathieu-Daudé wrote:
>> On 6/13/21 8:33 AM, Stefan Weil wrote:
>>> Am 13.06.21 um 03:40 schrieb Brad Smith:
>>>
>>>> This same problem also applies to OpenBSD as we have the same
>>>> version of ncurses with support for wide characters. I have a similar
>>>> patch in our QEMU port.
>>>
>>> Then we should either extend the conditional statement to handle 
>>> OpenBSD
>>> as well, or simply define both macros unconditionally:
>>>
>>>      # Newer versions of curses use NCURSES_WIDECHAR.
>>>      # Older versions (e. g. on MacOS, OpenBSD) still require
>>> _XOPEN_SOURCE_EXTENDED.
>>>      curses_compile_args = ['-DNCURSES_WIDECHAR=1',
>>> '-D_XOPEN_SOURCE_EXTENDED=1']
>>>
>>> Defining only _XOPEN_SOURCE_EXTENDED would also work with old and new
>>> versions, so that's another option.
>> It is simpler to ask Brad to upstream the OpenBSD patch :)
>
> That doesn't answer his question and that's the part that actually 
> matters.


The question is still unanswered: which alternative is preferred?

- define only _XOPEN_SOURCE_EXTENDED=1 unconditionally

- define DNCURSES_WIDECHAR=1 and _XOPEN_SOURCE_EXTENDED=1 unconditionally

- define DNCURSES_WIDECHAR=1 and _XOPEN_SOURCE_EXTENDED=1 for MacOS and BSD

All of them should work. We could also start and merge my commit which 
does not fix the issue for BSD but at least fixes it for MacOS.

Peter, you added NCURSES_WIDECHAR in commit b01a4fd3bd7d. Which solution 
would you suggest?

Stefan

Re: [PATCH] meson.build: Support ncurses on MacOS

[Daniel P. Berrangé]

On Tue, Nov 09, 2021 at 06:49:57PM +0100, Stefan Weil wrote:
> Am 15.06.21 um 03:53 schrieb Brad Smith:
> 
> > On 6/14/2021 1:45 AM, Philippe Mathieu-Daudé wrote:
> > > On 6/13/21 8:33 AM, Stefan Weil wrote:
> > > > Am 13.06.21 um 03:40 schrieb Brad Smith:
> > > > 
> > > > > This same problem also applies to OpenBSD as we have the same
> > > > > version of ncurses with support for wide characters. I have a similar
> > > > > patch in our QEMU port.
> > > > 
> > > > Then we should either extend the conditional statement to handle
> > > > OpenBSD
> > > > as well, or simply define both macros unconditionally:
> > > > 
> > > >      # Newer versions of curses use NCURSES_WIDECHAR.
> > > >      # Older versions (e. g. on MacOS, OpenBSD) still require
> > > > _XOPEN_SOURCE_EXTENDED.
> > > >      curses_compile_args = ['-DNCURSES_WIDECHAR=1',
> > > > '-D_XOPEN_SOURCE_EXTENDED=1']
> > > > 
> > > > Defining only _XOPEN_SOURCE_EXTENDED would also work with old and new
> > > > versions, so that's another option.
> > > It is simpler to ask Brad to upstream the OpenBSD patch :)
> > 
> > That doesn't answer his question and that's the part that actually
> > matters.
> 
> 
> The question is still unanswered: which alternative is preferred?
> 
> - define only _XOPEN_SOURCE_EXTENDED=1 unconditionally
> 
> - define DNCURSES_WIDECHAR=1 and _XOPEN_SOURCE_EXTENDED=1 unconditionally
> 
> - define DNCURSES_WIDECHAR=1 and _XOPEN_SOURCE_EXTENDED=1 for MacOS and BSD
> 
> All of them should work. We could also start and merge my commit which does
> not fix the issue for BSD but at least fixes it for MacOS.

Ignoring ncurses for a minute

 * QEMU sets _GNU_SOURCE=1
 * With GLibc on _GNU_SOURCE forces _XOPEN_SOURCE=700
 * _XOPEN_SOURCE_EXTENDED=1 is implied by _XOPEN_SOURCE >= 500

IOW for Linux/GLibC, none of this matters, we always get wide char
support via _GNU_SOURCE.

Setting NCURSES_WIDECHAR was added in

  commit b01a4fd3bd7d6f2ebd9eeba9cb6502d423c3bc85
  Author: Peter Maydell <peter.maydell@linaro.org>
  Date:   Fri Jun 2 15:35:38 2017 +0100

    configure: Define NCURSES_WIDECHAR if we're using curses

for the benefit for benefit of non-Linux / non_GLibC platforms
where _GNU_SOURCE doesn't take affect

Note it explicitly rejected the idea of using _XOPEN_SOURCE

[quote]
    We can't fix this by defining _XOPEN_SOURCE ourselves, because
    that also means "and don't provide any functions that aren't in
    that standard", and not all libcs provide any way to override
    that to also get the non-standard functions. In particular
    FreeBSD has no such mechanism, and OSX's _DARWIN_C_SOURCE
    doesn't reenable everything (for instance getpagesize()
    is still not prototyped if _DARWIN_C_SOURCE and _XOPEN_SOURCE
    are both defined).
[/quote]

Based on Stefan / Brad experiance patching in _XOPEN_SOURCE_EXTENDED=1
it seems it might not be too terrible in its effects, but perhaps we're
simply not noticing a silent bad effect.

I also don't like adding a global flag like _XOPEN_SOURCE_EXTENDED=1
in response to a feature that can be turned on/off via a configure
arg --without-curses.

Overall, I don't think we should be setting _XOPEN_SOURCE_EXTENDED=1
globally in CFLAGS in response to curses.

We could put it in any .c file that includes curses.h ie

  #if defined(__APPLE__) || defined(__OpenBSD__)
  #define _XOPEN_SOURCE_EXTENDED=1
  #endif
  #include <curses.h>

so that we isolate the effects of the global flag from the rest
of the QEMU codebase. The meson.build test would also need to
set this in its test program source.

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] meson.build: Support ncurses on MacOS

[Stefan Weil]

MacOS provides header files for curses 5.7 with support
for wide characters, but requires _XOPEN_SOURCE_EXTENDED=1
to activate that.

By default those old header files are used even if there
is a newer Homebrew installation of ncurses 6.2 available.

Change also the old macro definition of NCURSES_WIDECHAR
and set it to 1 like it is done in newer versions of
curses.h when _XOPEN_SOURCE_EXTENDED=1 is defined.

Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
Signed-off-by: Stefan Weil <sw@weilnetz.de>
---
 meson.build | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index d2a9ce91f5..3ca8faa264 100644
--- a/meson.build
+++ b/meson.build
@@ -606,7 +606,11 @@ if have_system and not get_option('curses').disabled()
     endif
   endforeach
   msg = get_option('curses').enabled() ? 'curses library not found' : ''
-  curses_compile_args = ['-DNCURSES_WIDECHAR']
+  if host_machine.system() == 'darwin'
+    curses_compile_args = [ '-D_XOPEN_SOURCE_EXTENDED=1']
+  else
+    curses_compile_args = ['-DNCURSES_WIDECHAR=1']
+  endif
   if curses.found()
     if cc.links(curses_test, args: curses_compile_args, dependencies: [curses])
       curses = declare_dependency(compile_args: curses_compile_args, dependencies: [curses])
-- 
2.30.1 (Apple Git-130)