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

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

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-statement

Would it be more readable to use name in upper case: BUILDDIR?

>      if "#include added by oe-selftest.py" \
>          not in ftools.read_file(os.path.join(builddir, "conf/local.conf")):
>              log.info("Adding: \"include selftest.inc\" in local.conf")
> @@ -149,7 +153,7 @@ def add_include():
>                      "\n#include added by oe-selftest.py\ninclude bblayers.inc")
>  
>  def remove_include():
> -    builddir = os.environ.get("BUILDDIR")
> +    global builddir
>      if builddir is None:
>          return
>      if "#include added by oe-selftest.py" \
> @@ -165,18 +169,21 @@ def remove_include():
>                      "\n#include added by oe-selftest.py\ninclude bblayers.inc")
>  
>  def remove_inc_files():
> +    global builddir
> +    if builddir is None:
> +        return
>      try:
> -        os.remove(os.path.join(os.environ.get("BUILDDIR"), "conf/selftest.inc"))
> +        os.remove(os.path.join(builddir, "conf/selftest.inc"))
>          for root, _, files in os.walk(get_test_layer()):
>              for f in files:
>                  if f == 'test_recipe.inc':
>                      os.remove(os.path.join(root, f))
> -    except (AttributeError, OSError,) as e:    # AttributeError may happen if BUILDDIR is not set
> +    except OSError as e:
>          pass
>  
>      for incl_file in ['conf/bblayers.inc', 'conf/machine.inc']:
>          try:
> -            os.remove(os.path.join(os.environ.get("BUILDDIR"), incl_file))
> +            os.remove(os.path.join(builddir, incl_file))
>          except:
>              pass
>  
> @@ -394,7 +401,7 @@ def coverage_setup(coverage_source, coverage_include, coverage_omit):
>      """ Set up the coverage measurement for the testcases to be run """
>      import datetime
>      import subprocess
> -    builddir = os.environ.get("BUILDDIR")
> +    global builddir
>      pokydir = os.path.dirname(os.path.dirname(os.path.realpath(__file__)))
>      curcommit= subprocess.check_output(["git", "--git-dir", os.path.join(pokydir, ".git"), "rev-parse", "HEAD"]).decode('utf-8')
>      coveragerc = "%s/.coveragerc" % builddir
> -- 
> 2.7.4
> 
> -- 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.openembedded.org/mailman/listinfo/openembedded-core

-- 
--
Regards,
Ed