Re: [PATCH] oe-selftest: Improve BUILDDIR environment handling

[Ed Bartosh <ed.bartosh@linux.intel.com>]

On Thu, Jan 05, 2017 at 10:10:50AM +0000, Richard Purdie wrote:
> On Thu, 2017-01-05 at 11:40 +0200, Ed Bartosh wrote:
> > On Wed, Jan 04, 2017 at 11:48:53PM +0000, Richard Purdie wrote:
> > > 
> > > Its possible something (like bitbake/tinfoil2) may mess around with
> > > the
> > > environment and using the enviroment as a global variable store
> > > isn't
> > > particularly nice anyway.
> > > 
> > > This patch changes the BUILDDIR usages so that the environment
> > > isn't used
> > > as a global store and a global variable is used instead. Whilst
> > > that
> > > is still not perfect, it does avoid the current double and triple
> > > backtraces
> > > we're seeing where tinfoil2/bitbake has trampled the enviroment
> > > leading
> > > to failures of failures making debugging even harder.
> > > 
> > > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
> > > ---
> > >  scripts/oe-selftest | 19 +++++++++++++------
> > >  1 file changed, 13 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/scripts/oe-selftest b/scripts/oe-selftest
> > > index bfcea66..e166521 100755
> > > --- a/scripts/oe-selftest
> > > +++ b/scripts/oe-selftest
> > > @@ -111,9 +111,13 @@ def get_args_parser():
> > >                          help='Submit test results to a
> > > repository')
> > >      return parser
> > >  
> > > +builddir = None
> > > +
> > >  
> > >  def preflight_check():
> > >  
> > > +    global builddir
> > > +
> > >      log.info("Checking that everything is in order before running
> > > the tests")
> > >  
> > >      if not os.environ.get("BUILDDIR"):
> > > @@ -135,7 +139,7 @@ def preflight_check():
> > >      return True
> > >  
> > >  def add_include():
> > > -    builddir = os.environ.get("BUILDDIR")
> > > +    global builddir
> > You don't need to use 'global' here. It's only mandatory if
> > you change variable value:
> > https://docs.python.org/3/reference/simple_stmts.html#the-global-stat
> > ement
> 
> In this case I did it purely for readability to make it clear how we
> were expecting it to work and to match the other uses. It doesn't hurt
> anything afaik.
> 
> > Would it be more readable to use name in upper case: BUILDDIR?
> 
> Not sure, that isn't something we've used as a style anywhere else so
> it doesn't really match any other style. Is that a python convention?
No, it's not. I thought it would be more readable as uppercase name
is the same as environment variable name. And it would be different
from local variable names, so you wouldn't need to use 'global'.

--
Regards,
Ed