[PATCH v2 0/9] devtool fixes

[PATCH v2 0/9] devtool fixes

[Paul Eggleton]

This series rolls up some patches previously posted by Markus (with
very minor tweaks to a couple of the commit messages), and adds some
additional patches from me.

Changes since v1:
* Drop "devtool: extract: remove patches when S=WORKDIR" as it needs
  further work
* Replace Markus's oe-selftest fix with a rewritten one which avoids
  almost any dependency on the number/name of patches within the
  mtd-utils recipe (since a recent change just broke the old test
  again)
* Add a couple of fixes for update-recipe
* Touch up a couple of the commit messages


The following changes since commit 46869abf72c1c5babce49537f221d7a4f53ca820:

  oeqa/selftest/toaster: fix bad indent (2015-04-27 10:09:55 +0100)

are available in the git repository at:

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

Markus Lehtonen (3):
  devtool: modify: use B=S if that is the default for the recipe
  devtool: modify: implement --no-same-dir
  devtool: include bbappends in recipe parsing

Paul Eggleton (6):
  oe-selftest: devtool: fix test_devtool_update_recipe_git
  devtool: handle . in recipe name
  devtool: add: use correct bbappend file name with -V option
  devtool: reset: avoid errors in case file no longer exists
  devtool: update-recipe: handle unversioned bbappends
  devtool: update-recipe: check if source tree is a git repository

 meta/lib/oe/recipeutils.py        |  10 ++--
 meta/lib/oeqa/selftest/devtool.py |  30 +++++++-----
 scripts/devtool                   |   2 +-
 scripts/lib/devtool/standard.py   | 100 +++++++++++++++++++++++++-------------
 4 files changed, 92 insertions(+), 50 deletions(-)

-- 
2.1.0

[PATCH v2 1/9] devtool: modify: use B=S if that is the default for the recipe

[Paul Eggleton]

From: Markus Lehtonen <markus.lehtonen@linux.intel.com>

Makes the build succeed for packages which do not support separate build
directory, e.g. zlib. The same outcome could be achieved with the
--same-dir option, but, it's generally hard to tell if a random package
would need that option. The negative side effect of this patch is that
dev srctree (of some packages that build fine without this modification)
gets dirtied by build artefacts.

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
Acked-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 scripts/lib/devtool/standard.py | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index d5ded2f..fbac34e 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -384,12 +384,19 @@ def modify(args, config, basepath, workspace):
         f.write('inherit externalsrc\n')
         f.write('# NOTE: We use pn- overrides here to avoid affecting multiple variants in the case where the recipe uses BBCLASSEXTEND\n')
         f.write('EXTERNALSRC_pn-%s = "%s"\n' % (args.recipename, srctree))
-        if args.same_dir or bb.data.inherits_class('autotools-brokensep', rd):
-            if args.same_dir:
-                logger.info('using source tree as build directory since --same-dir specified')
-            else:
-                logger.info('using source tree as build directory since original recipe inherits autotools-brokensep')
+
+        b_is_s = True
+        if args.same_dir:
+            logger.info('using source tree as build directory since --same-dir specified')
+        elif bb.data.inherits_class('autotools-brokensep', rd):
+            logger.info('using source tree as build directory since original recipe inherits autotools-brokensep')
+        elif rd.getVar('B', True) == s:
+            logger.info('using source tree as build directory since that is the default for this recipe')
+        else:
+            b_is_s = False
+        if b_is_s:
             f.write('EXTERNALSRC_BUILD_pn-%s = "%s"\n' % (args.recipename, srctree))
+
         if initial_rev:
             f.write('\n# initial_rev: %s\n' % initial_rev)
             for commit in commits:
-- 
2.1.0

[PATCH v2 2/9] devtool: modify: implement --no-same-dir

[Paul Eggleton]

From: Markus Lehtonen <markus.lehtonen@linux.intel.com>

This option can be used to have a separate build directory, in order to
keep the srctree directory clean for packages that do not need to be
built in the source directory.

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
---
 scripts/lib/devtool/standard.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index fbac34e..aa30a98 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -386,7 +386,10 @@ def modify(args, config, basepath, workspace):
         f.write('EXTERNALSRC_pn-%s = "%s"\n' % (args.recipename, srctree))
 
         b_is_s = True
-        if args.same_dir:
+        if args.no_same_dir:
+            logger.info('using separate build directory since --no-same-dir specified')
+            b_is_s = False
+        elif args.same_dir:
             logger.info('using source tree as build directory since --same-dir specified')
         elif bb.data.inherits_class('autotools-brokensep', rd):
             logger.info('using source tree as build directory since original recipe inherits autotools-brokensep')
@@ -664,7 +667,9 @@ def register_commands(subparsers, context):
     parser_modify.add_argument('srctree', help='Path to external source tree')
     parser_modify.add_argument('--wildcard', '-w', action="store_true", help='Use wildcard for unversioned bbappend')
     parser_modify.add_argument('--extract', '-x', action="store_true", help='Extract source as well')
-    parser_modify.add_argument('--same-dir', '-s', help='Build in same directory as source', action="store_true")
+    group = parser_modify.add_mutually_exclusive_group()
+    group.add_argument('--same-dir', '-s', help='Build in same directory as source', action="store_true")
+    group.add_argument('--no-same-dir', help='Force build in a separate build directory', action="store_true")
     parser_modify.add_argument('--branch', '-b', default="devtool", help='Name for development branch to checkout (only when using -x)')
     parser_modify.set_defaults(func=modify)
 
-- 
2.1.0

[PATCH v2 3/9] oe-selftest: devtool: fix test_devtool_update_recipe_git

[Paul Eggleton]

Make this test work after recent changes to the mtd-utils recipe, and
hopefully make it robust against any future changes.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/lib/oeqa/selftest/devtool.py | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/meta/lib/oeqa/selftest/devtool.py b/meta/lib/oeqa/selftest/devtool.py
index dc1cf21..1a5eecd 100644
--- a/meta/lib/oeqa/selftest/devtool.py
+++ b/meta/lib/oeqa/selftest/devtool.py
@@ -370,6 +370,11 @@ class DevtoolTests(oeSelfTest):
         recipefile = get_bb_var('FILE', testrecipe)
         src_uri = get_bb_var('SRC_URI', testrecipe)
         self.assertIn('git://', src_uri, 'This test expects the %s recipe to be a git recipe' % testrecipe)
+        patches = []
+        for entry in src_uri.split():
+            if entry.startswith('file://') and entry.endswith('.patch'):
+                patches.append(entry[7:].split(';')[0])
+        self.assertGreater(len(patches), 0, 'The %s recipe does not appear to contain any patches, so this test will not be effective' % testrecipe)
         result = runCmd('git status . --porcelain', cwd=os.path.dirname(recipefile))
         self.assertEqual(result.output.strip(), "", '%s recipe is not clean' % testrecipe)
         # First, modify a recipe
@@ -397,19 +402,22 @@ class DevtoolTests(oeSelfTest):
         result = runCmd('git status . --porcelain', cwd=os.path.dirname(recipefile))
         self.assertNotEqual(result.output.strip(), "", '%s recipe should be modified' % testrecipe)
         status = result.output.splitlines()
-        self.assertEqual(len(status), 3, 'Less/more files modified than expected. Entire status:\n%s' % result.output)
         for line in status:
-            if line.endswith('add-exclusion-to-mkfs-jffs2-git-2.patch'):
-                self.assertEqual(line[:3], ' D ', 'Unexpected status in line: %s' % line)
-            elif line.endswith('fix-armv7-neon-alignment.patch'):
-                self.assertEqual(line[:3], ' D ', 'Unexpected status in line: %s' % line)
-            elif re.search('%s_[^_]*.bb$' % testrecipe, line):
-                self.assertEqual(line[:3], ' M ', 'Unexpected status in line: %s' % line)
+            for patch in patches:
+                if line.endswith(patch):
+                    self.assertEqual(line[:3], ' D ', 'Unexpected status in line: %s' % line)
+                    break
             else:
-                raise AssertionError('Unexpected modified file in status: %s' % line)
+                if re.search('%s_[^_]*.bb$' % testrecipe, line):
+                    self.assertEqual(line[:3], ' M ', 'Unexpected status in line: %s' % line)
+                else:
+                    raise AssertionError('Unexpected modified file in status: %s' % line)
         result = runCmd('git diff %s' % os.path.basename(recipefile), cwd=os.path.dirname(recipefile))
         addlines = ['SRCREV = ".*"', 'SRC_URI = "git://git.infradead.org/mtd-utils.git"']
-        removelines = ['SRCREV = ".*"', 'SRC_URI = "git://git.infradead.org/mtd-utils.git \\\\', 'file://add-exclusion-to-mkfs-jffs2-git-2.patch \\\\', 'file://fix-armv7-neon-alignment.patch \\\\', '"']
+        srcurilines = src_uri.split()
+        srcurilines[0] = 'SRC_URI = "' + srcurilines[0]
+        srcurilines.append('"')
+        removelines = ['SRCREV = ".*"'] + srcurilines
         for line in result.output.splitlines():
             if line.startswith('+++') or line.startswith('---'):
                 continue
-- 
2.1.0

[PATCH v2 4/9] devtool: include bbappends in recipe parsing

[Paul Eggleton]

From: Markus Lehtonen <markus.lehtonen@linux.intel.com>

In order to get correct metadata, SRCREV for example.

Fixes [YOCTO #7648].

Signed-off-by: Markus Lehtonen <markus.lehtonen@linux.intel.com>
---
 meta/lib/oe/recipeutils.py      |  6 +++---
 scripts/lib/devtool/standard.py | 35 ++++++++++++++++++++++-------------
 2 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/meta/lib/oe/recipeutils.py b/meta/lib/oe/recipeutils.py
index 159a103..09bd7fd 100644
--- a/meta/lib/oe/recipeutils.py
+++ b/meta/lib/oe/recipeutils.py
@@ -44,10 +44,10 @@ def get_unavailable_reasons(cooker, pn):
     return taskdata.get_reasons(pn)
 
 
-def parse_recipe(fn, d):
+def parse_recipe(fn, appends, d):
     """Parse an individual recipe"""
     import bb.cache
-    envdata = bb.cache.Cache.loadDataFull(fn, [], d)
+    envdata = bb.cache.Cache.loadDataFull(fn, appends, d)
     return envdata
 
 
@@ -55,7 +55,7 @@ def get_var_files(fn, varlist, d):
     """Find the file in which each of a list of variables is set.
     Note: requires variable history to be enabled when parsing.
     """
-    envdata = parse_recipe(fn, d)
+    envdata = parse_recipe(fn, [], d)
     varfiles = {}
     for v in varlist:
         history = envdata.varhistory.variable(v)
diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index aa30a98..faf5c92 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -130,18 +130,29 @@ def _get_recipe_file(cooker, pn):
             logger.error("Unable to find any recipe file matching %s" % pn)
     return recipefile
 
+def _parse_recipe(config, tinfoil, pn, appends):
+    """Parse recipe of a package"""
+    import oe.recipeutils
+    recipefile = _get_recipe_file(tinfoil.cooker, pn)
+    if not recipefile:
+        # Error already logged
+        return None
+    if appends:
+        append_files = tinfoil.cooker.collection.get_file_appends(recipefile)
+        # Filter out appends from the workspace
+        append_files = [path for path in append_files if
+                        not path.startswith(config.workspace_path)]
+    return oe.recipeutils.parse_recipe(recipefile, append_files,
+                                       tinfoil.config_data)
 
 def extract(args, config, basepath, workspace):
     import bb
-    import oe.recipeutils
 
     tinfoil = setup_tinfoil()
 
-    recipefile = _get_recipe_file(tinfoil.cooker, args.recipename)
-    if not recipefile:
-        # Error already logged
+    rd = _parse_recipe(config, tinfoil, args.recipename, True)
+    if not rd:
         return -1
-    rd = oe.recipeutils.parse_recipe(recipefile, tinfoil.config_data)
 
     srctree = os.path.abspath(args.srctree)
     initial_rev = _extract_source(srctree, args.keep_temp, args.branch, rd)
@@ -327,11 +338,10 @@ def modify(args, config, basepath, workspace):
 
     tinfoil = setup_tinfoil()
 
-    recipefile = _get_recipe_file(tinfoil.cooker, args.recipename)
-    if not recipefile:
-        # Error already logged
+    rd = _parse_recipe(config, tinfoil, args.recipename, True)
+    if not rd:
         return -1
-    rd = oe.recipeutils.parse_recipe(recipefile, tinfoil.config_data)
+    recipefile = rd.getVar('FILE', True)
 
     if not _check_compatible_recipe(args.recipename, rd):
         return -1
@@ -428,11 +438,10 @@ def update_recipe(args, config, basepath, workspace):
     from oe.patch import GitApplyTree
     import oe.recipeutils
 
-    recipefile = _get_recipe_file(tinfoil.cooker, args.recipename)
-    if not recipefile:
-        # Error already logged
+    rd = _parse_recipe(config, tinfoil, args.recipename, True)
+    if not rd:
         return -1
-    rd = oe.recipeutils.parse_recipe(recipefile, tinfoil.config_data)
+    recipefile = rd.getVar('FILE', True)
 
     orig_src_uri = rd.getVar('SRC_URI', False) or ''
     if args.mode == 'auto':
-- 
2.1.0

[PATCH v2 5/9] devtool: handle . in recipe name

[Paul Eggleton]

Names such as glib-2.0 are valid (and used) recipe names, so we need to
support them.

Fixes [YOCTO #7643].

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/lib/oe/recipeutils.py | 4 ++--
 scripts/devtool            | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/meta/lib/oe/recipeutils.py b/meta/lib/oe/recipeutils.py
index 09bd7fd..19d97b6 100644
--- a/meta/lib/oe/recipeutils.py
+++ b/meta/lib/oe/recipeutils.py
@@ -269,8 +269,8 @@ def get_recipe_patches(d):
 def validate_pn(pn):
     """Perform validation on a recipe name (PN) for a new recipe."""
     reserved_names = ['forcevariable', 'append', 'prepend', 'remove']
-    if not re.match('[0-9a-z-]+', pn):
-        return 'Recipe name "%s" is invalid: only characters 0-9, a-z and - are allowed' % pn
+    if not re.match('[0-9a-z-.]+', pn):
+        return 'Recipe name "%s" is invalid: only characters 0-9, a-z, - and . are allowed' % pn
     elif pn in reserved_names:
         return 'Recipe name "%s" is invalid: is a reserved keyword' % pn
     elif pn.startswith('pn-'):
diff --git a/scripts/devtool b/scripts/devtool
index 981ff51..841831c 100755
--- a/scripts/devtool
+++ b/scripts/devtool
@@ -101,7 +101,7 @@ def read_workspace():
             _create_workspace(config.workspace_path, config, basepath)
 
     logger.debug('Reading workspace in %s' % config.workspace_path)
-    externalsrc_re = re.compile(r'^EXTERNALSRC(_pn-[a-zA-Z0-9-]*)? =.*$')
+    externalsrc_re = re.compile(r'^EXTERNALSRC(_pn-[^ =]+)? =.*$')
     for fn in glob.glob(os.path.join(config.workspace_path, 'appends', '*.bbappend')):
         pn = os.path.splitext(os.path.basename(fn))[0].split('_')[0]
         with open(fn, 'r') as f:
-- 
2.1.0

[PATCH v2 6/9] devtool: add: use correct bbappend file name with -V option

[Paul Eggleton]

We weren't adding the version into the bbappend file name when -V was
specified which meant that building or resetting failed.

Also adjust one of the tests so that we're testing devtool add both with
and without this option.

Fixes [YOCTO #7647].

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 meta/lib/oeqa/selftest/devtool.py | 4 ++--
 scripts/lib/devtool/standard.py   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/meta/lib/oeqa/selftest/devtool.py b/meta/lib/oeqa/selftest/devtool.py
index 1a5eecd..6881835 100644
--- a/meta/lib/oeqa/selftest/devtool.py
+++ b/meta/lib/oeqa/selftest/devtool.py
@@ -150,10 +150,10 @@ class DevtoolTests(oeSelfTest):
         result = runCmd('tar xfv libftdi1-1.1.tar.bz2', cwd=tempdir)
         srcdir = os.path.join(tempdir, 'libftdi1-1.1')
         self.assertTrue(os.path.isfile(os.path.join(srcdir, 'CMakeLists.txt')), 'Unable to find CMakeLists.txt in source directory')
-        # Test devtool add
+        # Test devtool add (and use -V so we test that too)
         self.track_for_cleanup(workspacedir)
         self.add_command_to_tearDown('bitbake-layers remove-layer */workspace')
-        result = runCmd('devtool add libftdi %s' % srcdir)
+        result = runCmd('devtool add libftdi %s -V 1.1' % srcdir)
         self.assertTrue(os.path.exists(os.path.join(workspacedir, 'conf', 'layer.conf')), 'Workspace directory not created')
         # Test devtool status
         result = runCmd('devtool status')
diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index faf5c92..8932265 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -73,7 +73,7 @@ def add(args, config, basepath, workspace):
         (stdout, _) = bb.process.run('git rev-parse HEAD', cwd=srctree)
         initial_rev = stdout.rstrip()
 
-    appendfile = os.path.join(appendpath, '%s.bbappend' % args.recipename)
+    appendfile = os.path.join(appendpath, '%s.bbappend' % bp)
     with open(appendfile, 'w') as f:
         f.write('inherit externalsrc\n')
         f.write('EXTERNALSRC = "%s"\n' % srctree)
-- 
2.1.0

[PATCH v2 7/9] devtool: reset: avoid errors in case file no longer exists

[Paul Eggleton]

If you manually delete files in the workspace layer (which you really
shouldn't) it was possible to get yourself into the situation where you
couldn't reset because we were attempting to check if the file had been
modified and erroring out if it couldn't be opened. If the file's not
there anymore there's not much point checking if it needs to be
preserved, just skip it.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 scripts/lib/devtool/standard.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index 8932265..d561e40 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -308,7 +308,14 @@ def _check_preserve(config, recipename):
                 splitline = line.rstrip().split('|')
                 if splitline[0] == recipename:
                     removefile = os.path.join(config.workspace_path, splitline[1])
-                    md5 = bb.utils.md5_file(removefile)
+                    try:
+                        md5 = bb.utils.md5_file(removefile)
+                    except IOError as err:
+                        if err.errno == 2:
+                            # File no longer exists, skip it
+                            continue
+                        else:
+                            raise
                     if splitline[2] != md5:
                         bb.utils.mkdirhier(preservepath)
                         preservefile = os.path.basename(removefile)
-- 
2.1.0

[PATCH v2 8/9] devtool: update-recipe: handle unversioned bbappends

[Paul Eggleton]

Use the proper bbappend file name, don't just assume it will have a
version suffix (because it won't if the original recipe doesn't).

Fixes [YOCTO #7651].

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 scripts/lib/devtool/standard.py | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index d561e40..a642112 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -434,12 +434,6 @@ def update_recipe(args, config, basepath, workspace):
         logger.error("no recipe named %s in your workspace" % args.recipename)
         return -1
 
-    # Get initial revision from bbappend
-    appends = glob.glob(os.path.join(config.workspace_path, 'appends', '%s_*.bbappend' % args.recipename))
-    if not appends:
-        logger.error('unable to find workspace bbappend for recipe %s' % args.recipename)
-        return -1
-
     tinfoil = setup_tinfoil()
     import bb
     from oe.patch import GitApplyTree
@@ -450,6 +444,12 @@ def update_recipe(args, config, basepath, workspace):
         return -1
     recipefile = rd.getVar('FILE', True)
 
+    # Get initial revision from bbappend
+    append = os.path.join(config.workspace_path, 'appends', '%s.bbappend' % os.path.splitext(os.path.basename(recipefile))[0])
+    if not os.path.exists(append):
+        logger.error('unable to find workspace bbappend for recipe %s' % args.recipename)
+        return -1
+
     orig_src_uri = rd.getVar('SRC_URI', False) or ''
     if args.mode == 'auto':
         if 'git://' in orig_src_uri:
@@ -520,7 +520,7 @@ def update_recipe(args, config, basepath, workspace):
             initial_rev = args.initial_rev
         else:
             initial_rev = None
-            with open(appends[0], 'r') as f:
+            with open(append, 'r') as f:
                 for line in f:
                     if line.startswith('# initial_rev:'):
                         initial_rev = line.split(':')[-1].strip()
-- 
2.1.0

[PATCH v2 9/9] devtool: update-recipe: check if source tree is a git repository

[Paul Eggleton]

If you've done "devtool add" (or "devtool modify" without -x) then it's
possible that the external source tree is not a git repository, so we
should handle that case here instead of printing a traceback.

Signed-off-by: Paul Eggleton <paul.eggleton@linux.intel.com>
---
 scripts/lib/devtool/standard.py | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index a642112..94b5e0b 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -477,13 +477,19 @@ def update_recipe(args, config, basepath, workspace):
         return updated
 
     srctree = workspace[args.recipename]
-    if mode == 'srcrev':
+
+    # Get HEAD revision
+    try:
         (stdout, _) = bb.process.run('git rev-parse HEAD', cwd=srctree)
-        srcrev = stdout.strip()
-        if len(srcrev) != 40:
-            logger.error('Invalid hash returned by git: %s' % stdout)
-            return 1
+    except bb.process.ExecutionError as err:
+        print('Failed to get HEAD revision in %s: %s' % (srctree, err))
+        return 1
+    srcrev = stdout.strip()
+    if len(srcrev) != 40:
+        logger.error('Invalid hash returned by git: %s' % stdout)
+        return 1
 
+    if mode == 'srcrev':
         logger.info('Updating SRCREV in recipe %s' % os.path.basename(recipefile))
         patchfields = {}
         patchfields['SRCREV'] = srcrev
-- 
2.1.0