[PATCH 0/5] devtool/recipetool and related fixes

[PATCH 0/5] devtool/recipetool and related fixes

[Paul Eggleton]

A couple of fixes for errors thrown up by oe-selftest tests on one of
my machines (plus one of the autobuilders), a supporting fix for
get_bb_var(), a bugfix for externalsrc, and an improvement for
recipetool appendfile's help text.

NOTE: These have been rebased on top of Markus's devtool refactoring
branch (marquiz/devtool/refactor), so that needs to be merged first.


The following changes since commit 5a4eb3a7b6402384e05d47cb897c685c530a8c3b:

  devtool: use DevtoolError for error handling (2015-06-17 14:09:20 +0300)

are available in the git repository at:

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

Paul Eggleton (5):
  recipetool: appendfile: clarify help text
  recipetool: appendfile: fix file command error handling
  classes/externalsrc: handle tasks with existing lockfiles
  lib/oeqa/utils/commands: ensure get_bb_var() works when value contains =
  devtool: deploy: fix preservation of symlinks and permissions/ownership

 meta/classes/externalsrc.bbclass  |  2 +-
 meta/lib/oeqa/selftest/devtool.py | 48 +++++++++++++++++++++++++++++++++++++--
 meta/lib/oeqa/utils/commands.py   |  2 +-
 scripts/lib/devtool/__init__.py   | 16 +++++++++++++
 scripts/lib/devtool/deploy.py     | 10 +++++---
 scripts/lib/recipetool/append.py  | 18 ++++++++-------
 6 files changed, 81 insertions(+), 15 deletions(-)

-- 
2.1.0

[PATCH 1/5] recipetool: appendfile: clarify help text

[Paul Eggleton]

Add a long description and tweak some of the argument descriptions so
that it's clearer what the appendfile subcommand does and how it works.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 scripts/lib/recipetool/append.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/scripts/lib/recipetool/append.py b/scripts/lib/recipetool/append.py
index 39117c1..28015c6 100644
--- a/scripts/lib/recipetool/append.py
+++ b/scripts/lib/recipetool/append.py
@@ -349,12 +349,12 @@ def appendfile(args):
 
 def register_command(subparsers):
     parser_appendfile = subparsers.add_parser('appendfile',
-                                                    help='Create a bbappend to replace a file',
-                                                    description='')
-    parser_appendfile.add_argument('destlayer', help='Destination layer to write the bbappend to')
-    parser_appendfile.add_argument('targetpath', help='Path within the image to the file to be replaced')
-    parser_appendfile.add_argument('newfile', help='Custom file to replace it with')
-    parser_appendfile.add_argument('-r', '--recipe', help='Override recipe to apply to (default is to find which recipe already packages it)')
+                                                help='Create/update a bbappend to replace a file',
+                                                description='Creates a bbappend (or updates an existing one) to replace the specified file that appears in the target system, determining the recipe that packages the file and the required path and name for the bbappend automatically. Note that the ability to determine the recipe packaging a particular file depends upon the recipe\'s do_packagedata task having already run prior to running this command (which it will have when the recipe has been built successfully, which in turn will have happened if one or more of the recipe\'s packages is included in an image that has been built successfully).')
+    parser_appendfile.add_argument('destlayer', help='Base directory of the destination layer to write the bbappend to')
+    parser_appendfile.add_argument('targetpath', help='Path to the file to be replaced (as it would appear within the target image, e.g. /etc/motd)')
+    parser_appendfile.add_argument('newfile', help='Custom file to replace the target file with')
+    parser_appendfile.add_argument('-r', '--recipe', help='Override recipe to apply to (default is to find which recipe already packages the file)')
     parser_appendfile.add_argument('-m', '--machine', help='Make bbappend changes specific to a machine only', metavar='MACHINE')
     parser_appendfile.add_argument('-w', '--wildcard-version', help='Use wildcard to make the bbappend apply to any recipe version', action='store_true')
     parser_appendfile.set_defaults(func=appendfile, parserecipes=True)
-- 
2.1.0

[PATCH 2/5] recipetool: appendfile: fix file command error handling

[Paul Eggleton]

* It turns out that not all versions of the file command support the -E
  option - the version in Ubuntu 14.04 doesn't support it for example.
  This option is supposed to force file to return an error if the file
  can't be opened - since we can't rely upon it then fall back to
  looking at the output instead. (The results of this issue were simply
  that we didn't notice if the file was executable and give a warning,
  which tripped an oe-selftest failure - so it was minor.)
* If we receive an error there's not much point looking at the output to
  see what type was returned because there wasn't one.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 scripts/lib/recipetool/append.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/scripts/lib/recipetool/append.py b/scripts/lib/recipetool/append.py
index 28015c6..ed4ce4a 100644
--- a/scripts/lib/recipetool/append.py
+++ b/scripts/lib/recipetool/append.py
@@ -262,10 +262,12 @@ def appendfile(args):
 
     stdout = ''
     try:
-        (stdout, _) = bb.process.run('LANG=C file -E -b %s' % args.newfile, shell=True)
+        (stdout, _) = bb.process.run('LANG=C file -b %s' % args.newfile, shell=True)
+        if 'cannot open' in stdout:
+            raise bb.process.ExecutionError(stdout)
     except bb.process.ExecutionError as err:
         logger.debug('file command returned error: %s' % err)
-        pass
+        stdout = ''
     if stdout:
         logger.debug('file command output: %s' % stdout.rstrip())
         if ('executable' in stdout and not 'shell script' in stdout) or 'shared object' in stdout:
-- 
2.1.0

[PATCH 3/5] classes/externalsrc: handle tasks with existing lockfiles

[Paul Eggleton]

We need to ensure we add a leading space to the value we are prepending
here in case lockfiles already has a value.

Fixes [YOCTO #7813].

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/classes/externalsrc.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/classes/externalsrc.bbclass b/meta/classes/externalsrc.bbclass
index 75bdb7a..8f7f479 100644
--- a/meta/classes/externalsrc.bbclass
+++ b/meta/classes/externalsrc.bbclass
@@ -55,7 +55,7 @@ python () {
                 bb.build.deltask(task, d)
             else:
                 # Since configure will likely touch ${S}, ensure only we lock so one task has access at a time
-                d.appendVarFlag(task, "lockfiles", "${S}/singletask.lock")
+                d.appendVarFlag(task, "lockfiles", " ${S}/singletask.lock")
 
             # We do not want our source to be wiped out, ever (kernel.bbclass does this for do_clean)
             cleandirs = d.getVarFlag(task, 'cleandirs', False)
-- 
2.1.0

[PATCH 4/5] lib/oeqa/utils/commands: ensure get_bb_var() works when value contains =

[Paul Eggleton]

Only split on the first equals character so that values that contain
equals characters (such as FAKEROOTENV) can be retrieved.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/lib/oeqa/utils/commands.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta/lib/oeqa/utils/commands.py b/meta/lib/oeqa/utils/commands.py
index dc8a983..1be7bed 100644
--- a/meta/lib/oeqa/utils/commands.py
+++ b/meta/lib/oeqa/utils/commands.py
@@ -141,7 +141,7 @@ def get_bb_var(var, target=None, postconfig=None):
     lastline = None
     for line in bbenv.splitlines():
         if re.search("^(export )?%s=" % var, line):
-            val = line.split('=')[1]
+            val = line.split('=', 1)[1]
             val = val.strip('\"')
             break
         elif re.match("unset %s$" % var, line):
-- 
2.1.0

[PATCH 5/5] devtool: deploy: fix preservation of symlinks and permissions/ownership

[Paul Eggleton]

It turns out that scp can't be used to copy symlinks because it follows
them instead of copying them, and this is by design (since it emulates
rcp which also behaved this way); the unfortunate result is that
symlinks that point to valid files on the host translate into the host
file being copied to the target (yuck).  The simplest alternative that
does not have this undesirable behaviour is to use tar and pipe it over
ssh.

At the same time, it would be even better if we properly reflect file
permissions and ownership on the target that have been established
within the pseudo environment. We can do this by executing the copy
process under pseudo, which turns out to be quite easy with access to
the pseudo environment set up by the build system.

Fixes [YOCTO #7868].

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/lib/oeqa/selftest/devtool.py | 48 +++++++++++++++++++++++++++++++++++++--
 scripts/lib/devtool/__init__.py   | 16 +++++++++++++
 scripts/lib/devtool/deploy.py     | 10 +++++---
 3 files changed, 69 insertions(+), 5 deletions(-)

diff --git a/meta/lib/oeqa/selftest/devtool.py b/meta/lib/oeqa/selftest/devtool.py
index 4e22e1d..c4a0399 100644
--- a/meta/lib/oeqa/selftest/devtool.py
+++ b/meta/lib/oeqa/selftest/devtool.py
@@ -60,6 +60,27 @@ class DevtoolBase(oeSelfTest):
             self.add_command_to_tearDown('bitbake-layers remove-layer %s || true' % templayerdir)
             result = runCmd('bitbake-layers add-layer %s' % templayerdir, cwd=self.builddir)
 
+    def _process_ls_output(self, output):
+        """
+        Convert ls -l output to a format we can reasonably compare from one context
+        to another (e.g. from host to target)
+        """
+        filelist = []
+        for line in output.splitlines():
+            splitline = line.split()
+            # Remove trailing . on perms
+            splitline[0] = splitline[0].rstrip('.')
+            # Remove leading . on paths
+            splitline[-1] = splitline[-1].lstrip('.')
+            # Drop fields we don't want to compare
+            del splitline[7]
+            del splitline[6]
+            del splitline[5]
+            del splitline[4]
+            del splitline[1]
+            filelist.append(' '.join(splitline))
+        return filelist
+
 
 class DevtoolTests(DevtoolBase):
 
@@ -796,9 +817,32 @@ class DevtoolTests(DevtoolBase):
         console.expect("login:", timeout=120)
         # Now really test deploy-target
         result = runCmd('devtool deploy-target -c %s root@%s' % (testrecipe, testhost))
-        result = runCmd('ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no root@%s %s' % (testhost, testcommand))
+        # Run a test command to see if it was installed properly
+        sshargs = '-o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no'
+        result = runCmd('ssh %s root@%s %s' % (sshargs, testhost, testcommand))
+        # Check if it deployed all of the files with the right ownership/perms
+        # First look on the host - need to do this under pseudo to get the correct ownership/perms
+        installdir = get_bb_var('D', testrecipe)
+        fakerootenv = get_bb_var('FAKEROOTENV', testrecipe)
+        fakerootcmd = get_bb_var('FAKEROOTCMD', testrecipe)
+        result = runCmd('%s %s find . -type f -exec ls -l {} \;' % (fakerootenv, fakerootcmd), cwd=installdir)
+        filelist1 = self._process_ls_output(result.output)
+
+        # Now look on the target
+        tempdir2 = tempfile.mkdtemp(prefix='devtoolqa')
+        self.track_for_cleanup(tempdir2)
+        tmpfilelist = os.path.join(tempdir2, 'files.txt')
+        with open(tmpfilelist, 'w') as f:
+            for line in filelist1:
+                splitline = line.split()
+                f.write(splitline[-1] + '\n')
+        result = runCmd('cat %s | ssh -q %s root@%s \'xargs ls -l\'' % (tmpfilelist, sshargs, testhost))
+        filelist2 = self._process_ls_output(result.output)
+        filelist1.sort(key=lambda item: item.split()[-1])
+        filelist2.sort(key=lambda item: item.split()[-1])
+        self.assertEqual(filelist1, filelist2)
         # Test undeploy-target
         result = runCmd('devtool undeploy-target -c %s root@%s' % (testrecipe, testhost))
-        result = runCmd('ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no root@%s %s' % (testhost, testcommand), ignore_status=True)
+        result = runCmd('ssh %s root@%s %s' % (sshargs, testhost, testcommand), ignore_status=True)
         self.assertNotEqual(result, 0, 'undeploy-target did not remove command as it should have')
         console.close()
diff --git a/scripts/lib/devtool/__init__.py b/scripts/lib/devtool/__init__.py
index ea0b63e..61b810c 100644
--- a/scripts/lib/devtool/__init__.py
+++ b/scripts/lib/devtool/__init__.py
@@ -80,6 +80,22 @@ def exec_watch(cmd, **options):
 
     return buf, None
 
+def exec_fakeroot(d, cmd, **kwargs):
+    """Run a command under fakeroot (pseudo, in fact) so that it picks up the appropriate file permissions"""
+    # Grab the command and check it actually exists
+    fakerootcmd = d.getVar('FAKEROOTCMD', True)
+    if not os.path.exists(fakerootcmd):
+        logger.error('pseudo executable %s could not be found - have you run a build yet? pseudo-native should install this and if you have run any build then that should have been built')
+        return 2
+    # Set up the appropriate environment
+    newenv = dict(os.environ)
+    fakerootenv = d.getVar('FAKEROOTENV', True)
+    for varvalue in fakerootenv.split():
+        if '=' in varvalue:
+            splitval = varvalue.split('=', 1)
+            newenv[splitval[0]] = splitval[1]
+    return subprocess.call("%s %s" % (fakerootcmd, cmd), env=newenv, **kwargs)
+
 def setup_tinfoil():
     """Initialize tinfoil api from bitbake"""
     import scriptpath
diff --git a/scripts/lib/devtool/deploy.py b/scripts/lib/devtool/deploy.py
index ca74a8e..448db96 100644
--- a/scripts/lib/devtool/deploy.py
+++ b/scripts/lib/devtool/deploy.py
@@ -19,7 +19,7 @@
 import os
 import subprocess
 import logging
-from devtool import exec_build_env_command, setup_tinfoil, DevtoolError
+from devtool import exec_fakeroot, setup_tinfoil, DevtoolError
 
 logger = logging.getLogger('devtool')
 
@@ -73,9 +73,13 @@ def deploy(args, config, basepath, workspace):
     extraoptions = ''
     if args.no_host_check:
         extraoptions += '-o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no'
-    if not args.show_status:
+    if args.show_status:
+        tarextractopts = 'xv'
+    else:
+        tarextractopts = 'x'
         extraoptions += ' -q'
-    ret = subprocess.call('scp -r %s %s/* %s:%s' % (extraoptions, recipe_outdir, args.target, destdir), shell=True)
+    # We cannot use scp here, because it doesn't preserve symlinks
+    ret = exec_fakeroot(rd, 'tar cf - . | ssh %s %s \'tar %s -C %s -f -\'' % (extraoptions, args.target, tarextractopts, destdir), cwd=recipe_outdir, shell=True)
     if ret != 0:
         raise DevtoolError('Deploy failed - rerun with -s to get a complete '
                            'error message')
-- 
2.1.0