On Wed, Nov 17, 2021 at 11:47 AM Bruce Richardson
<bruce.richard...@intel.com> wrote:
>
> On Tue, Nov 16, 2021 at 06:25:28PM +0100, Thomas Monjalon wrote:
> > 10/11/2021 18:34, Bruce Richardson:
> > > On Wed, Nov 10, 2021 at 05:48:14PM +0100, David Marchand wrote:
> > > > There is currently no way to know which libraries are optional.
> > > > Introduce a enable_libs option (close to what we have for drivers) so
> > > > that packagers or projects consuming DPDK can more easily select the
> > > > optional libraries that matter to them and disable other optional
> > > > libraries.
> > > >
> > > > Note: the enabled_libs variable is renamed for sake of consistency.
> > > >
> > > > Signed-off-by: David Marchand <david.march...@redhat.com>
> > > > ---
> > > This is the only patch of this set I would have some concerns about. I'm
> > > just not sure that it makes sense to have this option for libraries
> > > compared to drivers.
> > >
> > > Specifically:
> > > * We have over 200 drivers in DPDK (rough count using find), of which 2 
> > > are
> > >   mandatory, and therefore specifying just 1 or 2 that you want can make
> > >   sense.
> > > * On the other hand, we have 53 libraries, of which only 7 or so (after
> > >   this patchset) are optional. This means that use of the term
> > >   "enable_libs" is misleading - at least to me - in that it's only a very
> > >   small proportion of the libs which would be affected by that flag
> > >   (compared to 99% of the drivers)
> >
> > The options are described like this:
> >
> > option('disable_libs', type: 'string', value: '', description:
> >        'Comma-separated list of libraries to explicitly disable. [NOTE: not 
> > all libs can be disabled]')
> > +option('enable_libs', type: 'string', value: '', description:
> > +       'Comma-separated list of libraries to explicitly enable.')
> >
> > I feel we should mention it is enabling optional libraries,
> > and the default is to enable all.
> >
> > > * Also, while the number of mandatory drivers is unlikely to change much
> > >   (since there are only 2), it should be fairly safe to do builds using
> > >   "--enable-drivers". On the other hand, the list of libraries affected by
> > >   "--enable-libs" is likely to change, so short of each user naming each
> > >   and every lib they use (and each library those depend on), to the list,
> > >   it's quite possible that any --enable-libs use could lead to a broken
> > >   build in future if a library changes from mandatory to optional.
> >
> > In order to be safe, the user can list all required libs,
> > including non-optional ones.
> >
>
> Yes, they can, but that is the part I'm concerned about. It should not be
> expected for users to know what all libraries should be needed and
> dependencies between them. The list of mandatory libraries is quite long.
>
> > > Overall, I'm just concerned that this flag is premature, and would prefer
> > > to keep it just to the disable option until we are confident that out
> > > "optional library" list is relatively settled.
> >
> > I see it the opposite way:
> > Someone who does not wish to deliver extra libs could use this option
> > to list the required libs, so not-required libs will disappear from
> > the build once they are declared optional in future releases.
> >
>
> Yes, though as-above, I'm concerned about the difficulty of building up
> such a list. However, if this option is only for "expert" and
> distro-packaging use, then I suppose it's reasonable. Perhaps we should add
> a warning to the doc line too, noting that it's only recommended for
> advanced users.
>

Ok, we still need some discussion.
I'll just respin for Thomas to merge patches that are ready.


-- 
David Marchand

Reply via email to