Re: [OE-core] [PATCH] oe-selftest: Improve BUILDDIR environment handling
On Thu, Jan 05, 2017 at 10:10:50AM +, Richard Purdie wrote: > On Thu, 2017-01-05 at 11:40 +0200, Ed Bartosh wrote: > > On Wed, Jan 04, 2017 at 11:48:53PM +, 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> > > --- > > > 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 -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] oe-selftest: Improve BUILDDIR environment handling
On Thu, 2017-01-05 at 11:40 +0200, Ed Bartosh wrote: > On Wed, Jan 04, 2017 at 11:48:53PM +, 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> > --- > > 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? Cheers, Richard -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
Re: [OE-core] [PATCH] oe-selftest: Improve BUILDDIR environment handling
On Wed, Jan 04, 2017 at 11:48:53PM +, 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> --- > 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 -- ___ Openembedded-core mailing list Openembedded-core@lists.openembedded.org http://lists.openembedded.org/mailman/listinfo/openembedded-core
[OE-core] [PATCH] oe-selftest: Improve BUILDDIR environment handling
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--- 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 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