diff mbox series

[wrynose,2.18,1/4] data: Return a list from exported_vars()

Message ID ec9d2aacc2b3f3b9189a7f55c53abdcd7e5e4964.1788939009.git.yoann.congal@smile.fr
State New
Headers show
Series [wrynose,2.18,1/4] data: Return a list from exported_vars() | expand

Commit Message

Yoann Congal Sept. 9, 2026, 7:30 a.m. UTC 
From: Alejandro Mery <amery@apptly.co>

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.

Signed-off-by: Alejandro Mery <amery@apptly.co>
Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
(cherry picked from commit c870f5bd96ad02efc02c58133c27bdae3a300e3e)
Signed-off-by: Alejandro Mery <amery@apptly.co>
Signed-off-by: Yoann Congal <yoann.congal@smile.fr>
[YC: removed patch history from the commit message]
---
 bin/bitbake-worker   |  6 ++++--
 lib/bb/data.py       | 10 +++++++++-
 lib/bb/tests/data.py | 28 ++++++++++++++++++++++++++++
 3 files changed, 41 insertions(+), 3 deletions(-)
diff mbox series

Patch

diff --git a/bin/bitbake-worker b/bin/bitbake-worker
index aa14ef191..ae21f277f 100755
--- a/bin/bitbake-worker
+++ b/bin/bitbake-worker
@@ -288,8 +288,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()
diff --git a/lib/bb/data.py b/lib/bb/data.py
index b12972c03..7c01e6bf8 100644
--- a/lib/bb/data.py
+++ b/lib/bb/data.py
@@ -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."""
diff --git a/lib/bb/tests/data.py b/lib/bb/tests/data.py
index fd690a9e2..a83c71e2c 100644
--- a/lib/bb/tests/data.py
+++ b/lib/bb/tests/data.py
@@ -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))