[PATCH 0/2] Implement build-image plugin for devtool

[PATCH 0/2] Implement build-image plugin for devtool

[Ed Bartosh]

Hi,

Please, review implamentation of the new plugin for devtool.
This plugins is for building an image with all workspace packages included.

The following changes since commit a3d228a484cf471e999b6537987e2897d8a7bd4e:

  bitbake: main: Handle RuntimeError exception in list_extension_modules (2015-08-20 22:04:43 +0100)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib ed/devtool/8135
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=ed/devtool/8135

Ed Bartosh (2):
  devtool: make add_md5 a public API
  devtool: implement build-image plugin

 scripts/lib/devtool/__init__.py    |  6 ++++
 scripts/lib/devtool/build-image.py | 56 ++++++++++++++++++++++++++++++++++++++
 scripts/lib/devtool/standard.py    | 14 +++-------
 3 files changed, 66 insertions(+), 10 deletions(-)
 create mode 100644 scripts/lib/devtool/build-image.py

--
Ed

[PATCH 1/2] devtool: make add_md5 a public API

[Ed Bartosh]

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

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/devtool/__init__.py |  6 ++++++
 scripts/lib/devtool/standard.py | 14 ++++----------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/scripts/lib/devtool/__init__.py b/scripts/lib/devtool/__init__.py
index b54ddf5..1747fff 100644
--- a/scripts/lib/devtool/__init__.py
+++ b/scripts/lib/devtool/__init__.py
@@ -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))
diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index e1c5584..4aa6ebd 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -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))
 
-- 
2.1.4

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

[Ed Bartosh]

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 <ed.bartosh@linux.intel.com>
---
 scripts/lib/devtool/build-image.py | 56 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 scripts/lib/devtool/build-image.py

diff --git a/scripts/lib/devtool/build-image.py b/scripts/lib/devtool/build-image.py
new file mode 100644
index 0000000..d8e7b12
--- /dev/null
+++ b/scripts/lib/devtool/build-image.py
@@ -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)
+
-- 
2.1.4

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

[Paul Eggleton]

Hi Ed,

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 <ed.bartosh@linux.intel.com>
> ---
>  scripts/lib/devtool/build-image.py | 56
> ++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
>  create mode 100644 scripts/lib/devtool/build-image.py
> 
> diff --git a/scripts/lib/devtool/build-image.py
> b/scripts/lib/devtool/build-image.py new file mode 100644
> index 0000000..d8e7b12
> --- /dev/null
> +++ b/scripts/lib/devtool/build-image.py
> @@ -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')

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?

> +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))
}

> +    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.)

> +    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)
> +

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

-- 

Paul Eggleton
Intel Open Source Technology Centre

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

[Ed Bartosh]

Hi Paul,

Thank you for review!

My answers are below.

On Thu, Aug 27, 2015 at 03:07:37PM +0100, Paul Eggleton wrote:
> Hi Ed,
> 
> 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 <ed.bartosh@linux.intel.com>
> > ---
> >  scripts/lib/devtool/build-image.py | 56
> > ++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
> >  create mode 100644 scripts/lib/devtool/build-image.py
> > 
> > diff --git a/scripts/lib/devtool/build-image.py
> > b/scripts/lib/devtool/build-image.py new file mode 100644
> > index 0000000..d8e7b12
> > --- /dev/null
> > +++ b/scripts/lib/devtool/build-image.py
> > @@ -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')
> 
> 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.

> > +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))
> }
> 
> > +    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.

> > +    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)
> > +
> 
> 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