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.

(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'.)

> 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).

> 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.

(a few more comments below)

> Cheers,
> 
> Richard
> 
> 
> diff --git a/meta/recipes-devtools/quilt/quilt.inc 
> b/meta/recipes-devtools/quilt/quilt.inc
> index d7ecda7aaa6..f85de384d26 100644
> --- a/meta/recipes-devtools/quilt/quilt.inc
> +++ b/meta/recipes-devtools/quilt/quilt.inc
> @@ -14,36 +14,36 @@ SRC_URI = 
> "${SAVANNAH_GNU_MIRROR}/quilt/quilt-${PV}.tar.gz \
>          file://0001-tests-Allow-different-output-from-mv.patch \
>  "
>
> -SRC_URI_append_class-target = " file://gnu_patch_test_fix_target.patch"
> +SRC_URI:append:class-target = " file://gnu_patch_test_fix_target.patch"
>
>  SRC_URI[md5sum] = "6800c2404a2c0598ab2eff92a636ba70"
>  SRC_URI[sha256sum] = 
> "314b319a6feb13bf9d0f9ffa7ce6683b06919e734a41275087ea457cc9dc6e07"
>
>  inherit autotools-brokensep ptest
>
> -INHIBIT_AUTOTOOLS_DEPS_class-native = "1"
> -PATCHTOOL_class-native = "patch"
> +INHIBIT_AUTOTOOLS_DEPS:class-native = "1"
> +PATCHTOOL:class-native = "patch"
>
>  CLEANBROKEN = "1"
>
>  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.)



>  EXTRA_AUTORECONF += "--exclude=aclocal"
>
>  CACHED_CONFIGUREVARS += "ac_cv_path_BASH=/bin/bash ac_cv_path_COLUMN=column"
>
>  # Make sure we don't have "-w" in shebang lines: it breaks using
>  # "/usr/bin/env perl" as parser
> -do_configure_prepend () {
> +do_configure:prepend () {
>         find ${S} -name "*.in" -exec sed -i -e "1s,^#\!.*@PERL@ -w$,#\! 
> @PERL@\nuse warnings;," {} \;
>  }
>
>  # Don't setup symlinks to host utilities, we don't need them
> -do_configure_append () {
> +do_configure:append () {
>         sed -e 's,^COMPAT_SYMLINKS.*:=.*,COMPAT_SYMLINKS        :=,' -i 
> ${S}/Makefile
>  }
>
> -do_configure_class-native () {
> +do_configure:class-native () {
>      oe_runconf
>  }
>
> @@ -54,7 +54,7 @@ do_install () {
>         rm -rf ${D}/${datadir}/emacs
>  }
>
> -do_install_append_class-native () {
> +do_install:append:class-native () {
>      # Dummy quiltrc file for patch.bbclass
>      install -d ${D}${sysconfdir}/
>      touch ${D}${sysconfdir}/quiltrc
> @@ -75,16 +75,16 @@ do_install_ptest() {
>
>  PACKAGES += "guards guards-doc"
>
> -FILES_${PN} = "${sysconfdir} ${datadir}/quilt \
> +FILES:${PN} = "${sysconfdir} ${datadir}/quilt \
>                 ${bindir}/quilt ${libdir}/quilt"
> -FILES_guards = "${bindir}/guards"
> -FILES_${PN}-doc = "${mandir}/man1/quilt.1 ${docdir}/${BPN}"
> -FILES_guards-doc = "${mandir}/man1/guards.1"
> +FILES:guards = "${bindir}/guards"
> +FILES:${PN}-doc = "${mandir}/man1/quilt.1 ${docdir}/${BPN}"
> +FILES:guards-doc = "${mandir}/man1/guards.1"
>
> -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"
>
> -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"

--Mark

> 
> 
> 
> 
> 
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#1261): 
https://lists.openembedded.org/g/openembedded-architecture/message/1261
Mute This Topic: https://lists.openembedded.org/mt/84225642/21656
Group Owner: [email protected]
Unsubscribe: https://lists.openembedded.org/g/openembedded-architecture/unsub 
[[email protected]]
-=-=-=-=-=-=-=-=-=-=-=-

Reply via email to