Re: Should we change variable override formatting?


Richard Purdie
 

On Thu, 2021-07-15 at 09:26 -0500, Mark Hatle wrote:

On 7/15/21 8:56 AM, Richard Purdie wrote:
Breaking things down a bit, one thing I keep running into with our current 
codebase and metadata is that overrides are not clear. In my previous email,
the example was:

do_configure_class-native

A human can usually spot "class-native" is and "configure" or 
"configure_class-native" is not. Bitbake's parser struggles. It has huge 
internal lists including variables like x86_64 where it has to track 
whether "64" in an override.

One way of fixing this is to be explicit about overrides and use a different 
separator. See an example patch below I made to the quilt recipe using ":" 
instead to see how it looks. Personally, I think this looks like an improvement.

There are two challenges:

a) The work of migration. Do we migrate piecemeal or with a flag day? I'm not
   sure piecemeal is even possible unfortunately.

b) Some of the packaging code (at the very least) needs rewriting as it accesses
   both RDEPENDS_${PN} and puts ${PN} in OVERRIDES and accesses RDEPENDS. I'm not
   sure what else may be assuming this works.
Ya, this is all over the packaging code. Should be trivial to move to treating
it as a variable (and not override). I can help with this if you want.
I've been pondering this a bit more and my preference is to move this to use 
overrides more explicitly.

What I did realise is that d.getVar("RDEPENDS:${PN}") can actually split out
the override itself and apply it so I think the existing code can be made to
work just with the character change if we teach getVar how to handle it which
should be possible.

(It can of course also be moved to overrides, but that won't be as obvious to a
recipe creator, since they're not used to the package name being an 'override'.)
Whilst the instinct is to "hide" that, perhaps it could also make overrides more
obvious since people could see them in a different setting more clearly? I think
that is where I'm leaning right now.

This change does buy us cleaner looking metadata and ultimately, a faster
and cleaner internal bitbake that at least would use less memory. It could set
the stage to allow the defval idea I mentioned in a previous mail to happen.

It is a huge change and would need a lot of work to make it happen. Is it worth 
doing? Not sure. I'm putting it out there for discussion.
Is this something we do in parallel with master, and then flag day it? I could
easily see this work take a few weeks (if a bunch of people help).
I think we might be able to block convert much of the metadata with some kind of 
script. If that is possible, we'd have a flag day and a bunch of code tested in 
parallel before a final semi-automated conversion.

It is also going to be tempting that "if we're breaking things, lets break lots".
I want to be mindful that tempting as that may be, if users can't convert clearly,
it will be worlds of pain. The benefit of this change is that at least in the
general case, the transition should be clear. Taking steps rather than changing the
world may therefore be a better idea...
One of those things (variable syntax) I'm not sure 'steps' make sense. If we're
changing a fundamental syntax, it really becomes a new major version of bitbake
and thus a new version of OE.
Having lots of things change at once may make things much harder for layer maintainers
than having a controlled change to handle though?


 EXTRA_OECONF = "--with-perl='${USRBINPATH}/env perl' --with-patch=patch"
-EXTRA_OECONF_append_class-native = " --disable-nls"
+EXTRA_OECONF:append:class-native = " --disable-nls"
So operations (append) and overrides would both be delimited by ':'? It keeps
the ordering and makes it easier to read for sure, but I'm wondering if we need
to deliminate the operation differently. (No I have no suggestions, just a
thought. I don't object to the proposal at all.)
I wondered but this kind of felt neatest to me and it does work nicely with
the code backend handling it...



-RDEPENDS_${PN}-ptest = "make file sed gawk diffutils findutils ed perl \
+RDEPENDS:${PN}-ptest = "make file sed gawk diffutils findutils ed perl \
                         perl-module-filehandle perl-module-getopt-std \
                         perl-module-posix perl-module-file-temp \
                         perl-module-text-parsewords perl-module-overloading \
The above would work without code changes to the packaging class, but maybe we
really should make the change for readability. (I've often wondered if it
really should use 'flag' syntax instead, but lack of overrides make flag syntax
difficult to use.)

i.e.:

-FILES_${PN} = "${sysconfdir} ${datadir}/quilt \
+FILES[${PN}] = "${sysconfdir} ${datadir}/quilt \

or maybe something like? (This is a larger syntax change though)

-RDEPENDS_${PN} = "bash patch diffstat bzip2 util-linux less"
-RDEPENDS_${PN}_class-native = "diffstat-native patch-native bzip2-native"
+RDEPENDS[${PN}] = "bash patch diffstat bzip2 util-linux less"
+RDEPENDS[${PN}]:class-native = "diffstat-native patch-native bzip2-native"
'flags' in bitbake are really a poor cousin to the main variables and behave 
quite differently. Whether flags should have override support is a different 
question. It would hugely complicate the datastore code and I'm not sure the
above is more readable compared to the simpler ":". I've always seen flags as
supplemental data for a variable rather than anything like the above.

Cheers,

Richard

Join openembedded-architecture@lists.openembedded.org to automatically receive all group messages.