diff mbox series

[v2] bitbake-worker: Evaluate exported_vars() before emptying the environment

Message ID 20260727172856.2304118-1-amery@apptly.co
State New
Headers show
Series [v2] bitbake-worker: Evaluate exported_vars() before emptying the environment | expand

Commit Message

Alejandro Mery July 27, 2026, 5:28 p.m. UTC
exported_vars() returns a lazy generator. It was bound before
empty_environment() cleared the process environment and only iterated
afterwards, so any datastore expansion deferred until iteration ran
against the wiped environment rather than the one bitbake started with.

The corruption is not uniform. The loop 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).

Materialise the generator into a list before emptying the environment so
every expansion happens while that environment is still intact. Add
tests for the expansion timing this relies on: values materialised
before empty_environment() survive it, a generator consumed after it
does not.

- 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   |  5 ++++-
 lib/bb/tests/data.py | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/bin/bitbake-worker b/bin/bitbake-worker
index 4ad716975..b8dc9c862 100755
--- a/bin/bitbake-worker
+++ b/bin/bitbake-worker
@@ -296,7 +296,10 @@  def fork_off_task(cfg, data, databuilder, workerdata, extraconfigdata, runtask):
 
                 # 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 
-                exports = bb.data.exported_vars(the_data)
+                # Iterating it expands each value, so materialise the generator before
+                # empty_environment() below: an expansion deferred past that point runs
+                # against the wiped environment, not the one bitbake started with.
+                exports = list(bb.data.exported_vars(the_data))
 
                 bb.utils.empty_environment()
                 for e, v in exports:
diff --git a/lib/bb/tests/data.py b/lib/bb/tests/data.py
index fd690a9e2..06c88ffc6 100644
--- a/lib/bb/tests/data.py
+++ b/lib/bb/tests/data.py
@@ -723,3 +723,45 @@  class EmitVar(unittest.TestCase):
         self.assertEqual(self.get_output(out), ['bad_chars="a\\"b \\',
                                                 'c\\`d \\',
                                                 'e\\$f"'])
+
+class ExportedVars(unittest.TestCase):
+    """bb.data.exported_vars() expands each value as it is iterated.
+
+    bitbake-worker's fork_off_task() therefore materialises the generator
+    with list() before it calls bb.utils.empty_environment(): an exported
+    variable whose value expands from the environment (e.g. a gitver-style
+    PV that runs git and needs the real PATH) has to be expanded while that
+    environment still exists. bitbake-worker runs on import (only its
+    version check sits behind a __main__ guard), so these tests reproduce
+    its sequence against bb.data and pin the behaviour the worker relies
+    on, not the worker itself.
+    """
+
+    def setUp(self):
+        self.saved_environment = dict(os.environ)
+        os.environ["BB_TEST_EXPORTED_VARS"] = "intact"
+
+        self.d = bb.data.init()
+        self.d.setVar("TESTEXPORT",
+                      "${@os.environ.get('BB_TEST_EXPORTED_VARS', 'wiped')}")
+        self.d.setVarFlag("TESTEXPORT", "export", "1")
+
+    def tearDown(self):
+        # empty_environment() wipes the environment of this process, not a
+        # copy of it, so put it back whatever the test did.
+        os.environ.clear()
+        os.environ.update(self.saved_environment)
+
+    def test_materialised_before_empty_environment(self):
+        # What bitbake-worker does: list() expands every value while the
+        # environment is still intact, so the wipe cannot reach them.
+        exports = list(bb.data.exported_vars(self.d))
+        bb.utils.empty_environment()
+        self.assertEqual(dict(exports)["TESTEXPORT"], "intact")
+
+    def test_lazy_generator_expands_after_empty_environment(self):
+        # Why the list() is required: a generator bound before the wipe but
+        # consumed after it expands against the wiped environment instead.
+        exports = bb.data.exported_vars(self.d)
+        bb.utils.empty_environment()
+        self.assertEqual(dict(exports)["TESTEXPORT"], "wiped")