Re: [PATCH] meson: Propagate gnutls dependency

[Roman Bolshakov]

On Sat, Jan 02, 2021 at 01:25:07PM +0000, Peter Maydell wrote:
> On Sat, 2 Jan 2021 at 12:54, Roman Bolshakov <r.bolshakov@yadro.com> wrote:
> >
> > crypto/tlscreds.h includes GnuTLS headers if CONFIG_GNUTLS is set, but
> > GNUTLS_CFLAGS, that describe include path, are not propagated
> > transitively to all users of crypto and build fails if GnuTLS headers
> > reside in non-standard directory (which is a case for homebrew on Apple
> > Silicon).
> >
> > Signed-off-by: Roman Bolshakov <r.bolshakov@yadro.com>
> 
> Ah, this is https://bugs.launchpad.net/qemu/+bug/1909256
> -- thanks for finding a fix.
> 

No problem :)

> > ---
> >  block/meson.build          | 2 +-
> >  io/meson.build             | 2 +-
> >  meson.build                | 5 +++--
> >  storage-daemon/meson.build | 2 +-
> >  tests/meson.build          | 6 +++---
> >  ui/meson.build             | 2 +-
> >  6 files changed, 10 insertions(+), 9 deletions(-)
> 
> > diff --git a/ui/meson.build b/ui/meson.build
> > index 013258a01c..e6655c94a6 100644
> > --- a/ui/meson.build
> > +++ b/ui/meson.build
> > @@ -29,7 +29,7 @@ vnc_ss.add(files(
> >    'vnc-ws.c',
> >    'vnc-jobs.c',
> >  ))
> > -vnc_ss.add(zlib, png, jpeg)
> > +vnc_ss.add(zlib, png, jpeg, gnutls)
> >  vnc_ss.add(when: sasl, if_true: files('vnc-auth-sasl.c'))
> >  softmmu_ss.add_all(when: vnc, if_true: vnc_ss)
> >  softmmu_ss.add(when: vnc, if_false: files('vnc-stubs.c'))
> 
> Question to Paolo -- it seems pretty fragile to have to explicitly
> list "these source files need these extra CFLAGS" in half a dozen
> meson.build files, because it's pretty non-obvious that adding
> eg '#include "block/nbd.h"' to a .c file means that you also
> need to update the meson.build file to say "and now it needs these
> extra CFLAGS". Isn't there some way we can just have the CFLAGS
> added more globally so that if we use gnutls.h directly or
> indirectly from more .c files in future it Just Works ?
> 

Right. I converted a big C++ project to CMake 3 a few years ago and was
able to solve the problem in CMake because it properly supports
transitive dependencies.

In CMake I'd specify that crypto has public dependency on gnutls only
once and then all users of crypto (direct or indirect) would get
required CFLAGS, LDFLAGS and include directories.

I spent a few hours trying to figure out how to achieve the same in
meson (without code duplication and failed miserably). Here's a meson
project test that illustrates the problem of dependency duplication:

https://github.com/mesonbuild/meson/commit/ff5dc65ef841857dd306694dff1fb1cd2bf801e4

The project doesn't build because dependency on foo is not propagated
beyond foobar.

The only way to build it is to specify foo twice - in source set of
foobar and in declared_dependency (i.e. appending "dependencies: [foo]"
to declare_dependency helps).

Unfortunately, the approach doesn't work for meson/qemu because it
introduces duplicate symbols in different static libraries. That's why I
used much more uglier "specify headers where needed all over the code
base".

I'd be happy to hear what's the proper way to fix it.

Thanks,
Roman

> If the build failed for the common Linux case then it would be
> at least more obvious that you needed to update the meson.build
> files. I think it's better to avoid "you need to do this special
> thing that you'll only notice you're missing if you happen to test
> on a somewhat obscure host configuration" where we can.
> 
> (We don't want to link helper binaries etc against gnutls if
> they don't need it, but that's LDFLAGS, not CFLAGS.)
> 
> thanks
> -- PMM