[v3,3/3] build: RFC - add support for optional dependencies
Checks
Commit Message
In order to remove more libraries from the mandatory list, we need to
have support for optionally having a dependency from a driver or library
to another driver or lib. This patch adds this support by adding a new
optional_deps variable, the contents of which are added to the deps list
if those optional dependencies are present in the build.
Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
drivers/meson.build | 7 +++++++
lib/meson.build | 7 +++++++
2 files changed, 14 insertions(+)
Comments
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Wednesday, 20 December 2023 15.22
>
> In order to remove more libraries from the mandatory list, we need to
> have support for optionally having a dependency from a driver or
> library
> to another driver or lib. This patch adds this support by adding a new
> optional_deps variable, the contents of which are added to the deps
> list
> if those optional dependencies are present in the build.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> drivers/meson.build | 7 +++++++
> lib/meson.build | 7 +++++++
> 2 files changed, 14 insertions(+)
>
> diff --git a/drivers/meson.build b/drivers/meson.build
> index 5ba534049a..af2d8da5a8 100644
> --- a/drivers/meson.build
> +++ b/drivers/meson.build
> @@ -127,6 +127,7 @@ foreach subpath:subdirs
> includes = [include_directories(drv_path)]
> # set up internal deps. Drivers can append/override as
> necessary
> deps = std_deps
> + optional_deps = []
(I'm a meson noob, so please bear with my stupid questions.)
Is a separate "optional_deps" necessary? If a driver has any of these dependencies, why can it not just add them to the "deps" in the driver's meson.build file?
Ohhh... It's the other way around: The driver only depends on the other (optional) lib if that other lib is enabled! Correct?
Either way, please add an in-line comment describing optional_deps in the meson.build file.
> # ext_deps: Stores external library dependency got
> # using dependency() (preferred) or find_library().
> # For the find_library() case (but not with dependency()) we
> also
> @@ -168,6 +169,12 @@ foreach subpath:subdirs
> # get dependency objs from strings
> shared_deps = ext_deps
> static_deps = ext_deps
> + foreach d:optional_deps
> + #if optional dep exists, add it to the deps list
> + if is_variable('shared_rte_' + d)
> + deps += d
> + endif
> + endforeach
> foreach d:deps
> if not build
> break
The same feedback also applies to the lib/meson.build changes.
On Wed, Dec 20, 2023 at 04:08:08PM +0100, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Wednesday, 20 December 2023 15.22
> >
> > In order to remove more libraries from the mandatory list, we need to
> > have support for optionally having a dependency from a driver or
> > library
> > to another driver or lib. This patch adds this support by adding a new
> > optional_deps variable, the contents of which are added to the deps
> > list
> > if those optional dependencies are present in the build.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > ---
> > drivers/meson.build | 7 +++++++
> > lib/meson.build | 7 +++++++
> > 2 files changed, 14 insertions(+)
> >
> > diff --git a/drivers/meson.build b/drivers/meson.build
> > index 5ba534049a..af2d8da5a8 100644
> > --- a/drivers/meson.build
> > +++ b/drivers/meson.build
> > @@ -127,6 +127,7 @@ foreach subpath:subdirs
> > includes = [include_directories(drv_path)]
> > # set up internal deps. Drivers can append/override as
> > necessary
> > deps = std_deps
> > + optional_deps = []
>
> (I'm a meson noob, so please bear with my stupid questions.)
>
> Is a separate "optional_deps" necessary? If a driver has any of these dependencies, why can it not just add them to the "deps" in the driver's meson.build file?
>
> Ohhh... It's the other way around: The driver only depends on the other (optional) lib if that other lib is enabled! Correct?
>
Yes, it's a shortcut to save an app having to manually check for its
optional dependency itself in the meson.build file.
However, this is only really useful if it's an optional dependency where we
just have #ifdefs in the C code for it. For the initial example I was
thinking of to try and use it - meter lib in ethdev - something that simple
is not enough. So long as there are extra C files or headers that need to
be built when a dependency is present, we need to change the meson.build
file to explicitly check anyway. Hence it's only an RFC until such time as
we find a use-case or two that uses it.
/Bruce
On Wed, 20 Dec 2023 15:43:36 +0000
Bruce Richardson <bruce.richardson@intel.com> wrote:
> On Wed, Dec 20, 2023 at 04:08:08PM +0100, Morten Brørup wrote:
> > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > Sent: Wednesday, 20 December 2023 15.22
> > >
> > > In order to remove more libraries from the mandatory list, we need to
> > > have support for optionally having a dependency from a driver or
> > > library
> > > to another driver or lib. This patch adds this support by adding a new
> > > optional_deps variable, the contents of which are added to the deps
> > > list
> > > if those optional dependencies are present in the build.
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > > ---
> > > drivers/meson.build | 7 +++++++
> > > lib/meson.build | 7 +++++++
> > > 2 files changed, 14 insertions(+)
> > >
> > > diff --git a/drivers/meson.build b/drivers/meson.build
> > > index 5ba534049a..af2d8da5a8 100644
> > > --- a/drivers/meson.build
> > > +++ b/drivers/meson.build
> > > @@ -127,6 +127,7 @@ foreach subpath:subdirs
> > > includes = [include_directories(drv_path)]
> > > # set up internal deps. Drivers can append/override as
> > > necessary
> > > deps = std_deps
> > > + optional_deps = []
> >
> > (I'm a meson noob, so please bear with my stupid questions.)
> >
> > Is a separate "optional_deps" necessary? If a driver has any of these dependencies, why can it not just add them to the "deps" in the driver's meson.build file?
> >
> > Ohhh... It's the other way around: The driver only depends on the other (optional) lib if that other lib is enabled! Correct?
> >
> Yes, it's a shortcut to save an app having to manually check for its
> optional dependency itself in the meson.build file.
>
> However, this is only really useful if it's an optional dependency where we
> just have #ifdefs in the C code for it. For the initial example I was
> thinking of to try and use it - meter lib in ethdev - something that simple
> is not enough. So long as there are extra C files or headers that need to
> be built when a dependency is present, we need to change the meson.build
> file to explicitly check anyway. Hence it's only an RFC until such time as
> we find a use-case or two that uses it.
Reasonable solution, but it hasn't been an issue for anything so far over a year.
Marking it as "Awaiting upstream" since if a feature needs this, then use this.
@@ -127,6 +127,7 @@ foreach subpath:subdirs
includes = [include_directories(drv_path)]
# set up internal deps. Drivers can append/override as necessary
deps = std_deps
+ optional_deps = []
# ext_deps: Stores external library dependency got
# using dependency() (preferred) or find_library().
# For the find_library() case (but not with dependency()) we also
@@ -168,6 +169,12 @@ foreach subpath:subdirs
# get dependency objs from strings
shared_deps = ext_deps
static_deps = ext_deps
+ foreach d:optional_deps
+ #if optional dep exists, add it to the deps list
+ if is_variable('shared_rte_' + d)
+ deps += d
+ endif
+ endforeach
foreach d:deps
if not build
break
@@ -140,6 +140,7 @@ foreach l:libraries
# external package/library requirements
ext_deps = []
deps = []
+ optional_deps = []
# eal is standard dependency once built
if dpdk_conf.has('RTE_LIB_EAL')
deps += ['eal']
@@ -177,6 +178,12 @@ foreach l:libraries
shared_deps = ext_deps
static_deps = ext_deps
+ foreach d:optional_deps
+ #if optional dep exists, add it to the deps list
+ if is_variable('shared_rte_' + d)
+ deps += d
+ endif
+ endforeach
foreach d:deps
if not build
break