@@ -294,8 +294,10 @@ def fork_off_task(cfg, data, databuilder, workerdata, extraconfigdata, runtask):
else:
logger.debug("Skipping disable network for %s since %s is not a local uid." % (taskname, uid))
- # exported_vars() returns a generator which *cannot* be passed to os.environ.update()
- # successfully. We also need to unset anything from the environment which shouldn't be there
+ # exported_vars() expands each value, so it must be called before
+ # empty_environment() below: an expansion that shells out needs the
+ # environment bitbake was started with. We also need to unset
+ # anything from the environment which shouldn't be there
exports = bb.data.exported_vars(the_data)
bb.utils.empty_environment()
@@ -193,6 +193,12 @@ def exported_keys(d):
not bb.utils.to_boolean(d.getVarFlag(key, 'unexport')))
def exported_vars(d):
+ """Return the exported variables as a list of (key, value) pairs.
+
+ Every value is expanded before returning, so a caller that changes the
+ environment afterwards still gets what the expansion saw.
+ """
+ exported = []
k = list(exported_keys(d))
for key in k:
try:
@@ -202,7 +208,9 @@ def exported_vars(d):
continue
if value is not None:
- yield key, str(value)
+ exported.append((key, str(value)))
+
+ return exported
def emit_func(func, o=sys.__stdout__, d = init()):
"""Emits all items in the data store in a format such that it can be sourced by a shell."""
@@ -723,3 +723,31 @@ class EmitVar(unittest.TestCase):
self.assertEqual(self.get_output(out), ['bad_chars="a\\"b \\',
'c\\`d \\',
'e\\$f"'])
+
+class ExportedVars(unittest.TestCase):
+ def test_expanded_before_returning(self):
+ # Called while the variable is set, read once it is gone: the value
+ # has to be the one from the call, not from the read.
+ d = bb.data.init()
+ d.setVar("TESTVAR", "${@os.environ.get('BB_TEST_EXPORT', 'gone')}")
+ d.setVarFlag("TESTVAR", "export", "1")
+
+ with bb.utils.environment(BB_TEST_EXPORT="present"):
+ exported = bb.data.exported_vars(d)
+
+ self.assertEqual(dict(exported), {"TESTVAR": "present"})
+ self.assertIsInstance(exported, list)
+
+ def test_unexpandable_value_warns_and_is_skipped(self):
+ # The warning belongs to the call, not to a later iteration.
+ d = bb.data.init()
+ d.setVar("TESTVAR", "value")
+ d.setVarFlag("TESTVAR", "export", "1")
+ d.setVar("TESTBROKEN", "${@int('not a number')}")
+ d.setVarFlag("TESTBROKEN", "export", "1")
+
+ with LogRecord() as logs:
+ exported = bb.data.exported_vars(d)
+
+ self.assertEqual(dict(exported), {"TESTVAR": "value"})
+ self.assertTrue(logContains("Unable to export ${TESTBROKEN}", logs))
exported_vars() was a generator, so each value expanded when the result was iterated rather than when it was called. bitbake-worker binds it before bb.utils.empty_environment() and iterates it afterwards, so any expansion deferred until iteration ran against the wiped environment rather than the one bitbake started with. The corruption is not uniform. The worker writes each variable into os.environ as it goes, so only the first value expands against a fully empty environment and the rest see it partially rebuilt, in exported_keys() order. Recipes whose exported variables expand a command during that loop hit this. A gitver-style PV such as "${@get_git_pv(d, ...)}" runs git while PATH is empty, so the git wrapper on PATH is bypassed and the real git runs directly. Under pseudo this fakes uid 0 against a repository owned by the real user, and git aborts with "detected dubious ownership", failing do_package intermittently (only on reparse, when the value is re-expanded rather than served from cache). Build and return the list in exported_vars() itself, so the expansion is complete before it returns and no caller has to know it was ever deferred. A value that fails to expand now warns at that point rather than during iteration, and a result nothing consumes is expanded anyway; neither is a change for the one caller, which consumes all of it. Update the comment at the worker's call site, which documented the generator that is now gone, and add tests for the expansion timing and for the warning that moved with it. - v3: materialise in exported_vars() itself, not at the call site; two tests, using bb.utils.environment() rather than touching os.environ directly. - v2: comment the ordering at the call site, and add the tests. - v1: the list() wrap alone. Signed-off-by: Alejandro Mery <amery@apptly.co> --- bin/bitbake-worker | 6 ++++-- lib/bb/data.py | 10 +++++++++- lib/bb/tests/data.py | 28 ++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 3 deletions(-)