| Message ID | 20260720195834.2496647-1-amery@apptly.co |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [2.18] bitbake-worker: Evaluate exported_vars() before emptying the environment | expand |
On Mon, 2026-07-20 at 19:58 +0000, Alejandro Mery via lists.openembedded.org 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 with > PATH already wiped. > > 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 PATH is still intact. > > Signed-off-by: Alejandro Mery <amery@apptly.co> > --- > bin/bitbake-worker | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/bin/bitbake-worker b/bin/bitbake-worker > index aa14ef191..0db1531e0 100755 > --- a/bin/bitbake-worker > +++ b/bin/bitbake-worker > @@ -290,7 +290,7 @@ 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) > + exports = list(bb.data.exported_vars(the_data)) > > bb.utils.empty_environment() > for e, v in exports: Thanks for this. Nice work in tracking it down! I think in this case we need a couple of things, firstly will be a bit more explination in the code with a comment about why we need a list there. The failure case is so unusual we should also probably write a test case to make sure we test for and don't regress that issue which we'd add to the existing tests in bitbake-selftest. Help with that would be welcome or we'll get to it when we can... Cheers, Richard
On 23/07/2026 07:06, Richard Purdie wrote: > On Mon, 2026-07-20 at 19:58 +0000, Alejandro Mery via lists.openembedded.org 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 with >> PATH already wiped. >> >> 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 PATH is still intact. >> >> Signed-off-by: Alejandro Mery <amery@apptly.co> >> --- >> bin/bitbake-worker | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/bin/bitbake-worker b/bin/bitbake-worker >> index aa14ef191..0db1531e0 100755 >> --- a/bin/bitbake-worker >> +++ b/bin/bitbake-worker >> @@ -290,7 +290,7 @@ 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) >> + exports = list(bb.data.exported_vars(the_data)) >> >> bb.utils.empty_environment() >> for e, v in exports: > Thanks for this. Nice work in tracking it down! > > I think in this case we need a couple of things, firstly will be a bit > more explination in the code with a comment about why we need a list > there. > > The failure case is so unusual we should also probably write a test > case to make sure we test for and don't regress that issue which we'd > add to the existing tests in bitbake-selftest. > > Help with that would be welcome or we'll get to it when we can... I'll send a v2 soon
Le sam. 25 juil. 2026, 20:01, Alejandro Mery via lists.openembedded.org <amery=apptly.co@lists.openembedded.org> a écrit : > > On 23/07/2026 07:06, Richard Purdie wrote: > > On Mon, 2026-07-20 at 19:58 +0000, Alejandro Mery via > lists.openembedded.org 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 with > >> PATH already wiped. > >> > >> 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 PATH is still intact. > >> > >> Signed-off-by: Alejandro Mery <amery@apptly.co> > >> --- > >> bin/bitbake-worker | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/bin/bitbake-worker b/bin/bitbake-worker > >> index aa14ef191..0db1531e0 100755 > >> --- a/bin/bitbake-worker > >> +++ b/bin/bitbake-worker > >> @@ -290,7 +290,7 @@ 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) > >> + exports = list(bb.data.exported_vars(the_data)) > >> > >> bb.utils.empty_environment() > >> for e, v in exports: > > Thanks for this. Nice work in tracking it down! > > > > I think in this case we need a couple of things, firstly will be a bit > > more explination in the code with a comment about why we need a list > > there. > > > > The failure case is so unusual we should also probably write a test > > case to make sure we test for and don't regress that issue which we'd > > add to the existing tests in bitbake-selftest. > > > > Help with that would be welcome or we'll get to it when we can... > > > I'll send a v2 soon > Is this for master? In that case, please remove the [2.18] tag. Thanks!
diff --git a/bin/bitbake-worker b/bin/bitbake-worker index aa14ef191..0db1531e0 100755 --- a/bin/bitbake-worker +++ b/bin/bitbake-worker @@ -290,7 +290,7 @@ 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) + exports = list(bb.data.exported_vars(the_data)) bb.utils.empty_environment() for e, v in exports:
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 with PATH already wiped. 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 PATH is still intact. Signed-off-by: Alejandro Mery <amery@apptly.co> --- bin/bitbake-worker | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)