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-@basen...@.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',
