Thread (88 messages) 88 messages, 4 authors, 2019-03-01

Re: [dpdk-stable] [PATCH 2/2] build: use dependency() instead of find_library()

From: Luca Boccassi <hidden>
Date: 2019-01-07 16:39:34

On Mon, 2019-01-07 at 14:28 +0000, Bruce Richardson wrote:
On Thu, Jan 03, 2019 at 06:57:25PM +0100, Luca Boccassi wrote:
quoted
Whenever possible (if the library ships a pkg-config file) use
meson's
dependency() function to look for it, as it will automatically add
it
to the Requires.private list if needed, to allow for static builds
to
succeed for reverse dependencies of DPDK. Otherwise the recursive
dependencies are not parsed, and users doing static builds have to
resolve them manually by themselves.
When using this API avoid additional checks that are superfluos and
take extra time, and avoid adding the linker flag manually which
causes
it to be duplicated.

An internal checker has been added to Meson 0.42 to detect libpcap,
which ships a custom tool rather than a pkg-config file, so bump
the
minimum Meson version from 0.41 to 0.42.
If we are going to bump the version, I think we should bump it
further to
e.g. 0.46 or 0.47 unless there is anyone who still wants an earlier
version. That should get rid of a number of the meson version
warnings we
see on each run.
The distros situation, as far as I can see:

Debian 10 0.49
Ubuntu 18.04 LTS 0.45
Ubuntu 18.10 0.47
Fedora 27 0.43
Fedora 28 0.45
SUSE Leap 15 0.46
FreeBSD 10 0.46
CentOS 7 0.47

So by bumping to 0.47, required to fix the bug below, we'd leave behind
a fair few distros.

Now for me it's fine to go to 0.47 - but I think the CC to stable
should then be removed.
quoted
For libbsd, which is checked in a top level file and used to be
added
to the global linker flags array, add it to the ext_deps array of
all top level meson files (app, test, lib, examples, drivers). The
most correct change would be to let each individual
library/driver/app
depend on it individually if they use symbols from it, but it would
diverge from the legacy Makefile's behaviour and make life a bit
more
difficult for contributors.
It shouldn't be necessary to add libbsd as a dependency for
everything. I
think just adding it as a dependency of EAL should work fine. 
Won't that mean that the shared libraries other than EAL will have
undefined references?
Now, in practice it would be fine because I'm pretty sure none of them
can and would actually be used without EAL, so when linking executables
everything will be fine, but for example the Debian build tools will at
the very least print warnings if a shared library links
However, in
conjunction with meson version checks, I believe this was done this
way
originally because of a meson bug which caused recursive dependencies
for
things like this to get duplicated many times in the build.ninja
file.

https://github.com/mesonbuild/meson/issues/2150

If we take the approach of adding bsd explicitly using dependency
object
our minimum version needs to have the fix for this bug included.
Ah that's not nice. Just verified, and it happens with dependency() as
well as find_library(). It was fixed in 0.47.1.
quoted
Fixes: a25a650be5f0 ("build: add infrastructure for meson and ninja
builds")
Cc: stable@dpdk.org

Signed-off-by: Luca Boccassi <redacted>
---

Bruce, dependency() by default tries pkg-config first, then cmake,
then
the internal project-specific finders (like pcap). If you think
it's
worth it I can add fallbacks in case a system, for whatever reason,
does not install a pc file despite the upstream project providing
one.
It would add more clutter and more verbosity, but it would not
cause
other issues.
I'd prefer to keep it without that for now. If the lack of .pc files
causes
issues we can revisit.

/Bruce
Ok.

-- 
Kind regards,
Luca Boccassi
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help