Implement build-image plugin for devtool

PATCH In review Ed Bartosh 2 participants 5 messages Aug 24, 2015 – Aug 30, 2015

lore

Share & tools

lore

Ed Bartosh

author

2015-08-24 07:33 UTC #1 lore raw

PATCH 1/2 devtool: make add_md5 a public API

Moved _add_md5 function from standard.py to __init__.py to be able to call it from other modules.

Signed-off-by Ed Bartosh <[email protected]>

---
 scripts/lib/devtool/__init__.py |  6 ++++++
 scripts/lib/devtool/standard.py | 14 ++++----------
 2 files changed, 10 insertions(+), 10 deletions(-)

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

index b54ddf5..1747fff 100644
@@ -110,3 +110,9 @@ def setup_tinfoil(config_only=False):
     tinfoil.logger.setLevel(logger.getEffectiveLevel())
     return tinfoil
 
+def add_md5(config, recipename, filename):
+    """Record checksum of a recipe to the md5-file of the workspace"""
+    import bb.utils
+    md5 = bb.utils.md5_file(filename)
+    with open(os.path.join(config.workspace_path, '.devtool_md5'), 'a') as f:
+        f.write('%s|%s|%s\n' % (recipename, os.path.relpath(filename, config.workspace_path), md5))

scripts/lib/devtool/standard.py +4 ~~-10~~

index e1c5584..4aa6ebd 100644
@@ -26,6 +26,7 @@ import argparse
 import scriptutils
 import errno
 from devtool import exec_build_env_command, setup_tinfoil, DevtoolError
+from devtool import add_md5
 
 logger = logging.getLogger('devtool')

@@ -104,7 +105,7 @@ def add(args, config, basepath, workspace):
     except bb.process.ExecutionError as e:
         raise DevtoolError('Command \'%s\' failed:\n%s' % (e.command, e.stdout))
 
-    _add_md5(config, args.recipename, recipefile)
+    add_md5(config, args.recipename, recipefile)
 
     initial_rev = None
     if os.path.exists(os.path.join(srctree, '.git')):

@@ -120,7 +121,7 @@ def add(args, config, basepath, workspace):
         if initial_rev:
             f.write('\n# initial_rev: %s\n' % initial_rev)
 
-    _add_md5(config, args.recipename, appendfile)
+    add_md5(config, args.recipename, appendfile)
 
     return 0

@@ -371,13 +372,6 @@ def _extract_source(srctree, keep_temp, devbranch, d):
             shutil.rmtree(tempdir)
     return initial_rev
 
-def _add_md5(config, recipename, filename):
-    """Record checksum of a recipe to the md5-file of the workspace"""
-    import bb.utils
-    md5 = bb.utils.md5_file(filename)
-    with open(os.path.join(config.workspace_path, '.devtool_md5'), 'a') as f:
-        f.write('%s|%s|%s\n' % (recipename, os.path.relpath(filename, config.workspace_path), md5))
-
 def _check_preserve(config, recipename):
     """Check if a recipe was manually changed and needs to be saved in 'attic'
        directory"""

@@ -505,7 +499,7 @@ def modify(args, config, basepath, workspace):
             for commit in commits:
                 f.write('# commit: %s\n' % commit)
 
-    _add_md5(config, args.recipename, appendfile)
+    add_md5(config, args.recipename, appendfile)
 
     logger.info('Recipe %s now set up to build from %s' % (args.recipename, srctree))

Signature · 1 line

2.1.4

Ed Bartosh

author

2015-08-24 07:33 UTC #2 lore raw

PATCH 2/2 devtool: implement build-image plugin

Implemented new plugin to build image from workspace packages.

Plugin creates <image>.bbappend file, adds
all workspace packages to the image using IMAGE_INSTALL_append variable in bbappend file. After that it runs 'bitbake <image>'.

Signed-off-by Ed Bartosh <[email protected]>

---
 scripts/lib/devtool/build-image.py | 56 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 scripts/lib/devtool/build-image.py

scripts/lib/devtool/build-image.py new file +56 -0

new file mode 100644
index 0000000..d8e7b12
@@ -0,0 +1,56 @@
+# Development tool - build-image plugin
+#
+# Copyright (C) 2015 Intel Corporation
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program; if not, write to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+"""Devtool plugin containing the build-image subcommand."""
+
+import os
+import logging
+
+from bb.process import ExecutionError
+from devtool import exec_build_env_command, add_md5
+
+LOG = logging.getLogger('devtool')
+
+def plugin_init(pluginlist):
+    """Plugin initialization"""
+    pass
+
+def build_image(args, config, basepath, workspace):
+    """Entry point for the devtool 'build-image' subcommand."""
+    image = args.recipe
+    appendfile = os.path.join(config.workspace_path, 'appends',
+                              '%s.bbappend' % image)
+    with open(appendfile, 'w') as afile:
+        afile.write('IMAGE_INSTALL_append = " %s"\n' % \
+                    ' '.join(workspace.keys()))
+
+    add_md5(config, image, appendfile)
+
+    try:
+        exec_build_env_command(config.init_path, basepath,
+                               'bitbake %s' % image, watch=True)
+    except ExecutionError as err:
+        return err.exitcode
+
+    LOG.info('Successfully built %s', image)
+
+def register_commands(subparsers, context):
+    """Register devtool subcommands from the build-image plugin"""
+    parser_package = subparsers.add_parser('build-image', help='Build image')
+    parser_package.add_argument('recipe', help='Image recipe to build')
+    parser_package.set_defaults(func=build_image)
+

Signature · 1 line

2.1.4

1) Recipes that aren't for the target somehow need to be filtered out (e.g. native recipes). I'd imagine the way to do that would be to parse each one and check if "class-target" is in d.getVar('OVERRIDES', True).split(':').

2) We're making a direct mapping between recipe name and main package for the recipe. That'll work for most recipes but there will be some where that's not valid. I'm tempted to say that since we're parsing the recipe anyway, for now we should do a quick check of PACKAGES to ensure we're not adding something that'll cause the build to fail.

3) It would be nice to add an option to add all the packages in PACKAGES for each recipe to the image; and a separate option to just add the non-dev/-dbg/- staticdev packages. Perhaps we should leave this as a future enhancement though.

4) If there's no target recipes in the workspace then print something like "No recipes in workspace, building image <image> unmodified". On the other hand if there are we should report "Building image <image> with the following additional packages: <packagelist>'

5) If there are recipes can you add the following so that the build system prints something every time the image recipe gets built:

do_rootfs[prefuncs] += "devtool_warn_image_extended"
python devtool_warn_image_extended() {
bb.plain('NOTE: %s: building with additional packages due to "devtool
build-image", delete /path/to/bbappend to clear this' % d.getVar('PN', True)) }

scripts/lib/devtool/build-image.py line 41

Quoted 1 line

scripts/lib/devtool/build-image.py new file +1 -0
+    add_md5(config, image, appendfile)

This isn't actually doing much if we can't use "devtool reset" is it? (Since the recipe isn't officially in the workspace; we could make it so, but then we'd need to ensure the other commands either behave themselves or error out as appropriate if called with an image recipe that's in the workspace.)

Quoted 14 lines

scripts/lib/devtool/build-image.py new file +11 -0

+    try:
+        exec_build_env_command(config.init_path, basepath,
+                               'bitbake %s' % image, watch=True)
+    except ExecutionError as err:
+        return err.exitcode
+
+    LOG.info('Successfully built %s', image)
+
+def register_commands(subparsers, context):
+    """Register devtool subcommands from the build-image plugin"""
+    parser_package = subparsers.add_parser('build-image', help='Build

image')
+ parser_package.add_argument('recipe', help='Image recipe to
build')

scripts/lib/devtool/build-image.py line 56

Quoted 2 lines

scripts/lib/devtool/build-image.py new file +2 -0
+    parser_package.set_defaults(func=build_image)
+

Can you please change help for the command to 'Build image including workspace recipe packages' and add description='Builds an image, extending it to include packages from recipes in the workspace'.

Cheers,
Paul

Signature · 2 lines

Paul Eggleton
Intel Open Source Technology Centre

Ed Bartosh

author

2015-08-30 13:30 UTC #4 lore raw

Re: [PATCH 2/2] devtool: implement build-image plugin

Hi Paul,

Thank you for review!

My answers are below.

Quoted 43 lines · Paul Eggleton

Hi Ed,
Quoted 40 lines · On Monday 24 August 2015 10:33:31 Ed Bartosh wrote:
Implemented new plugin to build image from workspace packages.
Plugin creates <image>.bbappend file, adds
all workspace packages to the image using IMAGE_INSTALL_append variable in bbappend file. After that it runs 'bitbake <image>'.
Signed-off-by Ed Bartosh <[email protected]>
---
 scripts/lib/devtool/build-image.py | 56
++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
 create mode 100644 scripts/lib/devtool/build-image.py
(quoted diff) +0 -0
b/scripts/lib/devtool/build-image.py new file mode 100644 index 0000000..d8e7b12
scripts/lib/devtool/build-image.py new file +22 -0
@@ -0,0 +1,56 @@
+# Development tool - build-image plugin
+#
+# Copyright (C) 2015 Intel Corporation
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License along
+# with this program; if not, write to the Free Software Foundation, Inc.,
+# 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+
+"""Devtool plugin containing the build-image subcommand."""
+
+import os
+import logging
+

Quoted 9 lines

scripts/lib/devtool/build-image.py line 26
Quoted 4 lines
scripts/lib/devtool/build-image.py new file +4 -0
+from bb.process import ExecutionError
+from devtool import exec_build_env_command, add_md5
+
+LOG = logging.getLogger('devtool')
I recall you mentioning pylint had a problem with "logger" - we're using that throughout the devtool code (and elsewhere). "LOG" just looks a bit weird. What is it complaining about specifically?

C: 27, 0: Invalid constant name "logger" (invalid-name)

It's not a big deal. I'll use "logger" in v2.

Quoted 44 lines

scripts/lib/devtool/build-image.py line 39
Quoted 12 lines
scripts/lib/devtool/build-image.py new file +12 -0
+def plugin_init(pluginlist):
+    """Plugin initialization"""
+    pass
+
+def build_image(args, config, basepath, workspace):
+    """Entry point for the devtool 'build-image' subcommand."""
+    image = args.recipe
+    appendfile = os.path.join(config.workspace_path, 'appends',
+                              '%s.bbappend' % image)
+    with open(appendfile, 'w') as afile:
+        afile.write('IMAGE_INSTALL_append = " %s"\n' % \
+                    ' '.join(workspace.keys()))
Some notes here:
1) Recipes that aren't for the target somehow need to be filtered out (e.g. native recipes). I'd imagine the way to do that would be to parse each one and check if "class-target" is in d.getVar('OVERRIDES', True).split(':').
2) We're making a direct mapping between recipe name and main package for the recipe. That'll work for most recipes but there will be some where that's not valid. I'm tempted to say that since we're parsing the recipe anyway, for now we should do a quick check of PACKAGES to ensure we're not adding something that'll cause the build to fail.
3) It would be nice to add an option to add all the packages in PACKAGES for each recipe to the image; and a separate option to just add the non-dev/-dbg/- staticdev packages. Perhaps we should leave this as a future enhancement though.
4) If there's no target recipes in the workspace then print something like "No recipes in workspace, building image <image> unmodified". On the other hand if there are we should report "Building image <image> with the following additional packages: <packagelist>'
5) If there are recipes can you add the following so that the build system prints something every time the image recipe gets built:
do_rootfs[prefuncs] += "devtool_warn_image_extended"
python devtool_warn_image_extended() {
bb.plain('NOTE: %s: building with additional packages due to "devtool
build-image", delete /path/to/bbappend to clear this' % d.getVar('PN', True)) }

Quoted 7 lines

scripts/lib/devtool/build-image.py line 41
Quoted 1 line
scripts/lib/devtool/build-image.py new file +1 -0
+    add_md5(config, image, appendfile)
This isn't actually doing much if we can't use "devtool reset" is it? (Since the recipe isn't officially in the workspace; we could make it so, but then we'd need to ensure the other commands either behave themselves or error out as appropriate if called with an image recipe that's in the workspace.)

Fixed all of the above in v2. I'll send it for review soon.

Quoted 12 lines

scripts/lib/devtool/build-image.py new file +11 -0

+    try:
+        exec_build_env_command(config.init_path, basepath,
+                               'bitbake %s' % image, watch=True)
+    except ExecutionError as err:
+        return err.exitcode
+
+    LOG.info('Successfully built %s', image)
+
+def register_commands(subparsers, context):
+    """Register devtool subcommands from the build-image plugin"""
+    parser_package = subparsers.add_parser('build-image', help='Build

image')

Quoted 9 lines

scripts/lib/devtool/build-image.py line 56
Quoted 4 lines
+ parser_package.add_argument('recipe', help='Image recipe to
build')
scripts/lib/devtool/build-image.py new file +2 -0
+    parser_package.set_defaults(func=build_image)
+
Can you please change help for the command to 'Build image including workspace recipe packages' and add description='Builds an image, extending it to include packages from recipes in the workspace'.

Fixed in v2 too.

Regards,
Ed