[OE-core] [PATCH v2 1/2] bitbake.conf: DISTRO_FEATURES as overrides

[Patrick Ohly]

On Mon, 2017-06-12 at 19:23 -0400, Denys Dmytriyenko wrote:
> On Mon, Jun 12, 2017 at 11:05:19PM +0200, Patrick Ohly wrote:
> > On Mon, 2017-06-12 at 15:46 -0400, Denys Dmytriyenko wrote:
> > > This now breaks parsing my distro config on these lines:
> > > 
> > > ENABLE_SYSVINIT ?= "0"
> > > DISTRO_FEATURES_append = "${@base_conditional("ENABLE_SYSVINIT", "1", "", " systemd", d)}"
> > > 
> > > 
> > > Here's the log:
> > > 
> > > ERROR: Unable to parse /OE/arago-master/sources/bitbake/lib/bb/data_smart.py
> > > Traceback (most recent call last):
> > >   File "/OE/arago-master/sources/bitbake/lib/bb/data_smart.py", line 426, in DataSmart.expandWithRefs(s='${@base_conditional("ENABLE_SYSVINIT", "1", "", " systemd", d)}', varname='DISTRO_FEATURES_append'):
> > >                  except Exception as exc:
> > >     >                raise ExpansionError(varname, s, exc) from exc
> > >      
> > > bb.data_smart.ExpansionError: Failure expanding variable DISTRO_FEATURES_append, expression was ${@base_conditional("ENABLE_SYSVINIT", "1", "", " systemd", d)} which triggered exception NameError: name 'base_conditional' is not defined
> > 
> > base_conditional() seems to come from utils.bbclass, which gets
> > inherited by base.bbclass. Looks like DISTRO_FEATURES and thus this
> > DISTRO_FEATURES_append end up getting expanded before these classes are
> > fully parsed.
> 
> FWIW, replacing it with oe.utils.conditional() doesn't help.

How about the following patch? It solves the problem for me.

It also addresses some unease that I had when Richard proposed the
distro overrides approach (cyclic dependency of "DISTRO_FEATURES depends
on OVERRIDES which depends on DISTRO_FEATURES") by setting the overrides
once for the DISTRO_FEATURES as they are set at the end of the base
configuration. That's more deterministic; previously OVERRIDES could
basically change randomly during base configuration parsing.

I couldn't give a good example for this cyclic dependency at the time.
Let me add that now.

This code has an obvious cyclic dependency, and it indeed fails as I had
expected:

DISTRO_FEATURES_append = " ${@ bb.utils.contains('DISTRO_FEATURES', 'foo', 'bar', '', d) }"
DISTRO_FEATURES_append = " foo"

=> bb.data_smart.ExpansionError: Failure expanding variable
DISTRO_FEATURES, expression was ${@ bb.utils.contains('DISTRO_FEATURES',
'foo', 'bar', '', d) } which triggered exception RuntimeError: maximum
recursion depth exceeded

In this code, the cyclic dependency is a bit more hidden, and depending
on when things get evaluated, one gets foo and bar in DISTRO_FEATURES
(for example, in "bitbake -e" and thus recipes):
DISTRO_FEATURES_OVERRIDES += "bar"
DISTRO_FEATURES_append = " bar"
DISTRO_FEATURES_append_df-bar = " foo"

That last example still works with the patch below, but I would argue
that because of the conceptual issue (DISTRO_FEATURES depending on
content of DISTRO_FEATURES) it cannot be expected to work.

Once things get more complicated, it just fails in other unexpected
ways:
DISTRO_FEATURES_OVERRIDES += "bar foo"
DISTRO_FEATURES_append = " bar"
DISTRO_FEATURES_append_df-bar = " foo"

This adds bar and foo to DISTRO_FEATURES, but only bar ends up in
OVERRIDES when using the patch below. It was working with code
that sets distro overrides dynamically. This might be seen as a drawback
of the approach below, but as it only affects code that (IMHO) isn't
valid, I'm not too worried about that.

Let me also point out that the approach below completely sidesteps
potential vardeps problems, because the dependency of OVERRIDES on
DISTRO_FEATURES_OVERRIDES and DISTRO_FEATURES is hidden. My expectation
is that OVERRIDES are part of the base hash and that thus tracking of
how it gets computed isn't necessary. It seems to work in practice for
me:

DISTRO_FEATURES_append = " systemd"
DISTRO_FEATURES_OVERRIDES += "bar"
DISTRO_FEATURES_append = " bar"
PACKAGECONFIG_append_pn-systemd_df-bar = " gcrypt"

Changing either DISTRO_FEATURES_OVERRIDES or the DISTRO_FEATURES_append
changes the signature of systemd:do_configure as expected.

We are currently having a problem with the yocto-compat-layer.py
signature check in refkit where it complains about:
        List of dependencies for variable DISTROOVERRIDES changed from
        '{'DISTRO'}' to '{'DISTRO_FEATURES', 'DISTRO',
        'DISTROFEATURES2OVERRIDES'}'

That's for a slightly older OE-core and the distrooverrides.bbclass
approach. I don't know yet why that is now failing (it was working for
OE-core 2.3), but my expectation is that the approach below would avoid
it.

diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
index 78926656d77..356aeba72ad 100644
--- a/meta/classes/base.bbclass
+++ b/meta/classes/base.bbclass
@@ -234,6 +234,11 @@ python base_eventhandler() {
         # Works with the line in layer.conf which changes PATH to point here
         setup_hosttools_dir(d.getVar('HOSTTOOLS_DIR'), 'HOSTTOOLS', d)
         setup_hosttools_dir(d.getVar('HOSTTOOLS_DIR'), 'HOSTTOOLS_NONFATAL', d, fatal=False)
+        # Base configuration complete, DISTRO_FEATURES finalized => convert selected features
+        # into overrides.
+        e.data.setVar('DISTROFEATURESOVERRIDES',
+                      ''.join([':df-' + x for x in sorted(set(d.getVar('DISTRO_FEATURES_OVERRIDES').split()) & set(d.getVar('DISTRO_FEATURES').split()))]))
+
 
     if isinstance(e, bb.event.BuildStarted):
         localdata = bb.data.createCopy(e.data)
diff --git a/meta/conf/bitbake.conf b/meta/conf/bitbake.conf
index bc438cca826..6069130f488 100644
--- a/meta/conf/bitbake.conf
+++ b/meta/conf/bitbake.conf
@@ -727,15 +727,23 @@ FILESOVERRIDES = "${TRANSLATED_TARGET_ARCH}:${MACHINEOVERRIDES}:${DISTROOVERRIDE
 # distro features remain set also for native and nativesdk
 # recipes, so that these overrides can also be used there.
 #
-# Beware that this part of OVERRIDES changes during parsing, so usage
-# of these overrides should be limited to .bb and .bbappend files,
-# because then DISTRO_FEATURES is final.
+# Beware that DISTRO_FEATURES change during parsing of the
+# base configuration. These overrides can be used when setting
+# variables in the base configuration, but variable expansion
+# will only have the desired effect in recipes.
+#
+# Even worse, DISTRO_FEATURES might not be even usable during base
+# configuration parsing. For example, embedded Python code with calls
+# to base_conditional() can fail because the function is not defined.
+# yet. Therefore these conditionals are set by base.bbclass only after
+# parsing the base configuration is complete.
 DISTRO_FEATURES_OVERRIDES ??= ""
 DISTRO_FEATURES_OVERRIDES[doc] = "A space-separated list of <feature> entries. \
 Each entry is added to OVERRIDES as df-<feature> if <feature> is in DISTRO_FEATURES."
 DISTRO_FEATURES_FILTER_NATIVE_append = " ${DISTRO_FEATURES_OVERRIDES}"
 DISTRO_FEATURES_FILTER_NATIVESDK_append = " ${DISTRO_FEATURES_OVERRIDES}"
-DISTROFEATURESOVERRIDES = "${@ ''.join([':df-' + x for x in (set(d.getVar('DISTRO_FEATURES_OVERRIDES').split()) & set((d.getVar('DISTRO_FEATURES') or '').split()))]) }"
+# Initially empty, set once when DISTRO_FEATURES is stable.
+DISTROFEATURESOVERRIDES = ""
 
 ##################################################################
 # Include the rest of the config files.

-- 
Best Regards, Patrick Ohly

The content of this message is my personal opinion only and although
I am an employee of Intel, the statements I make here in no way
represent Intel's position on the issue, nor am I authorized to speak
on behalf of Intel on this matter.