[v4,3/4] build: select deprecated libraries
Checks
Commit Message
Rework deprecated libraries selection by introducing a new configuration
option.
This breaks existing configurations that were relying on disable_libs=''
for enabling deprecated libraries.
On the other hand, it will make enabling optional libraries more
straightforward by taking the deprecated libraries out of the picture.
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
.ci/linux-build.sh | 2 +-
devtools/test-meson-builds.sh | 8 ++++----
lib/meson.build | 9 +++++++--
meson_options.txt | 4 +++-
4 files changed, 15 insertions(+), 8 deletions(-)
Comments
On Wed, Jun 21, 2023 at 07:00:57PM +0200, David Marchand wrote:
> Rework deprecated libraries selection by introducing a new configuration
> option.
>
> This breaks existing configurations that were relying on disable_libs=''
> for enabling deprecated libraries.
> On the other hand, it will make enabling optional libraries more
> straightforward by taking the deprecated libraries out of the picture.
>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
This gives us a single on/off value for the deprecated libs. So if you
wants to build only a single deprecated lib, you need to turn on this
option and then use "disable_libs/enable_libs" option to then selectively
pick which of the deprecated libs you actually want. Is that the expected
behaviour? Just checking that we don't want this to be a list too.
On Thu, Jun 22, 2023 at 09:43:18AM +0100, Bruce Richardson wrote:
> On Wed, Jun 21, 2023 at 07:00:57PM +0200, David Marchand wrote:
> > Rework deprecated libraries selection by introducing a new configuration
> > option.
> >
> > This breaks existing configurations that were relying on disable_libs=''
> > for enabling deprecated libraries.
> > On the other hand, it will make enabling optional libraries more
> > straightforward by taking the deprecated libraries out of the picture.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
>
> This gives us a single on/off value for the deprecated libs. So if you
> wants to build only a single deprecated lib, you need to turn on this
> option and then use "disable_libs/enable_libs" option to then selectively
> pick which of the deprecated libs you actually want. Is that the expected
> behaviour? Just checking that we don't want this to be a list too.
>
The patch is fine itself though, so:
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
Hello Bruce,
On Thu, Jun 22, 2023 at 10:43 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Wed, Jun 21, 2023 at 07:00:57PM +0200, David Marchand wrote:
> > Rework deprecated libraries selection by introducing a new configuration
> > option.
> >
> > This breaks existing configurations that were relying on disable_libs=''
> > for enabling deprecated libraries.
> > On the other hand, it will make enabling optional libraries more
> > straightforward by taking the deprecated libraries out of the picture.
> >
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
>
> This gives us a single on/off value for the deprecated libs. So if you
> wants to build only a single deprecated lib, you need to turn on this
> option and then use "disable_libs/enable_libs" option to then selectively
> pick which of the deprecated libs you actually want. Is that the expected
> behaviour? Just checking that we don't want this to be a list too.
Yes, I wanted a single unified filtering stage.
But I think your suggestion is easier to use.
- That would make it simpler for people who simply want to enable kni,
as you mentionned before:
$ meson setup plop -Denable_deprecated_libs=kni
But I would make this list not overlap with the disable/enable_libs
options evaluation.
Otherwise, in the case of a enable_libs user, the user would have to
set kni in both lists, which is not that great:
$ meson setup plop -Denable_deprecated_libs=kni -Denable_libs=kni,vhost
Instead, I would make it so the config is done as:
$ meson setup plop -Denable_deprecated_libs=kni -Denable_libs=vhost
Is this what you had in mind?
- I don't have a usage for this, but if we go with separating
deprecated and "normal" optional libs filtering, should I introduce a
disable_deprecated_libs too?
On Fri, Jun 23, 2023 at 11:35:29AM +0200, David Marchand wrote:
> Hello Bruce,
>
> On Thu, Jun 22, 2023 at 10:43 AM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Wed, Jun 21, 2023 at 07:00:57PM +0200, David Marchand wrote:
> > > Rework deprecated libraries selection by introducing a new configuration
> > > option.
> > >
> > > This breaks existing configurations that were relying on disable_libs=''
> > > for enabling deprecated libraries.
> > > On the other hand, it will make enabling optional libraries more
> > > straightforward by taking the deprecated libraries out of the picture.
> > >
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> >
> > This gives us a single on/off value for the deprecated libs. So if you
> > wants to build only a single deprecated lib, you need to turn on this
> > option and then use "disable_libs/enable_libs" option to then selectively
> > pick which of the deprecated libs you actually want. Is that the expected
> > behaviour? Just checking that we don't want this to be a list too.
>
> Yes, I wanted a single unified filtering stage.
>
> But I think your suggestion is easier to use.
Slightly easier for the simple case.
>
> - That would make it simpler for people who simply want to enable kni,
> as you mentionned before:
> $ meson setup plop -Denable_deprecated_libs=kni
>
> But I would make this list not overlap with the disable/enable_libs
> options evaluation.
> Otherwise, in the case of a enable_libs user, the user would have to
> set kni in both lists, which is not that great:
> $ meson setup plop -Denable_deprecated_libs=kni -Denable_libs=kni,vhost
>
> Instead, I would make it so the config is done as:
> $ meson setup plop -Denable_deprecated_libs=kni -Denable_libs=vhost
>
> Is this what you had in mind?
>
I'm not sure myself what I had in mind, just asking if it had been
considered as much as anything else.
Having them not-overlap would seem to be necessary to provide a meaningful
interface.
>
> - I don't have a usage for this, but if we go with separating
> deprecated and "normal" optional libs filtering, should I introduce a
> disable_deprecated_libs too?
>
That would give us *way* to many options. I think for the sake of simplicity
we probably are as well to just go with what you are proposing in this set.
Since we only have two deprecated libraries - and hopefully never many more -
the benefit of the list for that setting is probably minimal. I'm keen to
avoid too much complexity if we can manage it.
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Friday, 23 June 2023 13.04
>
> On Fri, Jun 23, 2023 at 11:35:29AM +0200, David Marchand wrote:
> > Hello Bruce,
> >
> > On Thu, Jun 22, 2023 at 10:43 AM Bruce Richardson
> > <bruce.richardson@intel.com> wrote:
> > >
> > > On Wed, Jun 21, 2023 at 07:00:57PM +0200, David Marchand wrote:
> > > > Rework deprecated libraries selection by introducing a new configuration
> > > > option.
> > > >
> > > > This breaks existing configurations that were relying on disable_libs=''
> > > > for enabling deprecated libraries.
> > > > On the other hand, it will make enabling optional libraries more
> > > > straightforward by taking the deprecated libraries out of the picture.
> > > >
> > > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > >
> > > This gives us a single on/off value for the deprecated libs. So if you
> > > wants to build only a single deprecated lib, you need to turn on this
> > > option and then use "disable_libs/enable_libs" option to then selectively
> > > pick which of the deprecated libs you actually want. Is that the expected
> > > behaviour? Just checking that we don't want this to be a list too.
> >
> > Yes, I wanted a single unified filtering stage.
> >
> > But I think your suggestion is easier to use.
>
> Slightly easier for the simple case.
>
> >
> > - That would make it simpler for people who simply want to enable kni,
> > as you mentionned before:
> > $ meson setup plop -Denable_deprecated_libs=kni
> >
> > But I would make this list not overlap with the disable/enable_libs
> > options evaluation.
> > Otherwise, in the case of a enable_libs user, the user would have to
> > set kni in both lists, which is not that great:
> > $ meson setup plop -Denable_deprecated_libs=kni -Denable_libs=kni,vhost
> >
> > Instead, I would make it so the config is done as:
> > $ meson setup plop -Denable_deprecated_libs=kni -Denable_libs=vhost
> >
> > Is this what you had in mind?
> >
> I'm not sure myself what I had in mind, just asking if it had been
> considered as much as anything else.
>
> Having them not-overlap would seem to be necessary to provide a meaningful
> interface.
>
> >
> > - I don't have a usage for this, but if we go with separating
> > deprecated and "normal" optional libs filtering, should I introduce a
> > disable_deprecated_libs too?
> >
>
> That would give us *way* to many options. I think for the sake of simplicity
> we probably are as well to just go with what you are proposing in this set.
> Since we only have two deprecated libraries - and hopefully never many more -
> the benefit of the list for that setting is probably minimal. I'm keen to
> avoid too much complexity if we can manage it.
I strongly agree with Bruce about avoiding too many options. Here's an idea:
How about just having the disable/enable_libs options, and by default omit the deprecated libs.
Then, the deprecated libs can be enabled by using the enable_libs option.
Do we have special treatment for deprecated drivers?
On Fri, Jun 23, 2023 at 01:15:00PM +0200, Morten Brørup wrote:
> > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > Sent: Friday, 23 June 2023 13.04
> >
> > On Fri, Jun 23, 2023 at 11:35:29AM +0200, David Marchand wrote:
> > > Hello Bruce,
> > >
> > > On Thu, Jun 22, 2023 at 10:43 AM Bruce Richardson
> > > <bruce.richardson@intel.com> wrote:
> > > >
> > > > On Wed, Jun 21, 2023 at 07:00:57PM +0200, David Marchand wrote:
> > > > > Rework deprecated libraries selection by introducing a new configuration
> > > > > option.
> > > > >
> > > > > This breaks existing configurations that were relying on disable_libs=''
> > > > > for enabling deprecated libraries.
> > > > > On the other hand, it will make enabling optional libraries more
> > > > > straightforward by taking the deprecated libraries out of the picture.
> > > > >
> > > > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > >
> > > > This gives us a single on/off value for the deprecated libs. So if you
> > > > wants to build only a single deprecated lib, you need to turn on this
> > > > option and then use "disable_libs/enable_libs" option to then selectively
> > > > pick which of the deprecated libs you actually want. Is that the expected
> > > > behaviour? Just checking that we don't want this to be a list too.
> > >
> > > Yes, I wanted a single unified filtering stage.
> > >
> > > But I think your suggestion is easier to use.
> >
> > Slightly easier for the simple case.
> >
> > >
> > > - That would make it simpler for people who simply want to enable kni,
> > > as you mentionned before:
> > > $ meson setup plop -Denable_deprecated_libs=kni
> > >
> > > But I would make this list not overlap with the disable/enable_libs
> > > options evaluation.
> > > Otherwise, in the case of a enable_libs user, the user would have to
> > > set kni in both lists, which is not that great:
> > > $ meson setup plop -Denable_deprecated_libs=kni -Denable_libs=kni,vhost
> > >
> > > Instead, I would make it so the config is done as:
> > > $ meson setup plop -Denable_deprecated_libs=kni -Denable_libs=vhost
> > >
> > > Is this what you had in mind?
> > >
> > I'm not sure myself what I had in mind, just asking if it had been
> > considered as much as anything else.
> >
> > Having them not-overlap would seem to be necessary to provide a meaningful
> > interface.
> >
> > >
> > > - I don't have a usage for this, but if we go with separating
> > > deprecated and "normal" optional libs filtering, should I introduce a
> > > disable_deprecated_libs too?
> > >
> >
> > That would give us *way* to many options. I think for the sake of simplicity
> > we probably are as well to just go with what you are proposing in this set.
> > Since we only have two deprecated libraries - and hopefully never many more -
> > the benefit of the list for that setting is probably minimal. I'm keen to
> > avoid too much complexity if we can manage it.
>
> I strongly agree with Bruce about avoiding too many options. Here's an idea:
>
> How about just having the disable/enable_libs options, and by default omit the deprecated libs.
>
> Then, the deprecated libs can be enabled by using the enable_libs option.
>
That was the original suggestion and implementation. The trouble is that
if you specify, for example, -Denable_libs=kni, you have just disabled
every other optional library in DPDK, since the enable_libs option switches
DPDK to "allowlist"-only mode.
> Do we have special treatment for deprecated drivers?
>
Its the best solution we have come up with so far to work around the above
problem.
/Bruce
> From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> Sent: Friday, 23 June 2023 13.19
>
> On Fri, Jun 23, 2023 at 01:15:00PM +0200, Morten Brørup wrote:
> > > From: Bruce Richardson [mailto:bruce.richardson@intel.com]
> > > Sent: Friday, 23 June 2023 13.04
> > >
> > > On Fri, Jun 23, 2023 at 11:35:29AM +0200, David Marchand wrote:
> > > > Hello Bruce,
> > > >
> > > > On Thu, Jun 22, 2023 at 10:43 AM Bruce Richardson
> > > > <bruce.richardson@intel.com> wrote:
> > > > >
> > > > > On Wed, Jun 21, 2023 at 07:00:57PM +0200, David Marchand wrote:
> > > > > > Rework deprecated libraries selection by introducing a new
> configuration
> > > > > > option.
> > > > > >
> > > > > > This breaks existing configurations that were relying on
> disable_libs=''
> > > > > > for enabling deprecated libraries.
> > > > > > On the other hand, it will make enabling optional libraries more
> > > > > > straightforward by taking the deprecated libraries out of the
> picture.
> > > > > >
> > > > > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > > >
> > > > > This gives us a single on/off value for the deprecated libs. So if you
> > > > > wants to build only a single deprecated lib, you need to turn on this
> > > > > option and then use "disable_libs/enable_libs" option to then
> selectively
> > > > > pick which of the deprecated libs you actually want. Is that the
> expected
> > > > > behaviour? Just checking that we don't want this to be a list too.
> > > >
> > > > Yes, I wanted a single unified filtering stage.
> > > >
> > > > But I think your suggestion is easier to use.
> > >
> > > Slightly easier for the simple case.
> > >
> > > >
> > > > - That would make it simpler for people who simply want to enable kni,
> > > > as you mentionned before:
> > > > $ meson setup plop -Denable_deprecated_libs=kni
> > > >
> > > > But I would make this list not overlap with the disable/enable_libs
> > > > options evaluation.
> > > > Otherwise, in the case of a enable_libs user, the user would have to
> > > > set kni in both lists, which is not that great:
> > > > $ meson setup plop -Denable_deprecated_libs=kni -Denable_libs=kni,vhost
> > > >
> > > > Instead, I would make it so the config is done as:
> > > > $ meson setup plop -Denable_deprecated_libs=kni -Denable_libs=vhost
> > > >
> > > > Is this what you had in mind?
> > > >
> > > I'm not sure myself what I had in mind, just asking if it had been
> > > considered as much as anything else.
> > >
> > > Having them not-overlap would seem to be necessary to provide a meaningful
> > > interface.
> > >
> > > >
> > > > - I don't have a usage for this, but if we go with separating
> > > > deprecated and "normal" optional libs filtering, should I introduce a
> > > > disable_deprecated_libs too?
> > > >
> > >
> > > That would give us *way* to many options. I think for the sake of
> simplicity
> > > we probably are as well to just go with what you are proposing in this
> set.
> > > Since we only have two deprecated libraries - and hopefully never many
> more -
> > > the benefit of the list for that setting is probably minimal. I'm keen to
> > > avoid too much complexity if we can manage it.
> >
> > I strongly agree with Bruce about avoiding too many options. Here's an idea:
> >
> > How about just having the disable/enable_libs options, and by default omit
> the deprecated libs.
> >
> > Then, the deprecated libs can be enabled by using the enable_libs option.
> >
>
> That was the original suggestion and implementation. The trouble is that
> if you specify, for example, -Denable_libs=kni, you have just disabled
> every other optional library in DPDK, since the enable_libs option switches
> DPDK to "allowlist"-only mode.
OK. Hmmm.... Perhaps we can accept that, using this argument:
If you are in a situation where you are using some deprecated libs, you probably also know exactly which other optional libs you are using. So you must specify the full list on enable_libs.
I won't object if anyone disagrees with this idea. I'm only trying to keep it simple. :-)
>
> > Do we have special treatment for deprecated drivers?
> >
> Its the best solution we have come up with so far to work around the above
> problem.
>
> /Bruce
On Fri, Jun 23, 2023 at 1:33 PM Morten Brørup <mb@smartsharesystems.com> wrote:
> > > > > - I don't have a usage for this, but if we go with separating
> > > > > deprecated and "normal" optional libs filtering, should I introduce a
> > > > > disable_deprecated_libs too?
> > > > >
> > > >
> > > > That would give us *way* to many options. I think for the sake of
> > simplicity
> > > > we probably are as well to just go with what you are proposing in this
> > set.
> > > > Since we only have two deprecated libraries - and hopefully never many
> > more -
> > > > the benefit of the list for that setting is probably minimal. I'm keen to
> > > > avoid too much complexity if we can manage it.
> > >
> > > I strongly agree with Bruce about avoiding too many options. Here's an idea:
> > >
> > > How about just having the disable/enable_libs options, and by default omit
> > the deprecated libs.
> > >
> > > Then, the deprecated libs can be enabled by using the enable_libs option.
> > >
> >
> > That was the original suggestion and implementation. The trouble is that
> > if you specify, for example, -Denable_libs=kni, you have just disabled
> > every other optional library in DPDK, since the enable_libs option switches
> > DPDK to "allowlist"-only mode.
>
> OK. Hmmm.... Perhaps we can accept that, using this argument:
>
> If you are in a situation where you are using some deprecated libs, you probably also know exactly which other optional libs you are using. So you must specify the full list on enable_libs.
>
> I won't object if anyone disagrees with this idea. I'm only trying to keep it simple. :-)
- Mixing deprecated and "normal" libraries in a single generic option
means that people (who are selecting some optional libraries only atm)
could miss when an optional library is deprecated (in the future).
On the other hand, if we go with a specialised option for deprecated
libs, users will have to explicitly ask for keeping them.
I think it is also clearer from a user pov.
So I would stick with a separate handling of normal and deprecated libs.
- Adding a disable_deprecated_libs option does not make much sense for
only two libraries.
- For v23.07, I'll announce a change in behavior in the build system
wrt deprecated libraries.
I'll respin on top of Bruce idea (implementing enable_deprecated_libs
as a list) for now.
And we can conclude on how to best expose/configure this by v23.11.
@@ -97,7 +97,7 @@ if [ "$MINI" = "true" ]; then
OPTS="$OPTS -Denable_drivers=net/null"
OPTS="$OPTS -Ddisable_libs=*"
else
- OPTS="$OPTS -Ddisable_libs="
+ OPTS="$OPTS -Denable_deprecated_libs=true"
fi
if [ "$ASAN" = "true" ]; then
@@ -120,10 +120,10 @@ config () # <dir> <builddir> <meson options>
return
fi
options=
- # deprecated libs may be disabled by default, so for complete builds ensure
- # no libs are disabled
- if ! echo $* | grep -q -- 'disable_libs' ; then
- options="$options -Ddisable_libs="
+ # deprecated libs are disabled by default, so for complete builds
+ # enable them
+ if ! echo $* | grep -q -- 'enable_deprecated_libs' ; then
+ options="$options -Denable_deprecated_libs=true"
fi
if echo $* | grep -qw -- '--default-library=shared' ; then
options="$options -Dexamples=all"
@@ -136,13 +136,18 @@ foreach l:libraries
deps += ['eal']
endif
- if disabled_libs.contains(l)
+ if dpdk_libs_deprecated.contains(l) and not get_option('enable_deprecated_libs')
+ build = false
+ reason = 'deprecated libraries disabled via build config'
+ elif disabled_libs.contains(l)
build = false
reason = 'explicitly disabled via build config'
if dpdk_libs_deprecated.contains(l)
reason += ' (deprecated lib)'
endif
- else
+ endif
+
+ if build
if dpdk_libs_deprecated.contains(l)
warning('Enabling deprecated library, "@0@"'.format(l))
endif
@@ -10,7 +10,7 @@ option('disable_apps', type: 'string', value: '', description:
'Comma-separated list of apps to explicitly disable.')
option('disable_drivers', type: 'string', value: '', description:
'Comma-separated list of drivers to explicitly disable.')
-option('disable_libs', type: 'string', value: 'flow_classify,kni', description:
+option('disable_libs', type: 'string', value: '', description:
'Comma-separated list of libraries to explicitly disable. [NOTE: not all libs can be disabled]')
option('drivers_install_subdir', type: 'string', value: 'dpdk/pmds-<VERSION>', description:
'Subdirectory of libdir where to install PMDs. Defaults to using a versioned subdirectory.')
@@ -18,6 +18,8 @@ option('enable_docs', type: 'boolean', value: false, description:
'build documentation')
option('enable_apps', type: 'string', value: '', description:
'Comma-separated list of apps to build. If unspecified, build all apps.')
+option('enable_deprecated_libs', type: 'boolean', value: false, description:
+ 'Add deprecated libraries to the list of optional libraries to build')
option('enable_drivers', type: 'string', value: '', description:
'Comma-separated list of drivers to build. If unspecified, build all drivers.')
option('enable_driver_sdk', type: 'boolean', value: false, description: