devtool fixes
v2 cover devtool fixes
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(-)
Signature · 1 line
2.1.0
PATCH v2 1/9 devtool: modify: use B=S if that is the default for the recipe
From: Markus Lehtonen <[email protected]>
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.
--- scripts/lib/devtool/standard.py | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
scripts/lib/devtool/standard.py +12 -5
@@ -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:
Signature · 1 line
2.1.0
PATCH v2 2/9 devtool: modify: implement --no-same-dir
In reply to Paul EggletonFrom: Markus Lehtonen <[email protected]>
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.
--- scripts/lib/devtool/standard.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
scripts/lib/devtool/standard.py +7 -2
@@ -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)
Signature · 1 line
2.1.0
PATCH v2 3/9 oe-selftest: devtool: fix test_devtool_update_recipe_git
In reply to Paul EggletonMake this test work after recent changes to the mtd-utils recipe, and hopefully make it robust against any future changes.
--- meta/lib/oeqa/selftest/devtool.py | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-)
meta/lib/oeqa/selftest/devtool.py +17 -9
@@ -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
Signature · 1 line
2.1.0
PATCH v2 4/9 devtool: include bbappends in recipe parsing
In reply to Paul EggletonFrom: Markus Lehtonen <[email protected]>
In order to get correct metadata, SRCREV for example.
Fixes [YOCTO #7648].
--- meta/lib/oe/recipeutils.py | 6 +++--- scripts/lib/devtool/standard.py | 35 ++++++++++++++++++++++------------- 2 files changed, 25 insertions(+), 16 deletions(-)
meta/lib/oe/recipeutils.py +3 -3
@@ -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)
scripts/lib/devtool/standard.py +22 -13
@@ -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':
Signature · 1 line
2.1.0
PATCH v2 5/9 devtool: handle . in recipe name
In reply to Paul EggletonNames such as glib-2.0 are valid (and used) recipe names, so we need to support them.
Fixes [YOCTO #7643].
--- meta/lib/oe/recipeutils.py | 4 ++-- scripts/devtool | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
meta/lib/oe/recipeutils.py +2 -2
@@ -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-'):
scripts/devtool +1 -1
@@ -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:
Signature · 1 line
2.1.0
PATCH v2 6/9 devtool: add: use correct bbappend file name with -V option
In reply to Paul EggletonWe 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].
--- meta/lib/oeqa/selftest/devtool.py | 4 ++-- scripts/lib/devtool/standard.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
meta/lib/oeqa/selftest/devtool.py +2 -2
@@ -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')
scripts/lib/devtool/standard.py +1 -1
@@ -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)
Signature · 1 line
2.1.0
PATCH v2 7/9 devtool: reset: avoid errors in case file no longer exists
In reply to Paul EggletonIf 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.
--- scripts/lib/devtool/standard.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
scripts/lib/devtool/standard.py +8 -1
@@ -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)
Signature · 1 line
2.1.0
PATCH v2 8/9 devtool: update-recipe: handle unversioned bbappends
In reply to Paul EggletonUse 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].
--- scripts/lib/devtool/standard.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
scripts/lib/devtool/standard.py +7 -7
@@ -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()
Signature · 1 line
2.1.0
PATCH v2 9/9 devtool: update-recipe: check if source tree is a git repository
In reply to Paul EggletonIf 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.
--- scripts/lib/devtool/standard.py | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)
scripts/lib/devtool/standard.py +11 -5
@@ -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
Signature · 1 line
2.1.0