Re: [bitbake-devel] [PATCH v2] bitbake-worker: Evaluate exported_vars() before emptying the environment
Richard Purdie <[email protected]> Tue, 28 Jul 2026 22:15:43 +0100
| Newsgroups | org.openembedded.lists.bitbake-devel |
|---|---|
| Message-ID | <8b324e07c0501a6e613fe197aa6e462422360d93.camel@linuxfoundation.org> |
On Mon, 2026-07-27 at 17:28 +0000, Alejandro Mery via lists.openembedded.or=
g wrote:
> 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.
>=20
> 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.
>=20
> 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).
>=20
> 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.
>=20
> - v2: comment the ordering at the call site, and add the tests.
> - v1: the list() wrap alone.
>=20
> Signed-off-by: Alejandro Mery <[email protected]>
> ---
> =C2=A0bin/bitbake-worker=C2=A0=C2=A0 |=C2=A0 5 ++++-
> =C2=A0lib/bb/tests/data.py | 42 +++++++++++++++++++++++++++++++++++++++++=
+
> =C2=A02 files changed, 46 insertions(+), 1 deletion(-)
Thanks for the changes.
I'm worried that the test case doesn't cover what I'd ideally like it
to, specifically that bitbake-worker's environment can't get corrupted.
The code in bitbake-worker can be changed independently of the test
case and break things, which means the test isn't protecting against
what I most fear happening.
I did a bit more research and thinking about this and I think I might
suggest a new approach. I've noticed that this is the only user of
exported_vars() in the codebase, so perhaps we just stop returning a
generator?
It does appear to be causing a lot more problems than it solves,
particularly if we just turn it into a list anyway!
If we change things that way, it seems much more unlikely we can break
it in future and the need for the comments is removed.
Does that seem reasonable? Sorry about the change in direction but this
patch just doesn't look/feel right now I realise there is only one user
of the function.
Cheers,
Richard