Re: [PATCH v4 2/9] event/dlb2: skip configuration if no eventdev lib

[David Marchand <david.marchand@redhat.com>]

Hello Bruce,

On Tue, Jul 18, 2023 at 11:10 AM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Thu, Jun 29, 2023 at 11:39:53AM +0200, David Marchand wrote:
> > On Wed, Jun 28, 2023 at 12:19 PM David Marchand
> > <david.marchand@redhat.com> wrote:
> > >
> > > On Fri, Jun 23, 2023 at 5:07 PM Bruce Richardson
> > > <bruce.richardson@intel.com> wrote:
> > > >
> > > > While the build system will skip building most libs and drivers when a
> > > > dependency is missing for a component, for DLB2 driver, the
> > > > "static_rte_eventdev" object is referenced inside the meson.build file
> > > > itself, which will cause crashes if it doesn't exist i.e. if eventdev is
> > > > disabled. Prevent this issue by skipping processing the file if no
> > > > eventdev. [The build system will still report missing dependency, as the
> > > > dependency is set by default for all eventdev drivers]
> > > >
> > > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > >
> > > Could we evaluate the class "std_deps" before jumping to each driver
> > > meson.build?
> >
> > Hum, with my suggestion, we lose the opportunity for drivers to
> > rewrite completely their "deps".
> > I doubt we have cases where it really matters, but if this revealed to
> > be necessary, such driver may be directly referenced in
> > drivers/meson.build like we do for common/cnxk & friends.
> >
> > To illustrate the idea, I pushed your series along patches of mine
> > (target is v23.11) in my github repo.
> > https://github.com/david-marchand/dpdk/commit/enable_libs~8
> >
> I don't think we should need to worry too much about the case of a driver
> needing to rewrite deps. The standard deps should really just be the
> minimal deps for a class.
>
> However, for the implementation of that check, I'm not sure I like the
> approach of checking std_deps in a loop for each driver. Firstly, the logic
> inside the per-driver block is already pretty complicated, but secondly, I
> think we can skip that loop entirely if the standard deps for a class are
> not met. I think instead it would be better to process std_deps value once
> immediately after the "subdir(class)" line, and if not all deps are
> present, just loop through all the driver directories for that class
> immediately and mark them as unbuildable, before just moving on to the next
> class.

Skipping this loop makes the change smaller and the logic in
drivers/meson.build easier to understand.
We lose the exhaustive summary for disabled content (before, every
disabled driver was logged with the associated reason), but we can
still log the reason why a class is disabled.

Your suggestion looks good to me, let me send a patch.


-- 
David Marchand