[PATCH] lib/bb/utils.py: Do not preserve TERM in the environment

[PATCH] lib/bb/utils.py: Do not preserve TERM in the environment

[Jacob Kroon]

The value of TERM is leaking into OE-Core postinst-useradd-${PN} scripts,
which in turn can optionally be monitored by buildhistory. Prune the value in
order to make the OE-Core buildhistory output more deterministic.

Signed-off-by: Jacob Kroon <jacob.kroon@gmail.com>
---
 lib/bb/utils.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/lib/bb/utils.py b/lib/bb/utils.py
index 5f5767c1..50032e50 100644
--- a/lib/bb/utils.py
+++ b/lib/bb/utils.py
@@ -580,7 +580,6 @@ def preserved_envvars_exported():
         'PATH',
         'PWD',
         'SHELL',
-        'TERM',
         'USER',
         'LC_ALL',
         'BBSERVER',

Re: [bitbake-devel] [PATCH] lib/bb/utils.py: Do not preserve TERM in the environment

[Richard Purdie]

On Mon, 2020-06-15 at 14:22 +0200, Jacob Kroon wrote:
> The value of TERM is leaking into OE-Core postinst-useradd-${PN}
> scripts,
> which in turn can optionally be monitored by buildhistory. Prune the
> value in
> order to make the OE-Core buildhistory output more deterministic.
> 
> Signed-off-by: Jacob Kroon <jacob.kroon@gmail.com>
> ---
>  lib/bb/utils.py | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/lib/bb/utils.py b/lib/bb/utils.py
> index 5f5767c1..50032e50 100644
> --- a/lib/bb/utils.py
> +++ b/lib/bb/utils.py
> @@ -580,7 +580,6 @@ def preserved_envvars_exported():
>          'PATH',
>          'PWD',
>          'SHELL',
> -        'TERM',
>          'USER',
>          'LC_ALL',
>          'BBSERVER',

When you send a patch like this, please be clear that its for testing.
I know from a discussion on IRC that this patch needs close vetting to
see if its "safe" and its likely going to break things around the
menuconfig tasks for example. The patch needs to state that so people's
expectations are right.

Cheers,

Richard

Re: [bitbake-devel] [PATCH] lib/bb/utils.py: Do not preserve TERM in the environment

[Jacob Kroon]

On 6/15/20 2:29 PM, Richard Purdie wrote:
> On Mon, 2020-06-15 at 14:22 +0200, Jacob Kroon wrote:
>> The value of TERM is leaking into OE-Core postinst-useradd-${PN}
>> scripts,
>> which in turn can optionally be monitored by buildhistory. Prune the
>> value in
>> order to make the OE-Core buildhistory output more deterministic.
>>
>> Signed-off-by: Jacob Kroon <jacob.kroon@gmail.com>
>> ---
>>   lib/bb/utils.py | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/lib/bb/utils.py b/lib/bb/utils.py
>> index 5f5767c1..50032e50 100644
>> --- a/lib/bb/utils.py
>> +++ b/lib/bb/utils.py
>> @@ -580,7 +580,6 @@ def preserved_envvars_exported():
>>           'PATH',
>>           'PWD',
>>           'SHELL',
>> -        'TERM',
>>           'USER',
>>           'LC_ALL',
>>           'BBSERVER',
> 
> When you send a patch like this, please be clear that its for testing.
> I know from a discussion on IRC that this patch needs close vetting to
> see if its "safe" and its likely going to break things around the
> menuconfig tasks for example. The patch needs to state that so people's
> expectations are right.
> 

Right, I should have been more upfront with that in the patch. Should 
that go in the commit message itself, after the '---', or just a prefix 
in the email header ?

I understand TERM must be used for *something*, I've just never 
understood in what ways exactly. Is it for allowing 
screen/tmux/xterm/your-favourite-terminal to do extra fancy things ?

At a first glance I can't see anything breaking the kernels menuconfig, 
and in a devshell TERM is back, regardless if I use tmux or plain 
gnome-shell.

Jacob

Re: [bitbake-devel] [PATCH] lib/bb/utils.py: Do not preserve TERM in the environment

[Richard Purdie]

On Tue, 2020-06-16 at 10:14 +0200, Jacob Kroon wrote:
> On 6/15/20 2:29 PM, Richard Purdie wrote:
> > On Mon, 2020-06-15 at 14:22 +0200, Jacob Kroon wrote:
> > > The value of TERM is leaking into OE-Core postinst-useradd-${PN}
> > > scripts,
> > > which in turn can optionally be monitored by buildhistory. Prune
> > > the
> > > value in
> > > order to make the OE-Core buildhistory output more deterministic.
> > > 
> > > Signed-off-by: Jacob Kroon <jacob.kroon@gmail.com>
> > > ---
> > >   lib/bb/utils.py | 1 -
> > >   1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/lib/bb/utils.py b/lib/bb/utils.py
> > > index 5f5767c1..50032e50 100644
> > > --- a/lib/bb/utils.py
> > > +++ b/lib/bb/utils.py
> > > @@ -580,7 +580,6 @@ def preserved_envvars_exported():
> > >           'PATH',
> > >           'PWD',
> > >           'SHELL',
> > > -        'TERM',
> > >           'USER',
> > >           'LC_ALL',
> > >           'BBSERVER',
> > 
> > When you send a patch like this, please be clear that its for
> > testing.
> > I know from a discussion on IRC that this patch needs close vetting
> > to
> > see if its "safe" and its likely going to break things around the
> > menuconfig tasks for example. The patch needs to state that so
> > people's
> > expectations are right.
> > 
> 
> Right, I should have been more upfront with that in the patch.
> Should that go in the commit message itself, after the '---', or just
> a prefix in the email header ?

After the --- would be fine. Its just so everyone has the right
expectations about the patch.

> I understand TERM must be used for *something*, I've just never 
> understood in what ways exactly. Is it for allowing 
> screen/tmux/xterm/your-favourite-terminal to do extra fancy things ?
> 
> At a first glance I can't see anything breaking the kernels
> menuconfig, 
> and in a devshell TERM is back, regardless if I use tmux or plain 
> gnome-shell.

We're not seeing any issues testing this on the autobuilder which is
good. Its part of some very old code, created when we first started to
try and clean up the build environment so its possible other
improvements have obsoleted this need now. Its obviously just had to
test and be sure!

Cheers,

Richard

Re: [bitbake-devel] [PATCH] lib/bb/utils.py: Do not preserve TERM in the environment

[Jacob Kroon]

On 6/16/20 8:48 PM, Richard Purdie wrote:
> On Tue, 2020-06-16 at 10:14 +0200, Jacob Kroon wrote:
>> On 6/15/20 2:29 PM, Richard Purdie wrote:
>>> On Mon, 2020-06-15 at 14:22 +0200, Jacob Kroon wrote:
>>>> The value of TERM is leaking into OE-Core postinst-useradd-${PN}
>>>> scripts,
>>>> which in turn can optionally be monitored by buildhistory. Prune
>>>> the
>>>> value in
>>>> order to make the OE-Core buildhistory output more deterministic.
>>>>
>>>> Signed-off-by: Jacob Kroon <jacob.kroon@gmail.com>
>>>> ---
>>>>    lib/bb/utils.py | 1 -
>>>>    1 file changed, 1 deletion(-)
>>>>
>>>> diff --git a/lib/bb/utils.py b/lib/bb/utils.py
>>>> index 5f5767c1..50032e50 100644
>>>> --- a/lib/bb/utils.py
>>>> +++ b/lib/bb/utils.py
>>>> @@ -580,7 +580,6 @@ def preserved_envvars_exported():
>>>>            'PATH',
>>>>            'PWD',
>>>>            'SHELL',
>>>> -        'TERM',
>>>>            'USER',
>>>>            'LC_ALL',
>>>>            'BBSERVER',
>>>
>>> When you send a patch like this, please be clear that its for
>>> testing.
>>> I know from a discussion on IRC that this patch needs close vetting
>>> to
>>> see if its "safe" and its likely going to break things around the
>>> menuconfig tasks for example. The patch needs to state that so
>>> people's
>>> expectations are right.
>>>
>>
>> Right, I should have been more upfront with that in the patch.
>> Should that go in the commit message itself, after the '---', or just
>> a prefix in the email header ?
> 
> After the --- would be fine. Its just so everyone has the right
> expectations about the patch.
> 
>> I understand TERM must be used for *something*, I've just never
>> understood in what ways exactly. Is it for allowing
>> screen/tmux/xterm/your-favourite-terminal to do extra fancy things ?
>>
>> At a first glance I can't see anything breaking the kernels
>> menuconfig,
>> and in a devshell TERM is back, regardless if I use tmux or plain
>> gnome-shell.
> 
> We're not seeing any issues testing this on the autobuilder which is
> good. Its part of some very old code, created when we first started to
> try and clean up the build environment so its possible other
> improvements have obsoleted this need now. Its obviously just had to
> test and be sure!
> 

If we decide to drop it as the patch suggests, then I guess we could 
drop it from BB_HASHBASE_WHITELIST in oe-core/meta/conf/bitbake.conf as 
well. (There is also a reference to 'TERM' in the bitbake manual which 
should also be removed in that case)

Jacob

Re: [bitbake-devel] [PATCH] lib/bb/utils.py: Do not preserve TERM in the environment

[Richard Purdie]

On Tue, 2020-06-16 at 22:38 +0200, Jacob Kroon wrote:
> On 6/16/20 8:48 PM, Richard Purdie wrote:
> > We're not seeing any issues testing this on the autobuilder which
> > is good. Its part of some very old code, created when we first
> > started to try and clean up the build environment so its possible
> > other improvements have obsoleted this need now. Its obviously just
> > had to test and be sure!
> > 
> 
> If we decide to drop it as the patch suggests, then I guess we could 
> drop it from BB_HASHBASE_WHITELIST in oe-core/meta/conf/bitbake.conf
> as  well. (There is also a reference to 'TERM' in the bitbake manual
> which should also be removed in that case)

I'm about to merge this so if you could send the follow up patches that
would be great, thanks!

Cheers,

Richard