> -----Original Message-----
> From: Richard Purdie <richard.pur...@linuxfoundation.org>
> Sent: den 4 mars 2024 18:39
> To: Peter Kjellerstedt <peter.kjellerst...@axis.com>; 
> openembedded-core@lists.openembedded.org
> Subject: Re: [OE-core] [PATCH 1/2] kernel-module-dirs.bbclass: Add class
> 
> On Mon, 2024-03-04 at 17:20 +0000, Peter Kjellerstedt wrote:
> > > -----Original Message-----
> > > From: Richard Purdie <richard.pur...@linuxfoundation.org>
> > > Sent: den 4 mars 2024 17:17
> > > To: Peter Kjellerstedt <peter.kjellerst...@axis.com>; 
> > > openembedded-core@lists.openembedded.org
> > > Subject: Re: [OE-core] [PATCH 1/2] kernel-module-dirs.bbclass: Add class
> > >
> > > On Mon, 2024-03-04 at 16:57 +0100, Peter Kjellerstedt wrote:
> > > > Split out the two variables modulesloaddir and modprobedir from
> > > > kernel-module-split.bbclass as they can be useful to other recipes than
> > > > kernel module recipes.
> > > >
> > > > Signed-off-by: Peter Kjellerstedt <peter.kjellerst...@axis.com>
> > > > ---
> > > >  meta/classes-recipe/kernel-module-dirs.bbclass | 8 ++++++++
> > > >  1 file changed, 8 insertions(+)
> > > >  create mode 100644 meta/classes-recipe/kernel-module-dirs.bbclass
> > > >
> > > > diff --git a/meta/classes-recipe/kernel-module-dirs.bbclass 
> > > > b/meta/classes-recipe/kernel-module-dirs.bbclass
> > > > new file mode 100644
> > > > index 0000000000..eecc36ab52
> > > > --- /dev/null
> > > > +++ b/meta/classes-recipe/kernel-module-dirs.bbclass
> > > > @@ -0,0 +1,8 @@
> > > > +#
> > > > +# Copyright OpenEmbedded Contributors
> > > > +#
> > > > +# SPDX-License-Identifier: MIT
> > > > +#
> > > > +
> > > > +modulesloaddir ??= "${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 
> > > > '${nonarch_libdir}', '${sysconfdir}', d)}/modules-load.d"
> > > > +modprobedir ??= "${@bb.utils.contains('DISTRO_FEATURES', 'systemd', 
> > > > '${nonarch_base_libdir}', '${sysconfdir}', d)}/modprobe.d"
> > >
> > > Absolutely not. We are not having yet more kernel classes just for two
> > > variables.
> >
> > Ok. I considered adding them to bitbake.conf where all other similar *dir
> > variables are defined, but I opted for a bbclass since I expect it to only
> > be relatively few recipes that need either of them (we currently have ~10
> > non-module recipes that could use them).
> 
> bitbake.conf is also an absolute no.

Yes, that was what I expected.

> 
> > > There is probably a better way moving some definitions to a new conf
> > > file for the kernel in general.
> >
> > I am not sure what to make of this. What I wanted to achieve was to be
> > able to make the two path variables available to non-module recipes. I am
> > not sure how that matches what you meant with "a new conf file for the
> > kernel". Did you actually mean a .conf file where the variables would be
> > added to the global state, or did you mean a .bbclass/.inc file similar to
> > the one I proposed, but with a more generic name so that it can take other
> > variables than just the two path variables above?
> 
> I mean something more like meta/conf/image-uefi.conf but kernel focused.

Hmm, the naming of that file messes with the expectations I've learnt over 
the years of working with OE. I've always  thought that .conf files are 
used for definitions that are part of the global configuration, and .inc 
files are used for more local definitions. While I of course know that 
there is no technical difference between the two, that kind of semantics 
is helpful when looking at an individual file to know the context in which 
it is used.

> 
> We need to do better about more focused conf/inc files.

In that regard, kernel-module-dirs.bbclass was very focused. ;)

Do you see a difference in, e.g., kernel-module-dirs.bbclass vs.
kernel-module-dirs.inc? I.e., why is an .inc (or .conf) file more suitable 
than a .bbclass file in this case?

One reason I can see for why a .bbclass would be preferred, is because it 
is only inherited once even if there are multiple inherits. E.g., say I 
would take the proposed kernel-module-dirs.bbclass and turn it into a more 
generic kernel-vars.inc file. This file would then most likely be needed 
by, e.g., kernel.bbclass and module.bbclass. However, based on its current 
contents, it would also be needed by kernel-module-split.bbclass that both 
of them inherit. But since it is an .inc file, requiring it from both 
kernel.bbclass and kernel-module-split.bbclass would result in a lot of 
warnings about duplicate inclusion. On the other hand, having 
kernel-module-split.bbclass rely on that whatever inherits it has already 
required the kernel-vars.inc file seems wrong.

> 
> Cheers,
> 
> Richard

//Peter

-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#196616): 
https://lists.openembedded.org/g/openembedded-core/message/196616
Mute This Topic: https://lists.openembedded.org/mt/104724883/21656
Group Owner: openembedded-core+ow...@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub 
[arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to