Re: Re: [PATCH v2 4/4] meson: Link with libinotify on FreeBSD

[Ilya Leoshkevich]

On Mon, Feb 05, 2024 at 07:36:32PM +0100, Philippe Mathieu-Daudé wrote:
> Hi Ilya,
> 
> On 5/2/24 19:11, Ilya Leoshkevich wrote:
> > make vm-build-freebsd fails with:
> > 
> >      ld: error: undefined symbol: inotify_init1
> >      >>> referenced by filemonitor-inotify.c:183 
> > (../src/util/filemonitor-inotify.c:183)
> >      >>>               util_filemonitor-inotify.c.o:(qemu_file_monitor_new) 
> > in archive libqemuutil.a
> > 
> > On FreeBSD inotify functions are defined in libinotify.so. Add it to
> > the dependencies.
> > 
> > Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> > ---
> >   meson.build      | 12 +++++++++++-
> >   util/meson.build |  6 +++++-
> >   2 files changed, 16 insertions(+), 2 deletions(-)
> 
> (for some reason your git-diff context is very verbose,
>  making review somehow annoying).

This is because of patch 3. It's essentially the snippet that Daniel
posted, except that patch -p1 applied it at a wrong location! So I
figured I'll send this series with a larger context, but I couldn't
find how to apply this setting to just 1 patch in git send-email.

> > +# libinotify-kqueue
> > +inotify = not_found
> > +if host_os == 'freebsd'
> > +  inotify = cc.find_library('inotify')
> > +endif
> > +
> >   #################
> >   # config-host.h #
> >   #################
> 
> 
> > @@ -2376,61 +2382,62 @@ have_asan_fiber = false
> >   if get_option('sanitizers') and \
> >      not cc.has_function('__sanitizer_start_switch_fiber',
> >                            args: '-fsanitize=address',
> >                            prefix: '#include <sanitizer/asan_interface.h>')
> >     warning('Missing ASAN due to missing fiber annotation interface')
> >     warning('Without code annotation, the report may be inferior.')
> >   else
> >     have_asan_fiber = true
> >   endif
> >   config_host_data.set('CONFIG_ASAN_IFACE_FIBER', have_asan_fiber)
> >   # has_header_symbol
> 
> 
> >   config_host_data.set('CONFIG_INOTIFY',
> >                        cc.has_header_symbol('sys/inotify.h', 
> > 'inotify_init'))
> >   config_host_data.set('CONFIG_INOTIFY1',
> > -                     cc.has_header_symbol('sys/inotify.h', 
> > 'inotify_init1'))
> > +                     cc.has_header_symbol('sys/inotify.h', 
> > 'inotify_init1') and
> > +                     (host_os != 'freebsd' or inotify.found()))
> 
> Maybe we could use the same pattern as 'have_asan_fiber':
> 
>  have_inotify_init1 = cc.has_header_symbol('sys/inotify.h', 'inotify_init1')
>  if have_inotify_init1 and host_os == 'freebsd'
>    have_inotify_init1 = cc.find_library('inotify')
>  endif
>  config_host_data.set('CONFIG_INOTIFY1', have_inotify_init1)

I agree, this looks nicer. I will send a v3.

> I wonder why we don't need the similar library check for the
> inotify_init symbol.

Sounds reasonable, it's just that currently the respective config value
is used only in linux-user, but for completeness it won't hurt.

> 
> Regards,
> 
> Phil.