[v5,2/4] build: use dependency() instead of find_library()

Message ID 20190122131054.25196-2-bluca@debian.org (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v5,1/4] build: bump minimum Meson version to 0.47.1 |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Luca Boccassi Jan. 22, 2019, 1:10 p.m. UTC
  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 superfluous and
take extra time, and avoid adding the linker flag manually which causes
it to be duplicated.

Signed-off-by: Luca Boccassi <bluca@debian.org>
---
v2: split libbsd change in a separate commit, remove CC to stable
    as a meson bump will be required
v4: update comment in drivers/meson.build to clarify role of
    pkgconfig_extra_libs
v5: remove -lz from drivers/compress/zlib

 drivers/compress/zlib/meson.build  | 1 -
 drivers/crypto/ccp/meson.build     | 1 -
 drivers/crypto/openssl/meson.build | 1 -
 drivers/crypto/qat/meson.build     | 1 -
 drivers/meson.build                | 9 +++++----
 drivers/net/bnx2x/meson.build      | 2 +-
 drivers/net/mlx4/meson.build       | 6 +++---
 drivers/net/mlx5/meson.build       | 6 +++---
 drivers/net/pcap/meson.build       | 5 ++---
 lib/librte_bpf/meson.build         | 4 ++--
 lib/librte_telemetry/meson.build   | 2 +-
 11 files changed, 17 insertions(+), 21 deletions(-)
  

Comments

Bruce Richardson Jan. 22, 2019, 1:46 p.m. UTC | #1
On Tue, Jan 22, 2019 at 01:10:52PM +0000, Luca Boccassi wrote:
> 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 superfluous and
> take extra time, and avoid adding the linker flag manually which causes
> it to be duplicated.
> 
> Signed-off-by: Luca Boccassi <bluca@debian.org>
> ---
> v2: split libbsd change in a separate commit, remove CC to stable
>     as a meson bump will be required
> v4: update comment in drivers/meson.build to clarify role of
>     pkgconfig_extra_libs
> v5: remove -lz from drivers/compress/zlib

How is this removal possible? Does drivers/compress/zlib no longer depend
on libz, or is there a dependency added somewhere else in this patch that
I'm missing? Similar question with the -lcrypto removal?

/Bruce
  
Luca Boccassi Jan. 22, 2019, 2:09 p.m. UTC | #2
On Tue, 2019-01-22 at 13:46 +0000, Bruce Richardson wrote:
> On Tue, Jan 22, 2019 at 01:10:52PM +0000, Luca Boccassi wrote:
> > 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 superfluous
> > and
> > take extra time, and avoid adding the linker flag manually which
> > causes
> > it to be duplicated.
> > 
> > Signed-off-by: Luca Boccassi <bluca@debian.org>
> > ---
> > v2: split libbsd change in a separate commit, remove CC to stable
> >     as a meson bump will be required
> > v4: update comment in drivers/meson.build to clarify role of
> >     pkgconfig_extra_libs
> > v5: remove -lz from drivers/compress/zlib
> 
> How is this removal possible? Does drivers/compress/zlib no longer
> depend
> on libz, or is there a dependency added somewhere else in this patch
> that
> I'm missing? Similar question with the -lcrypto removal?
> 
> /Bruce

Note that the removal is from the pkgconfig_extra_libs list - same for
lcrypto.
They use dependency() already, and the dep is added to the ext_deps, so
the dependency comes in that way (and the right flags are picked via
pkg-config).
  
Bruce Richardson Jan. 22, 2019, 2:24 p.m. UTC | #3
On Tue, Jan 22, 2019 at 02:09:41PM +0000, Luca Boccassi wrote:
> On Tue, 2019-01-22 at 13:46 +0000, Bruce Richardson wrote:
> > On Tue, Jan 22, 2019 at 01:10:52PM +0000, Luca Boccassi wrote:
> > > 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 superfluous
> > > and
> > > take extra time, and avoid adding the linker flag manually which
> > > causes
> > > it to be duplicated.
> > > 
> > > Signed-off-by: Luca Boccassi <bluca@debian.org>
> > > ---
> > > v2: split libbsd change in a separate commit, remove CC to stable
> > >     as a meson bump will be required
> > > v4: update comment in drivers/meson.build to clarify role of
> > >     pkgconfig_extra_libs
> > > v5: remove -lz from drivers/compress/zlib
> > 
> > How is this removal possible? Does drivers/compress/zlib no longer
> > depend
> > on libz, or is there a dependency added somewhere else in this patch
> > that
> > I'm missing? Similar question with the -lcrypto removal?
> > 
> > /Bruce
> 
> Note that the removal is from the pkgconfig_extra_libs list - same for
> lcrypto.
> They use dependency() already, and the dep is added to the ext_deps, so
> the dependency comes in that way (and the right flags are picked via
> pkg-config).
> 

Ok, that is clear now. I didn't realise that they already used dependency
for it.

/Bruce
  
Bruce Richardson Jan. 22, 2019, 2:25 p.m. UTC | #4
On Tue, Jan 22, 2019 at 01:10:52PM +0000, Luca Boccassi wrote:
> 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 superfluous and
> take extra time, and avoid adding the linker flag manually which causes
> it to be duplicated.
> 
> Signed-off-by: Luca Boccassi <bluca@debian.org>
> ---
> v2: split libbsd change in a separate commit, remove CC to stable
>     as a meson bump will be required
> v4: update comment in drivers/meson.build to clarify role of
>     pkgconfig_extra_libs
> v5: remove -lz from drivers/compress/zlib
> 
This is a good idea. Using dependency objects is much better than trying to
track flags manually.

Acked-by: Bruce Richardson <bruce.richardson@intel.com>
  

Patch

diff --git a/drivers/compress/zlib/meson.build b/drivers/compress/zlib/meson.build
index 7748de2df..b036703c7 100644
--- a/drivers/compress/zlib/meson.build
+++ b/drivers/compress/zlib/meson.build
@@ -9,6 +9,5 @@  endif
 deps += 'bus_vdev'
 sources = files('zlib_pmd.c', 'zlib_pmd_ops.c')
 ext_deps += dep
-pkgconfig_extra_libs += '-lz'
 
 allow_experimental_apis = true
diff --git a/drivers/crypto/ccp/meson.build b/drivers/crypto/ccp/meson.build
index e43b00591..915c4c854 100644
--- a/drivers/crypto/ccp/meson.build
+++ b/drivers/crypto/ccp/meson.build
@@ -18,4 +18,3 @@  sources = files('rte_ccp_pmd.c',
 		'ccp_pmd_ops.c')
 
 ext_deps += dep
-pkgconfig_extra_libs += '-lcrypto'
diff --git a/drivers/crypto/openssl/meson.build b/drivers/crypto/openssl/meson.build
index 77a6596d7..d56a32366 100644
--- a/drivers/crypto/openssl/meson.build
+++ b/drivers/crypto/openssl/meson.build
@@ -9,4 +9,3 @@  allow_experimental_apis = true
 deps += 'bus_vdev'
 sources = files('rte_openssl_pmd.c', 'rte_openssl_pmd_ops.c')
 ext_deps += dep
-pkgconfig_extra_libs += '-lcrypto'
diff --git a/drivers/crypto/qat/meson.build b/drivers/crypto/qat/meson.build
index 9cc98d2c2..21f969735 100644
--- a/drivers/crypto/qat/meson.build
+++ b/drivers/crypto/qat/meson.build
@@ -13,6 +13,5 @@  if dep.found()
 			     'qat_sym.c',
 			     'qat_sym_session.c')
 	qat_ext_deps += dep
-	pkgconfig_extra_libs += '-lcrypto'
 	qat_cflags += '-DBUILD_QAT_SYM'
 endif
diff --git a/drivers/meson.build b/drivers/meson.build
index e37d4fe2b..69d0556d3 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -46,10 +46,11 @@  foreach class:driver_classes
 		# set up internal deps. Drivers can append/override as necessary
 		deps = std_deps
 		# ext_deps: Stores external library dependency got
-		# using dependency() or cc.find_library(). For most cases, we
-		# probably also need to specify the "-l" flags in
-		# pkgconfig_extra_libs variable too, so that it can be reflected
-		# in the pkgconfig output for static builds
+		# using dependency() (preferred) or find_library().
+		# For the find_library() case (but not with dependency()) we also
+		# need to specify the "-l" flags in pkgconfig_extra_libs variable
+		# too, so that it can be reflected in the pkgconfig output for
+		# static builds.
 		ext_deps = []
 		pkgconfig_extra_libs = []
 
diff --git a/drivers/net/bnx2x/meson.build b/drivers/net/bnx2x/meson.build
index e3c688869..dd189ffc4 100644
--- a/drivers/net/bnx2x/meson.build
+++ b/drivers/net/bnx2x/meson.build
@@ -1,7 +1,7 @@ 
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2018 Intel Corporation
 
-dep = cc.find_library('z', required: false)
+dep = dependency('zlib', required: false)
 build = dep.found()
 ext_deps += dep
 cflags += '-DZLIB_CONST'
diff --git a/drivers/net/mlx4/meson.build b/drivers/net/mlx4/meson.build
index 4bccd68e0..b4f9672e7 100644
--- a/drivers/net/mlx4/meson.build
+++ b/drivers/net/mlx4/meson.build
@@ -14,9 +14,9 @@  if pmd_dlopen
 	]
 endif
 libs = [
-	cc.find_library('mnl', required:false),
-	cc.find_library('mlx4', required:false),
-	cc.find_library('ibverbs', required:false),
+	dependency('libmnl', required:false),
+	dependency('libmlx4', required:false),
+	dependency('libibverbs', required:false),
 ]
 build = true
 foreach lib:libs
diff --git a/drivers/net/mlx5/meson.build b/drivers/net/mlx5/meson.build
index 4540c455b..0cf2f0873 100644
--- a/drivers/net/mlx5/meson.build
+++ b/drivers/net/mlx5/meson.build
@@ -14,9 +14,9 @@  if pmd_dlopen
 	]
 endif
 libs = [
-	cc.find_library('mnl', required:false),
-	cc.find_library('mlx5', required:false),
-	cc.find_library('ibverbs', required:false),
+	dependency('libmnl', required:false),
+	dependency('libmlx5', required:false),
+	dependency('libibverbs', required:false),
 ]
 build = true
 foreach lib:libs
diff --git a/drivers/net/pcap/meson.build b/drivers/net/pcap/meson.build
index 0c4e0201a..89c9d7a74 100644
--- a/drivers/net/pcap/meson.build
+++ b/drivers/net/pcap/meson.build
@@ -1,12 +1,11 @@ 
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation
 
-pcap_dep = cc.find_library('pcap', required: false)
-if pcap_dep.found() and cc.has_header('pcap.h', dependencies: pcap_dep)
+pcap_dep = dependency('pcap', required: false)
+if pcap_dep.found()
 	build = true
 else
 	build = false
 endif
 sources = files('rte_eth_pcap.c')
 ext_deps += pcap_dep
-pkgconfig_extra_libs += '-lpcap'
diff --git a/lib/librte_bpf/meson.build b/lib/librte_bpf/meson.build
index bc0cd78f9..c3b1f698e 100644
--- a/lib/librte_bpf/meson.build
+++ b/lib/librte_bpf/meson.build
@@ -18,8 +18,8 @@  install_headers = files('bpf_def.h',
 
 deps += ['mbuf', 'net', 'ethdev']
 
-dep = cc.find_library('elf', required: false)
-if dep.found() == true and cc.has_header('libelf.h', dependencies: dep)
+dep = dependency('libelf', required: false)
+if dep.found()
 	sources += files('bpf_load_elf.c')
 	ext_deps += dep
 endif
diff --git a/lib/librte_telemetry/meson.build b/lib/librte_telemetry/meson.build
index 9492f544e..cafb26f08 100644
--- a/lib/librte_telemetry/meson.build
+++ b/lib/librte_telemetry/meson.build
@@ -6,7 +6,7 @@  headers = files('rte_telemetry.h', 'rte_telemetry_internal.h', 'rte_telemetry_pa
 deps += ['metrics', 'ethdev']
 cflags += '-DALLOW_EXPERIMENTAL_API'
 
-jansson = cc.find_library('jansson', required: false)
+jansson = dependency('jansson', required: false)
 if jansson.found()
 	ext_deps += jansson
 	dpdk_app_link_libraries += ['telemetry']