Skip to contentSkip to conversation

devtool/recipetool and related fixes

PATCH New Paul Eggleton 1 participant 6 messages Jun 17, 2015

lore
Share & tools
lore
#0 lore raw

cover devtool/recipetool and related fixes

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(-)
Signature · 1 line
2.1.0
#1 lore raw

PATCH 1/5 recipetool: appendfile: clarify help text

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.

---
 scripts/lib/recipetool/append.py | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
scripts/lib/recipetool/append.py +6 -6
index 39117c1..28015c6 100644
@@ -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)
Signature · 1 line
2.1.0
#2 lore raw

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

In reply to 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.

---
 scripts/lib/recipetool/append.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
scripts/lib/recipetool/append.py +4 -2
index 28015c6..ed4ce4a 100644
@@ -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:
Signature · 1 line
2.1.0
#3 lore raw

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

In reply to 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].

---
 meta/classes/externalsrc.bbclass | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
meta/classes/externalsrc.bbclass +1 -1
index 75bdb7a..8f7f479 100644
@@ -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)
Signature · 1 line
2.1.0
#4 lore raw

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

In reply to Paul Eggleton

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

---
 meta/lib/oeqa/utils/commands.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
meta/lib/oeqa/utils/commands.py +1 -1
index dc8a983..1be7bed 100644
@@ -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):
Signature · 1 line
2.1.0
#5 lore raw

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

In reply to 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].

---
 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(-)
meta/lib/oeqa/selftest/devtool.py +46 -2
index 4e22e1d..c4a0399 100644
@@ -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()
scripts/lib/devtool/__init__.py +16 -0
index ea0b63e..61b810c 100644
@@ -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
scripts/lib/devtool/deploy.py +7 -3
index ca74a8e..448db96 100644
@@ -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')
Signature · 1 line
2.1.0