devtool fixes

PATCH In review Paul Eggleton 2 participants 6 messages Sep 23, 2015

Share & tools

Paul Eggleton

author

2015-09-23 10:05 UTC #0 lore raw

v2 cover devtool fixes

A couple of patches Markus previously sent rebased on top of master (also applicable on top of poky master-next) to fix some minor issues in devtool, plus one minor optimisation from me.

The following changes since commit 2ad7308ee7166641eff99f3b9fe6794de143f6bc:

oeqa/utils/qemurunner.py: Remove duplicate message on LoggingThread start (2015-09-22 18:13:02 +0100)

are available in the git repository at:

git://git.openembedded.org/openembedded-core-contrib paule/devtool-fixes6
http://cgit.openembedded.org/cgit.cgi/openembedded-core-contrib/log/?h=paule/devtool-fixes6

Markus Lehtonen (2):
devtool: second fix for running from a different directory
devtool: upgrade: use shutil.move instead of os.rename

Paul Eggleton (1):
devtool: runqemu: avoid recipe parse

 scripts/devtool                    |  5 +----
 scripts/lib/devtool/__init__.py    |  6 +++++-
 scripts/lib/devtool/build-image.py |  2 +-
 scripts/lib/devtool/deploy.py      |  2 +-
 scripts/lib/devtool/package.py     |  2 +-
 scripts/lib/devtool/runqemu.py     |  2 +-
 scripts/lib/devtool/search.py      |  2 +-
 scripts/lib/devtool/standard.py    | 10 +++++-----
 scripts/lib/devtool/upgrade.py     |  4 ++--
 9 files changed, 18 insertions(+), 17 deletions(-)

Signature · 1 line

2.1.0

Paul Eggleton

author

2015-09-23 10:05 UTC #1 lore raw

PATCH v2 1/3 devtool: second fix for running from a different directory

From: Markus Lehtonen <[email protected]>

Do not change change current working directory permanently, but, only for the duration of tinfoil initialization instead. The previous fix caused very unintuitive behavior where using relative paths were solved with respect to the builddir instead of the current working directory. E.g. calling "devtool extract zlib ./zlib" would always create create srctree in ${TOPDIR}/zlib, independent of the users cwd.

(From OE-Core rev: 4c7f159b0e17a0475a4a4e9dc4dd012e3d2e6a1f)

Signed-off-by Markus Lehtonen <[email protected]>

Signed-off-by Ross Burton <[email protected]>

Signed-off-by Paul Eggleton <[email protected]>

---
 scripts/devtool                    |  5 +----
 scripts/lib/devtool/__init__.py    |  6 +++++-
 scripts/lib/devtool/build-image.py |  2 +-
 scripts/lib/devtool/deploy.py      |  2 +-
 scripts/lib/devtool/package.py     |  2 +-
 scripts/lib/devtool/runqemu.py     |  2 +-
 scripts/lib/devtool/search.py      |  2 +-
 scripts/lib/devtool/standard.py    | 10 +++++-----
 scripts/lib/devtool/upgrade.py     |  2 +-
 9 files changed, 17 insertions(+), 16 deletions(-)

scripts/devtool +1 -4

index 87df951..e4d9db3 100755
@@ -221,9 +221,6 @@ def main():
     if not config.read():
         return -1
 
-    # We need to be in this directory or we won't be able to initialise tinfoil
-    os.chdir(basepath)
-
     bitbake_subdir = config.get('General', 'bitbake_subdir', '')
     if bitbake_subdir:
         # Normally set for use within the SDK

@@ -244,7 +241,7 @@ def main():
     scriptutils.logger_setup_color(logger, global_args.color)
 
     if global_args.bbpath is None:
-        tinfoil = setup_tinfoil(config_only=True)
+        tinfoil = setup_tinfoil(config_only=True, basepath=basepath)
         global_args.bbpath = tinfoil.config_data.getVar('BBPATH', True)
     else:
         tinfoil = None

scripts/lib/devtool/__init__.py +5 -1

index f815ef2..7b1ab11 100644
@@ -96,9 +96,12 @@ def exec_fakeroot(d, cmd, **kwargs):
             newenv[splitval[0]] = splitval[1]
     return subprocess.call("%s %s" % (fakerootcmd, cmd), env=newenv, **kwargs)
 
-def setup_tinfoil(config_only=False):
+def setup_tinfoil(config_only=False, basepath=None):
     """Initialize tinfoil api from bitbake"""
     import scriptpath
+    orig_cwd = os.path.abspath(os.curdir)
+    if basepath:
+        os.chdir(basepath)
     bitbakepath = scriptpath.add_bitbake_lib_path()
     if not bitbakepath:
         logger.error("Unable to find bitbake by searching parent directory of this script or PATH")

@@ -108,6 +111,7 @@ def setup_tinfoil(config_only=False):
     tinfoil = bb.tinfoil.Tinfoil()
     tinfoil.prepare(config_only)
     tinfoil.logger.setLevel(logger.getEffectiveLevel())
+    os.chdir(orig_cwd)
     return tinfoil
 
 def get_recipe_file(cooker, pn):

scripts/lib/devtool/build-image.py +1 -1

index 05c1d75..e53239d 100644
@@ -59,7 +59,7 @@ def build_image(args, config, basepath, workspace):
     if os.path.isfile(appendfile):
         os.unlink(appendfile)
 
-    tinfoil = setup_tinfoil()
+    tinfoil = setup_tinfoil(basepath=basepath)
     rd = parse_recipe(config, tinfoil, image, True)
     if not rd:
         # Error already shown

scripts/lib/devtool/deploy.py +1 -1

index 41b666f..c90c6b1 100644
@@ -40,7 +40,7 @@ def deploy(args, config, basepath, workspace):
     deploy_dir = os.path.join(basepath, 'target_deploy', args.target)
     deploy_file = os.path.join(deploy_dir, args.recipename + '.list')
 
-    tinfoil = setup_tinfoil()
+    tinfoil = setup_tinfoil(basepath=basepath)
     try:
         rd = oe.recipeutils.parse_recipe_simple(tinfoil.cooker, args.recipename, tinfoil.config_data)
     except Exception as e:

scripts/lib/devtool/package.py +1 -1

index 28ecfed..b8d8423 100644
@@ -34,7 +34,7 @@ def package(args, config, basepath, workspace):
 
     image_pkgtype = config.get('Package', 'image_pkgtype', '')
     if not image_pkgtype:
-        tinfoil = setup_tinfoil()
+        tinfoil = setup_tinfoil(basepath=basepath)
         try:
             tinfoil.prepare(config_only=True)
             image_pkgtype = tinfoil.config_data.getVar('IMAGE_PKGTYPE', True)

scripts/lib/devtool/runqemu.py +1 -1

index e7f26ab..4d08d8c 100644
@@ -29,7 +29,7 @@ logger = logging.getLogger('devtool')
 def runqemu(args, config, basepath, workspace):
     """Entry point for the devtool 'runqemu' subcommand"""
 
-    tinfoil = setup_tinfoil()
+    tinfoil = setup_tinfoil(basepath=basepath)
     machine = tinfoil.config_data.getVar('MACHINE', True)
     bindir_native = tinfoil.config_data.getVar('STAGING_BINDIR_NATIVE', True)
     tinfoil.shutdown()

scripts/lib/devtool/search.py +1 -1

index e6ae922..c2f420c 100644
@@ -29,7 +29,7 @@ logger = logging.getLogger('devtool')
 def search(args, config, basepath, workspace):
     """Entry point for the devtool 'search' subcommand"""
 
-    tinfoil = setup_tinfoil(config_only=True)
+    tinfoil = setup_tinfoil(config_only=True, basepath=basepath)
     pkgdata_dir = tinfoil.config_data.getVar('PKGDATA_DIR', True)
     tinfoil.shutdown()

scripts/lib/devtool/standard.py +5 -5

index 96b271c..1dcf7cd 100644
@@ -111,7 +111,7 @@ def add(args, config, basepath, workspace):
         (stdout, _) = bb.process.run('git rev-parse HEAD', cwd=srctree)
         initial_rev = stdout.rstrip()
 
-    tinfoil = setup_tinfoil(config_only=True)
+    tinfoil = setup_tinfoil(config_only=True, basepath=basepath)
     rd = oe.recipeutils.parse_recipe(recipefile, None, tinfoil.config_data)
     if not rd:
         return 1

@@ -231,7 +231,7 @@ class BbTaskExecutor(object):
 def _prep_extract_operation(config, basepath, recipename):
     """HACK: Ugly workaround for making sure that requirements are met when
        trying to extract a package. Returns the tinfoil instance to be used."""
-    tinfoil = setup_tinfoil()
+    tinfoil = setup_tinfoil(basepath=basepath)
     rd = parse_recipe(config, tinfoil, recipename, True)
 
     if bb.data.inherits_class('kernel-yocto', rd):

@@ -239,7 +239,7 @@ def _prep_extract_operation(config, basepath, recipename):
         try:
             stdout, _ = exec_build_env_command(config.init_path, basepath,
                                                'bitbake kern-tools-native')
-            tinfoil = setup_tinfoil()
+            tinfoil = setup_tinfoil(basepath=basepath)
         except bb.process.ExecutionError as err:
             raise DevtoolError("Failed to build kern-tools-native:\n%s" %
                                err.stdout)

@@ -443,7 +443,7 @@ def modify(args, config, basepath, workspace):
     if args.extract:
         tinfoil = _prep_extract_operation(config, basepath, args.recipename)
     else:
-        tinfoil = setup_tinfoil()
+        tinfoil = setup_tinfoil(basepath=basepath)
 
     rd = parse_recipe(config, tinfoil, args.recipename, True)
     if not rd:

@@ -797,7 +797,7 @@ def update_recipe(args, config, basepath, workspace):
             raise DevtoolError('conf/layer.conf not found in bbappend '
                                'destination layer "%s"' % args.append)
 
-    tinfoil = setup_tinfoil()
+    tinfoil = setup_tinfoil(basepath=basepath)
 
     rd = parse_recipe(config, tinfoil, args.recipename, True)
     if not rd:

scripts/lib/devtool/upgrade.py +1 -1

index 6c1dfee..aa53c8e 100644
@@ -294,7 +294,7 @@ def upgrade(args, config, basepath, workspace):
     if reason:
         raise DevtoolError(reason)
 
-    tinfoil = setup_tinfoil()
+    tinfoil = setup_tinfoil(basepath=basepath)
 
     rd = parse_recipe(config, tinfoil, args.recipename, True)
     if not rd:

Signature · 1 line

2.1.0

Paul Eggleton

author

2015-09-23 10:05 UTC #4 lore raw

PATCH v2 2/3 devtool: runqemu: avoid recipe parse

In reply to Paul Eggleton

We only need the base configuration to get the variable values we want to get here, there's no need to parse recipes / load the cache.

Signed-off-by Paul Eggleton <[email protected]>

---
 scripts/lib/devtool/runqemu.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

scripts/lib/devtool/runqemu.py +1 -1

index 4d08d8c..5282afb 100644
@@ -29,7 +29,7 @@ logger = logging.getLogger('devtool')
 def runqemu(args, config, basepath, workspace):
     """Entry point for the devtool 'runqemu' subcommand"""
 
-    tinfoil = setup_tinfoil(basepath=basepath)
+    tinfoil = setup_tinfoil(config_only=True, basepath=basepath)
     machine = tinfoil.config_data.getVar('MACHINE', True)
     bindir_native = tinfoil.config_data.getVar('STAGING_BINDIR_NATIVE', True)
     tinfoil.shutdown()

Signature · 1 line

2.1.0

Paul Eggleton

author

2015-09-23 10:05 UTC #5 lore raw

PATCH v2 3/3 devtool: upgrade: use shutil.move instead of os.rename

In reply to Paul Eggleton

From: Markus Lehtonen <[email protected]>

Rename fails over filesystem boundaries.

Signed-off-by Markus Lehtonen <[email protected]>

Signed-off-by Paul Eggleton <[email protected]>

---
 scripts/lib/devtool/upgrade.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

scripts/lib/devtool/upgrade.py +1 -1

index aa53c8e..4f850cf 100644
@@ -53,7 +53,7 @@ def _copy_source_code(orig, dest):
         dest_dir = os.path.join(dest, os.path.dirname(path))
         bb.utils.mkdirhier(dest_dir)
         dest_path = os.path.join(dest, path)
-        os.rename(os.path.join(orig, path), dest_path)
+        shutil.move(os.path.join(orig, path), dest_path)
 
 def _get_checksums(rf):
     import re

Signature · 1 line

2.1.0

Christopher Larson

2015-09-23 15:09 UTC #2 lore raw

Re: [PATCH v2 1/3] devtool: second fix for running from a different directory

In reply to Paul Eggleton

Quoted 12 lines · Paul Eggleton

scripts/lib/devtool/__init__.py +4 -1

-def setup_tinfoil(config_only=False):
+def setup_tinfoil(config_only=False, basepath=None):
     """Initialize tinfoil api from bitbake"""
     import scriptpath
+    orig_cwd = os.path.abspath(os.curdir)
+    if basepath:
+        os.chdir(basepath)
     bitbakepath = scriptpath.add_bitbake_lib_path()
     if not bitbakepath:
         logger.error("Unable to find bitbake by searching parent

directory of this script or PATH")

scripts/lib/devtool/__init__.py def setup_tinfoil(config_only=False): · line 116

Quoted 6 lines

scripts/lib/devtool/__init__.py +1 -0

@@ -108,6 +111,7 @@ def setup_tinfoil(config_only=False):
     tinfoil = bb.tinfoil.Tinfoil()
     tinfoil.prepare(config_only)
     tinfoil.logger.setLevel(logger.getEffectiveLevel())
+    os.chdir(orig_cwd)
     return tinfoil

Just from a correctness standpoint, this will not go back to orig_cwd if an exception is raised. We should be using a try/finally block for things like this.

Signature · 5 lines

Christopher Larson
clarson at kergoth dot com
Founder - BitBake, OpenEmbedded, OpenZaurus
Maintainer - Tslib
Senior Software Engineer, Mentor Graphics

Paul Eggleton

author

2015-09-23 16:05 UTC #3 lore raw

Re: [PATCH v2 1/3] devtool: second fix for running from a different directory

Quoted 25 lines · On Wednesday 23 September 2015 08:09:37 Christopher Larson wrote:

On Wed, Sep 23, 2015 at 3:05 AM, Paul Eggleton <

Quoted 22 lines · [email protected]> wrote:

scripts/lib/devtool/__init__.py +4 -1

-def setup_tinfoil(config_only=False):
 
+def setup_tinfoil(config_only=False, basepath=None):
     """Initialize tinfoil api from bitbake"""
     import scriptpath
 
+    orig_cwd = os.path.abspath(os.curdir)
+    if basepath:
+        os.chdir(basepath)
 
     bitbakepath = scriptpath.add_bitbake_lib_path()
 
     if not bitbakepath:
         logger.error("Unable to find bitbake by searching parent

directory of this script or PATH")

scripts/lib/devtool/__init__.py +0 -0

@@ -108,6 +111,7 @@ def setup_tinfoil(config_only=False):
     tinfoil = bb.tinfoil.Tinfoil()
     tinfoil.prepare(config_only)
     tinfoil.logger.setLevel(logger.getEffectiveLevel())

Quoted 7 lines

scripts/lib/devtool/__init__.py def setup_tinfoil(config_only=False):
Quoted 3 lines
scripts/lib/devtool/__init__.py +1 -0
+    os.chdir(orig_cwd)
 
     return tinfoil
Just from a correctness standpoint, this will not go back to orig_cwd if an exception is raised. We should be using a try/finally block for things like this.

That is a good point yes. I can send out a v3.

Cheers,
Paul

Signature · 2 lines

Paul Eggleton
Intel Open Source Technology Centre