* [dpdk-stable] [PATCH 1/2] build: use static deps of libs for pkg-config libs.private @ 2019-01-03 17:57 Luca Boccassi 2019-01-03 17:57 ` [dpdk-stable] [PATCH 2/2] build: use dependency() instead of find_library() Luca Boccassi 2019-01-11 12:38 ` [dpdk-stable] [PATCH v2 1/3] build: use static deps of libs for pkg-config libs.private Luca Boccassi 0 siblings, 2 replies; 15+ messages in thread From: Luca Boccassi @ 2019-01-03 17:57 UTC (permalink / raw) To: dev; +Cc: bruce.richardson, Luca Boccassi, stable Dependencies of the RTE libraries were not being added to the Requires.private field of the pc file since the variable used for dynamic linking was passed to the related field of pkg.generate. Use the static one so that dependencies are included. Fixes: 57ae0ec62620 ("build: add dependency on telemetry to apps with meson") Cc: stable@dpdk.org Signed-off-by: Luca Boccassi <bluca@debian.org> --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 7cee3c94a..617e88589 100644 --- a/meson.build +++ b/meson.build @@ -81,7 +81,7 @@ pkg.generate(name: meson.project_name(), filebase: 'lib' + meson.project_name().to_lower(), version: meson.project_version(), libraries: dpdk_libraries, - libraries_private: dpdk_drivers + dpdk_libraries + + libraries_private: dpdk_drivers + dpdk_static_libraries + ['-Wl,-Bdynamic'] + dpdk_extra_ldflags, description: 'The Data Plane Development Kit (DPDK)', subdirs: [get_option('include_subdir_arch'), '.'], -- 2.19.2 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [dpdk-stable] [PATCH 2/2] build: use dependency() instead of find_library() 2019-01-03 17:57 [dpdk-stable] [PATCH 1/2] build: use static deps of libs for pkg-config libs.private Luca Boccassi @ 2019-01-03 17:57 ` Luca Boccassi 2019-01-07 14:28 ` Bruce Richardson 2019-01-11 12:38 ` [dpdk-stable] [PATCH v2 1/3] build: use static deps of libs for pkg-config libs.private Luca Boccassi 1 sibling, 1 reply; 15+ messages in thread From: Luca Boccassi @ 2019-01-03 17:57 UTC (permalink / raw) To: dev; +Cc: bruce.richardson, Luca Boccassi, stable 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. 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. Fixes: a25a650be5f0 ("build: add infrastructure for meson and ninja builds") Cc: stable@dpdk.org Signed-off-by: Luca Boccassi <bluca@debian.org> --- 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. app/meson.build | 2 +- config/meson.build | 10 +++++----- drivers/crypto/ccp/meson.build | 1 - drivers/crypto/openssl/meson.build | 1 - drivers/crypto/qat/meson.build | 1 - drivers/meson.build | 4 ++-- drivers/net/bnx2x/meson.build | 2 +- drivers/net/mlx4/meson.build | 6 +++--- drivers/net/mlx5/meson.build | 6 +++--- drivers/net/pcap/meson.build | 5 ++--- examples/meson.build | 2 +- lib/librte_bpf/meson.build | 4 ++-- lib/librte_telemetry/meson.build | 2 +- lib/meson.build | 2 +- meson.build | 2 +- test/test/meson.build | 1 + 16 files changed, 24 insertions(+), 27 deletions(-) diff --git a/app/meson.build b/app/meson.build index 47a2a8615..e31386f1a 100644 --- a/app/meson.build +++ b/app/meson.build @@ -29,7 +29,7 @@ foreach app:apps # use "deps" for internal DPDK dependencies, and "ext_deps" for # external package/library requirements - ext_deps = [] + ext_deps = [libbsd] deps = dpdk_app_link_libraries subdir(name) diff --git a/config/meson.build b/config/meson.build index db32499b3..e1af468ee 100644 --- a/config/meson.build +++ b/config/meson.build @@ -74,11 +74,11 @@ if numa_dep.found() and cc.has_header('numaif.h') endif # check for strlcpy -if host_machine.system() == 'linux' and cc.find_library('bsd', - required: false).found() and cc.has_header('bsd/string.h') - dpdk_conf.set('RTE_USE_LIBBSD', 1) - add_project_link_arguments('-lbsd', language: 'c') - dpdk_extra_ldflags += '-lbsd' +if host_machine.system() == 'linux' + libbsd = dependency('libbsd', required: false) + if libbsd.found() + dpdk_conf.set('RTE_USE_LIBBSD', 1) + endif endif # add -include rte_config to cflags 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 c2a0dd8ba..80e5e8835 100644 --- a/drivers/crypto/openssl/meson.build +++ b/drivers/crypto/openssl/meson.build @@ -8,4 +8,3 @@ endif 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 c3c66bbc0..f442f9719 100644 --- a/drivers/meson.build +++ b/drivers/meson.build @@ -46,11 +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 + # using dependency(). 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 - ext_deps = [] + ext_deps = [libbsd] pkgconfig_extra_libs = [] # pull in driver directory which should assign to each of the above 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 7de571e2a..4ba4e93b6 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 28938db0f..7ff3660fa 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/examples/meson.build b/examples/meson.build index af81c762e..881e2da33 100644 --- a/examples/meson.build +++ b/examples/meson.build @@ -33,7 +33,7 @@ foreach example: examples allow_experimental_apis = false cflags = default_cflags - ext_deps = [execinfo] + ext_deps = [execinfo libbsd] includes = [include_directories(example)] deps = ['eal', 'mempool', 'net', 'mbuf', 'ethdev', 'cmdline'] subdir(example) 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'] diff --git a/lib/meson.build b/lib/meson.build index a2dd52e17..b2a18f488 100644 --- a/lib/meson.build +++ b/lib/meson.build @@ -51,7 +51,7 @@ foreach l:libraries # use "deps" for internal DPDK dependencies, and "ext_deps" for # external package/library requirements - ext_deps = [] + ext_deps = [libbsd] deps = [] # eal is standard dependency once built if dpdk_conf.has('RTE_LIBRTE_EAL') diff --git a/meson.build b/meson.build index 617e88589..f87dc235f 100644 --- a/meson.build +++ b/meson.build @@ -5,7 +5,7 @@ project('DPDK', 'C', version: '19.02.0-rc1', license: 'BSD', default_options: ['buildtype=release', 'default_library=static'], - meson_version: '>= 0.41' + meson_version: '>= 0.42' ) # set up some global vars for compiler, platform, configuration, etc. diff --git a/test/test/meson.build b/test/test/meson.build index 5a4816fed..886c29fb4 100644 --- a/test/test/meson.build +++ b/test/test/meson.build @@ -279,6 +279,7 @@ foreach d:test_deps test_dep_objs += get_variable(def_lib + '_rte_' + d) endforeach test_dep_objs += cc.find_library('execinfo', required: false) +test_dep_objs += libbsd link_libs = [] if get_option('default_library') == 'static' -- 2.19.2 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-stable] [PATCH 2/2] build: use dependency() instead of find_library() 2019-01-03 17:57 ` [dpdk-stable] [PATCH 2/2] build: use dependency() instead of find_library() Luca Boccassi @ 2019-01-07 14:28 ` Bruce Richardson 2019-01-07 16:39 ` Luca Boccassi 0 siblings, 1 reply; 15+ messages in thread From: Bruce Richardson @ 2019-01-07 14:28 UTC (permalink / raw) To: Luca Boccassi; +Cc: dev, stable On Thu, Jan 03, 2019 at 06:57:25PM +0100, 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 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. > > 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. 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. > > Fixes: a25a650be5f0 ("build: add infrastructure for meson and ninja builds") > Cc: stable@dpdk.org > > Signed-off-by: Luca Boccassi <bluca@debian.org> > --- > > 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 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-stable] [PATCH 2/2] build: use dependency() instead of find_library() 2019-01-07 14:28 ` Bruce Richardson @ 2019-01-07 16:39 ` Luca Boccassi 2019-01-07 16:55 ` Bruce Richardson 0 siblings, 1 reply; 15+ messages in thread From: Luca Boccassi @ 2019-01-07 16:39 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev, stable 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: > > 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. > > > > 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. > > > > Fixes: a25a650be5f0 ("build: add infrastructure for meson and ninja > > builds") > > Cc: stable@dpdk.org > > > > Signed-off-by: Luca Boccassi <bluca@debian.org> > > --- > > > > 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 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-stable] [PATCH 2/2] build: use dependency() instead of find_library() 2019-01-07 16:39 ` Luca Boccassi @ 2019-01-07 16:55 ` Bruce Richardson 2019-01-07 17:03 ` [dpdk-stable] [dpdk-techboard] " Thomas Monjalon 2019-01-11 11:10 ` [dpdk-stable] [dpdk-dev] " Luca Boccassi 0 siblings, 2 replies; 15+ messages in thread From: Bruce Richardson @ 2019-01-07 16:55 UTC (permalink / raw) To: Luca Boccassi; +Cc: dev, stable, techboard On Mon, Jan 07, 2019 at 04:39:34PM +0000, Luca Boccassi wrote: > 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: > > > 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. > Looking at the list probably 0.45 is safe for now. The follow-up question is whether expecting people to get updated meson using pip is reasonable. If it is, then I'm all in favour of bumping min to 0.47.1 and taking in your changes below. > > > > > > 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? Should not happen. AFAIK when you link against a library in meson it will also link against any of that libraries dependencies too. For shared libraries meson always disallowed undefined references in the linker commandline. [To have libs with undefined refs, e.g. plugins, you need to use "shared_module" rather than "shared_library" command]. > 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. > Yep, it was a right royal pain when I was doing the original work. Now that there is a fix in, we can do cleanups like you suggest if we are prepared to bump our minimum version. I'll refer back to the key question here: "Is it reasonable to ask users compiling DPDK to pull meson from pip rather than using the distro built-in version?" [Adding techboard on CC, in the hopes they might have some thoughts] If it is ok for most folks, and personally I don't think it's a big deal, then that gives us a faster path forward. If not, we raise the minimum more slowly, and keep the existing way of managing the dependencies for a while longer. Worst case, I'd still hope by 19.11 LTS for us to have minimum 0.47.1 to have the fix in question. /Bruce ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-stable] [dpdk-techboard] [PATCH 2/2] build: use dependency() instead of find_library() 2019-01-07 16:55 ` Bruce Richardson @ 2019-01-07 17:03 ` Thomas Monjalon 2019-01-07 17:45 ` Thomas Monjalon 2019-01-11 11:10 ` [dpdk-stable] [dpdk-dev] " Luca Boccassi 1 sibling, 1 reply; 15+ messages in thread From: Thomas Monjalon @ 2019-01-07 17:03 UTC (permalink / raw) To: Bruce Richardson; +Cc: techboard, Luca Boccassi, dev, stable 07/01/2019 17:55, Bruce Richardson: > On Mon, Jan 07, 2019 at 04:39:34PM +0000, Luca Boccassi wrote: > > On Mon, 2019-01-07 at 14:28 +0000, Bruce Richardson wrote: > > > 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. > > > > Yep, it was a right royal pain when I was doing the original work. Now that > there is a fix in, we can do cleanups like you suggest if we are prepared > to bump our minimum version. > > I'll refer back to the key question here: > "Is it reasonable to ask users compiling DPDK to pull meson from pip rather > than using the distro built-in version?" > [Adding techboard on CC, in the hopes they might have some thoughts] > > If it is ok for most folks, and personally I don't think it's a big deal, > then that gives us a faster path forward. If not, we raise the minimum more > slowly, and keep the existing way of managing the dependencies for a while > longer. Worst case, I'd still hope by 19.11 LTS for us to have minimum > 0.47.1 to have the fix in question. Please, could you describe what are the meson versions in major distros? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-stable] [dpdk-techboard] [PATCH 2/2] build: use dependency() instead of find_library() 2019-01-07 17:03 ` [dpdk-stable] [dpdk-techboard] " Thomas Monjalon @ 2019-01-07 17:45 ` Thomas Monjalon 0 siblings, 0 replies; 15+ messages in thread From: Thomas Monjalon @ 2019-01-07 17:45 UTC (permalink / raw) To: Bruce Richardson, Luca Boccassi; +Cc: stable, techboard, dev 07/01/2019 18:03, Thomas Monjalon: > 07/01/2019 17:55, Bruce Richardson: > > On Mon, Jan 07, 2019 at 04:39:34PM +0000, Luca Boccassi wrote: > > > On Mon, 2019-01-07 at 14:28 +0000, Bruce Richardson wrote: > > > > 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. > > > > > > > Yep, it was a right royal pain when I was doing the original work. Now that > > there is a fix in, we can do cleanups like you suggest if we are prepared > > to bump our minimum version. > > > > I'll refer back to the key question here: > > "Is it reasonable to ask users compiling DPDK to pull meson from pip rather > > than using the distro built-in version?" > > [Adding techboard on CC, in the hopes they might have some thoughts] > > > > If it is ok for most folks, and personally I don't think it's a big deal, > > then that gives us a faster path forward. If not, we raise the minimum more > > slowly, and keep the existing way of managing the dependencies for a while > > longer. Worst case, I'd still hope by 19.11 LTS for us to have minimum > > 0.47.1 to have the fix in question. > > Please, could you describe what are the meson versions in major distros? It was already listed by Luca in this thread (thanks Bruce). I looks like latest Debian/Ubuntu and Fedora have meson 0.47 or higher. I vote for bumping to meson 0.47. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 2/2] build: use dependency() instead of find_library() 2019-01-07 16:55 ` Bruce Richardson 2019-01-07 17:03 ` [dpdk-stable] [dpdk-techboard] " Thomas Monjalon @ 2019-01-11 11:10 ` Luca Boccassi 2019-01-11 11:52 ` Bruce Richardson 1 sibling, 1 reply; 15+ messages in thread From: Luca Boccassi @ 2019-01-11 11:10 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev, stable On Mon, 2019-01-07 at 16:55 +0000, Bruce Richardson wrote: > On Mon, Jan 07, 2019 at 04:39:34PM +0000, Luca Boccassi wrote: > > 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: > > > > 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? > > Should not happen. AFAIK when you link against a library in meson it > will > also link against any of that libraries dependencies too. For shared > libraries meson always disallowed undefined references in the linker > commandline. [To have libs with undefined refs, e.g. plugins, you > need to > use "shared_module" rather than "shared_library" command]. Looked at this again, and rte_cmdline is using strlcpy as well, and it's built before rte_eal, so it fails: lib/76b5a35@@rte_cmdline@sta/librte_cmdline_cmdline_parse.c.o: In function `cmdline_complete': cmdline_parse.c:(.text+0x861): undefined reference to `strlcpy' Adding it to ext_deps in both rte_cmdline and rte_eal works. Is that an acceptable compromise? -- Kind regards, Luca Boccassi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 2/2] build: use dependency() instead of find_library() 2019-01-11 11:10 ` [dpdk-stable] [dpdk-dev] " Luca Boccassi @ 2019-01-11 11:52 ` Bruce Richardson 2019-01-11 12:39 ` Luca Boccassi 0 siblings, 1 reply; 15+ messages in thread From: Bruce Richardson @ 2019-01-11 11:52 UTC (permalink / raw) To: Luca Boccassi; +Cc: dev, stable On Fri, Jan 11, 2019 at 11:10:28AM +0000, Luca Boccassi wrote: > On Mon, 2019-01-07 at 16:55 +0000, Bruce Richardson wrote: > > On Mon, Jan 07, 2019 at 04:39:34PM +0000, Luca Boccassi wrote: > > > 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: > > > > > 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? > > > > Should not happen. AFAIK when you link against a library in meson it > > will > > also link against any of that libraries dependencies too. For shared > > libraries meson always disallowed undefined references in the linker > > commandline. [To have libs with undefined refs, e.g. plugins, you > > need to > > use "shared_module" rather than "shared_library" command]. > > Looked at this again, and rte_cmdline is using strlcpy as well, and > it's built before rte_eal, so it fails: > > lib/76b5a35@@rte_cmdline@sta/librte_cmdline_cmdline_parse.c.o: In function `cmdline_complete': > cmdline_parse.c:(.text+0x861): undefined reference to `strlcpy' > > Adding it to ext_deps in both rte_cmdline and rte_eal works. Is that an acceptable compromise? > Sure. If eal has a dependency on cmdline, you probably don't need to add it as an external dependency to EAL too, but it doesn't really hurt to do so. /Bruce ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 2/2] build: use dependency() instead of find_library() 2019-01-11 11:52 ` Bruce Richardson @ 2019-01-11 12:39 ` Luca Boccassi 2019-01-11 14:24 ` Bruce Richardson 0 siblings, 1 reply; 15+ messages in thread From: Luca Boccassi @ 2019-01-11 12:39 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev, stable On Fri, 2019-01-11 at 11:52 +0000, Bruce Richardson wrote: > On Fri, Jan 11, 2019 at 11:10:28AM +0000, Luca Boccassi wrote: > > On Mon, 2019-01-07 at 16:55 +0000, Bruce Richardson wrote: > > > On Mon, Jan 07, 2019 at 04:39:34PM +0000, Luca Boccassi wrote: > > > > 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: > > > > > > 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? > > > > > > Should not happen. AFAIK when you link against a library in meson > > > it > > > will > > > also link against any of that libraries dependencies too. For > > > shared > > > libraries meson always disallowed undefined references in the > > > linker > > > commandline. [To have libs with undefined refs, e.g. plugins, you > > > need to > > > use "shared_module" rather than "shared_library" command]. > > > > Looked at this again, and rte_cmdline is using strlcpy as well, and > > it's built before rte_eal, so it fails: > > > > lib/76b5a35@@rte_cmdline@sta/librte_cmdline_cmdline_parse.c.o: In > > function `cmdline_complete': > > cmdline_parse.c:(.text+0x861): undefined reference to `strlcpy' > > > > Adding it to ext_deps in both rte_cmdline and rte_eal works. Is > > that an acceptable compromise? > > > > Sure. If eal has a dependency on cmdline, you probably don't need to > add it > as an external dependency to EAL too, but it doesn't really hurt to > do so. > > /Bruce eal does not depend on cmdline, so added in both in v2. I also split the libbsd change and meson bump to 0.47.1 in a separate commit in the series. -- Kind regards, Luca Boccassi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 2/2] build: use dependency() instead of find_library() 2019-01-11 12:39 ` Luca Boccassi @ 2019-01-11 14:24 ` Bruce Richardson 2019-01-11 14:56 ` Luca Boccassi 0 siblings, 1 reply; 15+ messages in thread From: Bruce Richardson @ 2019-01-11 14:24 UTC (permalink / raw) To: Luca Boccassi; +Cc: dev, stable On Fri, Jan 11, 2019 at 12:39:23PM +0000, Luca Boccassi wrote: > On Fri, 2019-01-11 at 11:52 +0000, Bruce Richardson wrote: > > On Fri, Jan 11, 2019 at 11:10:28AM +0000, Luca Boccassi wrote: > > > On Mon, 2019-01-07 at 16:55 +0000, Bruce Richardson wrote: > > > > On Mon, Jan 07, 2019 at 04:39:34PM +0000, Luca Boccassi wrote: > > > > > 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: > > > > > > > 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? > > > > > > > > Should not happen. AFAIK when you link against a library in meson > > > > it > > > > will > > > > also link against any of that libraries dependencies too. For > > > > shared > > > > libraries meson always disallowed undefined references in the > > > > linker > > > > commandline. [To have libs with undefined refs, e.g. plugins, you > > > > need to > > > > use "shared_module" rather than "shared_library" command]. > > > > > > Looked at this again, and rte_cmdline is using strlcpy as well, and > > > it's built before rte_eal, so it fails: > > > > > > lib/76b5a35@@rte_cmdline@sta/librte_cmdline_cmdline_parse.c.o: In > > > function `cmdline_complete': > > > cmdline_parse.c:(.text+0x861): undefined reference to `strlcpy' > > > > > > Adding it to ext_deps in both rte_cmdline and rte_eal works. Is > > > that an acceptable compromise? > > > > > > > Sure. If eal has a dependency on cmdline, you probably don't need to > > add it > > as an external dependency to EAL too, but it doesn't really hurt to > > do so. > > > > /Bruce > > eal does not depend on cmdline, so added in both in v2. I also split > the libbsd change and meson bump to 0.47.1 in a separate commit in the > series. > > -- If there is no dependency, why is it being built first? ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 2/2] build: use dependency() instead of find_library() 2019-01-11 14:24 ` Bruce Richardson @ 2019-01-11 14:56 ` Luca Boccassi 2019-01-11 15:49 ` Bruce Richardson 0 siblings, 1 reply; 15+ messages in thread From: Luca Boccassi @ 2019-01-11 14:56 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev, stable On Fri, 2019-01-11 at 14:24 +0000, Bruce Richardson wrote: > On Fri, Jan 11, 2019 at 12:39:23PM +0000, Luca Boccassi wrote: > > On Fri, 2019-01-11 at 11:52 +0000, Bruce Richardson wrote: > > > On Fri, Jan 11, 2019 at 11:10:28AM +0000, Luca Boccassi wrote: > > > > On Mon, 2019-01-07 at 16:55 +0000, Bruce Richardson wrote: > > > > > On Mon, Jan 07, 2019 at 04:39:34PM +0000, Luca Boccassi > > > > > wrote: > > > > > > 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: > > > > > > > > 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? > > > > > > > > > > Should not happen. AFAIK when you link against a library in > > > > > meson > > > > > it > > > > > will > > > > > also link against any of that libraries dependencies too. For > > > > > shared > > > > > libraries meson always disallowed undefined references in the > > > > > linker > > > > > commandline. [To have libs with undefined refs, e.g. plugins, > > > > > you > > > > > need to > > > > > use "shared_module" rather than "shared_library" command]. > > > > > > > > Looked at this again, and rte_cmdline is using strlcpy as well, > > > > and > > > > it's built before rte_eal, so it fails: > > > > > > > > lib/76b5a35@@rte_cmdline@sta/librte_cmdline_cmdline_parse.c.o: > > > > In > > > > function `cmdline_complete': > > > > cmdline_parse.c:(.text+0x861): undefined reference to `strlcpy' > > > > > > > > Adding it to ext_deps in both rte_cmdline and rte_eal works. Is > > > > that an acceptable compromise? > > > > > > > > > > Sure. If eal has a dependency on cmdline, you probably don't need > > > to > > > add it > > > as an external dependency to EAL too, but it doesn't really hurt > > > to > > > do so. > > > > > > /Bruce > > > > eal does not depend on cmdline, so added in both in v2. I also > > split > > the libbsd change and meson bump to 0.47.1 in a separate commit in > > the > > series. > > > > -- > > If there is no dependency, why is it being built first? Comment says "ethdev depends on cmdline for parsing functions" https://git.dpdk.org/dpdk/tree/lib/meson.build#n12 -- Kind regards, Luca Boccassi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 2/2] build: use dependency() instead of find_library() 2019-01-11 14:56 ` Luca Boccassi @ 2019-01-11 15:49 ` Bruce Richardson 2019-01-11 16:27 ` Luca Boccassi 0 siblings, 1 reply; 15+ messages in thread From: Bruce Richardson @ 2019-01-11 15:49 UTC (permalink / raw) To: Luca Boccassi; +Cc: dev, stable On Fri, Jan 11, 2019 at 02:56:28PM +0000, Luca Boccassi wrote: > On Fri, 2019-01-11 at 14:24 +0000, Bruce Richardson wrote: > > On Fri, Jan 11, 2019 at 12:39:23PM +0000, Luca Boccassi wrote: > > > On Fri, 2019-01-11 at 11:52 +0000, Bruce Richardson wrote: > > > > On Fri, Jan 11, 2019 at 11:10:28AM +0000, Luca Boccassi wrote: > > > > > On Mon, 2019-01-07 at 16:55 +0000, Bruce Richardson wrote: > > > > > > On Mon, Jan 07, 2019 at 04:39:34PM +0000, Luca Boccassi > > > > > > wrote: > > > > > > > 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: > > > > > > > > > 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? > > > > > > > > > > > > Should not happen. AFAIK when you link against a library in > > > > > > meson > > > > > > it > > > > > > will > > > > > > also link against any of that libraries dependencies too. For > > > > > > shared > > > > > > libraries meson always disallowed undefined references in the > > > > > > linker > > > > > > commandline. [To have libs with undefined refs, e.g. plugins, > > > > > > you > > > > > > need to > > > > > > use "shared_module" rather than "shared_library" command]. > > > > > > > > > > Looked at this again, and rte_cmdline is using strlcpy as well, > > > > > and > > > > > it's built before rte_eal, so it fails: > > > > > > > > > > lib/76b5a35@@rte_cmdline@sta/librte_cmdline_cmdline_parse.c.o: > > > > > In > > > > > function `cmdline_complete': > > > > > cmdline_parse.c:(.text+0x861): undefined reference to `strlcpy' > > > > > > > > > > Adding it to ext_deps in both rte_cmdline and rte_eal works. Is > > > > > that an acceptable compromise? > > > > > > > > > > > > > Sure. If eal has a dependency on cmdline, you probably don't need > > > > to > > > > add it > > > > as an external dependency to EAL too, but it doesn't really hurt > > > > to > > > > do so. > > > > > > > > /Bruce > > > > > > eal does not depend on cmdline, so added in both in v2. I also > > > split > > > the libbsd change and meson bump to 0.47.1 in a separate commit in > > > the > > > series. > > > > > > -- > > > > If there is no dependency, why is it being built first? > > Comment says "ethdev depends on cmdline for parsing functions" > > https://git.dpdk.org/dpdk/tree/lib/meson.build#n12 > > -- So it looks like it could be placed between EAL and ethdev. Not a bit deal either way, so I'm happy enough to leave it as-is for now. /Bruce ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [dpdk-stable] [dpdk-dev] [PATCH 2/2] build: use dependency() instead of find_library() 2019-01-11 15:49 ` Bruce Richardson @ 2019-01-11 16:27 ` Luca Boccassi 0 siblings, 0 replies; 15+ messages in thread From: Luca Boccassi @ 2019-01-11 16:27 UTC (permalink / raw) To: Bruce Richardson; +Cc: dev, stable On Fri, 2019-01-11 at 15:49 +0000, Bruce Richardson wrote: > On Fri, Jan 11, 2019 at 02:56:28PM +0000, Luca Boccassi wrote: > > On Fri, 2019-01-11 at 14:24 +0000, Bruce Richardson wrote: > > > On Fri, Jan 11, 2019 at 12:39:23PM +0000, Luca Boccassi wrote: > > > > On Fri, 2019-01-11 at 11:52 +0000, Bruce Richardson wrote: > > > > > On Fri, Jan 11, 2019 at 11:10:28AM +0000, Luca Boccassi > > > > > wrote: > > > > > > On Mon, 2019-01-07 at 16:55 +0000, Bruce Richardson wrote: > > > > > > > On Mon, Jan 07, 2019 at 04:39:34PM +0000, Luca Boccassi > > > > > > > wrote: > > > > > > > > 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: > > > > > > > > > > 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? > > > > > > > > > > > > > > Should not happen. AFAIK when you link against a library > > > > > > > in > > > > > > > meson > > > > > > > it > > > > > > > will > > > > > > > also link against any of that libraries dependencies too. > > > > > > > For > > > > > > > shared > > > > > > > libraries meson always disallowed undefined references in > > > > > > > the > > > > > > > linker > > > > > > > commandline. [To have libs with undefined refs, e.g. > > > > > > > plugins, > > > > > > > you > > > > > > > need to > > > > > > > use "shared_module" rather than "shared_library" > > > > > > > command]. > > > > > > > > > > > > Looked at this again, and rte_cmdline is using strlcpy as > > > > > > well, > > > > > > and > > > > > > it's built before rte_eal, so it fails: > > > > > > > > > > > > lib/76b5a35@@rte_cmdline@sta/librte_cmdline_cmdline_parse.c > > > > > > .o: > > > > > > In > > > > > > function `cmdline_complete': > > > > > > cmdline_parse.c:(.text+0x861): undefined reference to > > > > > > `strlcpy' > > > > > > > > > > > > Adding it to ext_deps in both rte_cmdline and rte_eal > > > > > > works. Is > > > > > > that an acceptable compromise? > > > > > > > > > > > > > > > > Sure. If eal has a dependency on cmdline, you probably don't > > > > > need > > > > > to > > > > > add it > > > > > as an external dependency to EAL too, but it doesn't really > > > > > hurt > > > > > to > > > > > do so. > > > > > > > > > > /Bruce > > > > > > > > eal does not depend on cmdline, so added in both in v2. I also > > > > split > > > > the libbsd change and meson bump to 0.47.1 in a separate commit > > > > in > > > > the > > > > series. > > > > > > > > -- > > > > > > If there is no dependency, why is it being built first? > > > > Comment says "ethdev depends on cmdline for parsing functions" > > > > https://git.dpdk.org/dpdk/tree/lib/meson.build#n12 > > > > -- > > So it looks like it could be placed between EAL and ethdev. Not a bit > deal > either way, so I'm happy enough to leave it as-is for now. > > /Bruce Reordering seems to work just fine, so I'll just go ahead and add another patch for it in v3. -- Kind regards, Luca Boccassi ^ permalink raw reply [flat|nested] 15+ messages in thread
* [dpdk-stable] [PATCH v2 1/3] build: use static deps of libs for pkg-config libs.private 2019-01-03 17:57 [dpdk-stable] [PATCH 1/2] build: use static deps of libs for pkg-config libs.private Luca Boccassi 2019-01-03 17:57 ` [dpdk-stable] [PATCH 2/2] build: use dependency() instead of find_library() Luca Boccassi @ 2019-01-11 12:38 ` Luca Boccassi 1 sibling, 0 replies; 15+ messages in thread From: Luca Boccassi @ 2019-01-11 12:38 UTC (permalink / raw) To: dev; +Cc: bruce.richardson, Luca Boccassi, stable Dependencies of the RTE libraries were not being added to the Requires.private field of the pc file since the variable used for dynamic linking was passed to the related field of pkg.generate. Use the static one so that dependencies are included. Fixes: 57ae0ec62620 ("build: add dependency on telemetry to apps with meson") Cc: stable@dpdk.org Signed-off-by: Luca Boccassi <bluca@debian.org> --- meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/meson.build b/meson.build index 426e0bf3a..d500507c5 100644 --- a/meson.build +++ b/meson.build @@ -81,7 +81,7 @@ pkg.generate(name: meson.project_name(), filebase: 'lib' + meson.project_name().to_lower(), version: meson.project_version(), libraries: dpdk_libraries, - libraries_private: dpdk_drivers + dpdk_libraries + + libraries_private: dpdk_drivers + dpdk_static_libraries + ['-Wl,-Bdynamic'] + dpdk_extra_ldflags, description: '''The Data Plane Development Kit (DPDK). Note that CFLAGS might contain an -march flag higher than typical baseline. -- 2.20.1 ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2019-01-11 16:27 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-01-03 17:57 [dpdk-stable] [PATCH 1/2] build: use static deps of libs for pkg-config libs.private Luca Boccassi 2019-01-03 17:57 ` [dpdk-stable] [PATCH 2/2] build: use dependency() instead of find_library() Luca Boccassi 2019-01-07 14:28 ` Bruce Richardson 2019-01-07 16:39 ` Luca Boccassi 2019-01-07 16:55 ` Bruce Richardson 2019-01-07 17:03 ` [dpdk-stable] [dpdk-techboard] " Thomas Monjalon 2019-01-07 17:45 ` Thomas Monjalon 2019-01-11 11:10 ` [dpdk-stable] [dpdk-dev] " Luca Boccassi 2019-01-11 11:52 ` Bruce Richardson 2019-01-11 12:39 ` Luca Boccassi 2019-01-11 14:24 ` Bruce Richardson 2019-01-11 14:56 ` Luca Boccassi 2019-01-11 15:49 ` Bruce Richardson 2019-01-11 16:27 ` Luca Boccassi 2019-01-11 12:38 ` [dpdk-stable] [PATCH v2 1/3] build: use static deps of libs for pkg-config libs.private Luca Boccassi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).