Re: [PATCH] meson: Propagate gnutls dependency

[Roman Bolshakov]

On Sat, Jan 02, 2021 at 08:43:51PM +0100, Paolo Bonzini wrote:
> On 02/01/21 14:25, Peter Maydell wrote:
> > 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 ?
> > 
> > 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.)
> 
> The gnutls dependency will already propagate from
> 
> if 'CONFIG_GNUTLS' in config_host
>   crypto_ss.add(gnutls)
> endif
> 
> to
> 
> libcrypto = static_library('crypto', crypto_ss.sources() + genh,
>                           dependencies: [crypto_ss.dependencies()], ...)
> crypto = declare_dependency(link_whole: libcrypto,
>                             dependencies: [authz, qom])
> 

Hi Paolo,

I'm sorry I didn't reply earlier. As I showed in an example to Peter
(https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg00085.html):
https://github.com/mesonbuild/meson/commit/ff5dc65ef841857dd306694dff1fb1cd2bf801e4

The approach doesn't propogate dependencies of crypto beyond libcrypto.
i.e. if you specify crypto somewhere else as depedency, it won't pull
CFLAGS needed for gnutls.

> That is, Meson does know that everything that needs crypto needs gnutls (see
> get_dependencies in mesonbuild/build.py if you're curious).
> 

Thanks. I've been thinking to tinker with it (that's why I made the test case).
Sounds like meson has some issues with transitive dependencies.

> I think the issue is that dependencies are listed too late---in the
> declare_dependency rather than the static_library.  Take io/ for example:
> 
> libio = static_library('io', io_ss.sources() + genh,
>                        dependencies: [io_ss.dependencies()],
>                        link_with: libqemuutil,
>                        name_suffix: 'fa',
>                        build_by_default: false)
> io = declare_dependency(link_whole: libio, dependencies: [crypto, qom])
> 
> Listing "crypto" in io's declare_dependency is enough to propagate the
> gnutls LDFLAGS down to the executables, but it does not add the CFLAGS to
> io/ files itself.  So for the io/ files we aren't telling meson that they
> need crypto (and thus in turn gnutls on the include path).
> 
> The fix should be pretty simple and localized to the "Library dependencies"
> section of meson.build.  For the two libraries above, the fixed version
> would look like:
> 
> crypto_ss.add(authz, qom)
> libcrypto = ... # same as above
> crypto = declare_dependency(link_whole: libcrypto)
> 
> io_ss.add(crypto, qom)
> ...
> libio = ... # same as above
> io = declare_dependency(link_whole: libio)
> 
> (Roman, feel free to plunder the above if you want to turn it into a commit
> message, and if it's correct of course).
> 

Unfortunately it doesn't work, even if crypto is added to io_ss. I think
that's the same issue as in shown in test case above. The patch is
below:

diff --git a/hw/nvram/meson.build b/hw/nvram/meson.build
index fd2951a860..1f2ed013b2 100644
--- a/hw/nvram/meson.build
+++ b/hw/nvram/meson.build
@@ -1,6 +1,3 @@
-# QOM interfaces must be available anytime QOM is used.
-qom_ss.add(files('fw_cfg-interface.c'))
-
 softmmu_ss.add(files('fw_cfg.c'))
 softmmu_ss.add(when: 'CONFIG_CHRP_NVRAM', if_true: files('chrp_nvram.c'))
 softmmu_ss.add(when: 'CONFIG_DS1225Y', if_true: files('ds1225y.c'))
diff --git a/io/meson.build b/io/meson.build
index bcd8b1e737..a844271b17 100644
--- a/io/meson.build
+++ b/io/meson.build
@@ -12,4 +12,4 @@ io_ss.add(files(
   'dns-resolver.c',
   'net-listener.c',
   'task.c',
-))
+), crypto)
diff --git a/meson.build b/meson.build
index 372576f82c..c293ee39e4 100644
--- a/meson.build
+++ b/meson.build
@@ -1538,6 +1538,33 @@ libqemuutil = static_library('qemuutil',
 qemuutil = declare_dependency(link_with: libqemuutil,
                               sources: genh + version_res)
 
+# QOM interfaces must be available anytime QOM is used.
+qom_ss.add(files('hw/nvram/fw_cfg-interface.c'))
+qom_ss = qom_ss.apply(config_host, strict: false)
+libqom = static_library('qom', qom_ss.sources() + genh,
+                        dependencies: [qom_ss.dependencies()],
+                        name_suffix: 'fa')
+
+qom = declare_dependency(link_whole: libqom)
+
+authz_ss = authz_ss.apply(config_host, strict: false)
+libauthz = static_library('authz', authz_ss.sources() + genh,
+                          dependencies: [authz_ss.dependencies()],
+                          name_suffix: 'fa',
+                          build_by_default: false)
+
+authz = declare_dependency(link_whole: libauthz,
+                           dependencies: qom)
+
+crypto_ss = crypto_ss.apply(config_host, strict: false)
+libcrypto = static_library('crypto', crypto_ss.sources() + genh,
+                           dependencies: [crypto_ss.dependencies()],
+                           name_suffix: 'fa',
+                           build_by_default: false)
+
+crypto = declare_dependency(link_whole: libcrypto,
+                            dependencies: [authz, qom])
+
 decodetree = generator(find_program('scripts/decodetree.py'),
                        output: 'decode-@BASENAME@.c.inc',
                        arguments: ['@INPUT@', '@EXTRA_ARGS@', '-o', 
'@OUTPUT@'])
@@ -1652,31 +1679,6 @@ qemu_syms = custom_target('qemu.syms', output: 
'qemu.syms',
                              capture: true,
                              command: [undefsym, nm, '@INPUT@'])
 
-qom_ss = qom_ss.apply(config_host, strict: false)
-libqom = static_library('qom', qom_ss.sources() + genh,
-                        dependencies: [qom_ss.dependencies()],
-                        name_suffix: 'fa')
-
-qom = declare_dependency(link_whole: libqom)
-
-authz_ss = authz_ss.apply(config_host, strict: false)
-libauthz = static_library('authz', authz_ss.sources() + genh,
-                          dependencies: [authz_ss.dependencies()],
-                          name_suffix: 'fa',
-                          build_by_default: false)
-
-authz = declare_dependency(link_whole: libauthz,
-                           dependencies: qom)
-
-crypto_ss = crypto_ss.apply(config_host, strict: false)
-libcrypto = static_library('crypto', crypto_ss.sources() + genh,
-                           dependencies: [crypto_ss.dependencies()],
-                           name_suffix: 'fa',
-                           build_by_default: false)
-
-crypto = declare_dependency(link_whole: libcrypto,
-                            dependencies: [authz, qom])
-
 io_ss = io_ss.apply(config_host, strict: false)
 libio = static_library('io', io_ss.sources() + genh,
                        dependencies: [io_ss.dependencies()],
@@ -1684,7 +1686,7 @@ libio = static_library('io', io_ss.sources() + genh,
                        name_suffix: 'fa',
                        build_by_default: false)
 
-io = declare_dependency(link_whole: libio, dependencies: [crypto, qom])
+io = declare_dependency(link_whole: libio, dependencies: [qom])
 
 libmigration = static_library('migration', sources: migration_files + genh,
                               name_suffix: 'fa',