[PATCH 0/2] Two new devtool plugins: export and import

[PATCH 0/2] Two new devtool plugins: export and import

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

These two plugins can be used to share the devtool workspace between uses.
On the import plugin, the are some limitations with the proposed patche:

  * It cannot not limit what is being imported, thus all exported
    workspace is imported for the moment.

  * It is not smart enough to see if the imported metadata is compatible
    as explained on [1], thus after import, the may be issues when bulding the particular
    recipe, i.e imported bbappend does not has the bb counterpart, etc.

[1] https://bugzilla.yoctoproject.org/show_bug.cgi?id=10510#c3

The following changes since commit ef506f58da3a95fba2696df749b2b81f9c118847:

  cve-check-tool: backport a patch to make CVE checking work (2017-05-18 14:01:48 +0100)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib lsandov1/devtool-import-export
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=lsandov1/devtool-import-export

Leonardo Sandoval (2):
  export: new plugin to export the devtool workspace
  import: new plugin to import the devtool workspace

 scripts/lib/devtool/export.py | 94 ++++++++++++++++++++++++++++++++++++++++
 scripts/lib/devtool/import.py | 99 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 193 insertions(+)
 create mode 100644 scripts/lib/devtool/export.py
 create mode 100644 scripts/lib/devtool/import.py

-- 
2.12.0

[PATCH 1/2] export: new plugin to export the devtool workspace

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

By default, exports the whole workspace (all recipes) including the source code.
User can also limit what is exported with --included/--excluded flags. As
a result of this operation, a tar archive containing only workspace metadata
and its corresponding source code is created, which can be properly imported
with 'devtool import'.

https://bugzilla.yoctoproject.org/show_bug.cgi?id=10510

[YOCTO #10510]

Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
---
 scripts/lib/devtool/export.py | 94 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)
 create mode 100644 scripts/lib/devtool/export.py

diff --git a/scripts/lib/devtool/export.py b/scripts/lib/devtool/export.py
new file mode 100644
index 00000000000..8e3ba8ca68b
--- /dev/null
+++ b/scripts/lib/devtool/export.py
@@ -0,0 +1,94 @@
+# Development tool - export command plugin
+#
+# Copyright (C) 2014-2017 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 export plugin"""
+
+import os
+import argparse
+import tarfile
+import logging
+
+logger = logging.getLogger('devtool')
+default_arcname = "workspace.tar"
+
+def export(args, config, basepath, workspace):
+    """Entry point for the devtool 'export' subcommand"""
+
+    def create_arcname(name):
+        """Create arc name by removing the workspace path or $HOME prefix from name"""
+        _name = name
+        if name.startswith(config.workspace_path):
+            _name = name.replace(config.workspace_path, '')
+        else:
+            _name = name.replace(os.environ['HOME'], '')
+        return (name, _name)
+
+    def reset(tarinfo):
+        tarinfo.uname = tarinfo.gname = "nobody"
+        return tarinfo
+
+    def add(tar, value):
+        # Get  arcnames
+        arcnames = []
+        for key in ['srctree', 'bbappend', 'recipefile']:
+            if key in value and value[key]:
+                arcnames.append(create_arcname(value[key]))
+
+        # Archive
+        for name, arcname in arcnames:
+            tar.add(name, arcname=arcname, filter=reset)
+
+    # include the default archiver filename if missing
+    name = args.name
+    if os.path.isdir(name):
+        if name[-1] != '/':
+            name = name + '/'
+        name = name + default_arcname
+
+    if os.path.exists(name) and not args.force:
+        logger.error('Tar archive %s exists. Use -f to force removal')
+        return 1
+
+    included = []
+    with tarfile.open(name, "w") as tar:
+        if args.include:
+            for recipe in args.include:
+                if recipe in workspace:
+                    add(tar, workspace[recipe])
+                    included.append(recipe)
+                else:
+                    logger.warn('recipe %s not in workspace, not in archive file')
+        else:
+            for recipe, value in workspace.items():
+                if recipe not in args.exclude:
+                    add(tar, value)
+                    included.append(recipe)
+
+    logger.info('Tar archive create at %s with the following recipes: %s' % (name, included))
+
+def register_commands(subparsers, context):
+    """Register devtool export subcommands"""
+    parser = subparsers.add_parser('export',
+                                   help='Export workspace into a tar archive',
+                                   description='Export one or more recipes from current workspace into a tar archive',
+                                   group='advanced')
+
+    parser.add_argument('--name', '-n', default=default_arcname, help='Name of the tar archive')
+    parser.add_argument('--force', '-f', action="store_true", help='Overwrite previous export tar archive')
+    group = parser.add_mutually_exclusive_group()
+    group.add_argument('--include', '-i', nargs='+', default=[], help='Include the defined recipes into the tar archive')
+    group.add_argument('--exclude', '-e', nargs='+', default=[], help='Exclude the defined recipes into the tar archive')
+    parser.set_defaults(func=export)
-- 
2.12.0

Re: [PATCH 1/2] export: new plugin to export the devtool workspace

[Paul Eggleton]

Hi Leo,

A few notes below.

On Friday, 26 May 2017 9:31:47 AM NZST leonardo.sandoval.gonzalez@linux.intel.com wrote:
> From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
> 
> By default, exports the whole workspace (all recipes) including the source
> code.
> User can also limit what is exported with --included/--excluded flags. As
> a result of this operation, a tar archive containing only workspace metadata
> and its corresponding source code is created, which can be properly imported
> with 'devtool import'.
> 
> https://bugzilla.yoctoproject.org/show_bug.cgi?id=10510
> 
> [YOCTO #10510]
> 
> Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
> ---
>  scripts/lib/devtool/export.py | 94 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 94 insertions(+)
>  create mode 100644 scripts/lib/devtool/export.py
> 
> diff --git a/scripts/lib/devtool/export.py b/scripts/lib/devtool/export.py
> new file mode 100644
> index 00000000000..8e3ba8ca68b
> --- /dev/null
> +++ b/scripts/lib/devtool/export.py
> @@ -0,0 +1,94 @@
> +# Development tool - export command plugin
> +#
> +# Copyright (C) 2014-2017 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 export plugin"""
> +
> +import os
> +import argparse
> +import tarfile
> +import logging
> +
> +logger = logging.getLogger('devtool')
> +default_arcname = "workspace.tar"

This should have the date and time in it.  I'd probably structure it
differently but this will work:

import datetime
...
default_arcname = "workspace-export-%s.tar" % datetime.datetime.now().strftime('%Y%m%d%H%M%S'))


> +
> +def export(args, config, basepath, workspace):
> +    """Entry point for the devtool 'export' subcommand"""
> +
> +    def create_arcname(name):
> +        """Create arc name by removing the workspace path or $HOME prefix from name"""
> +        _name = name
> +        if name.startswith(config.workspace_path):
> +            _name = name.replace(config.workspace_path, '')
> +        else:
> +            _name = name.replace(os.environ['HOME'], '')
> +        return (name, _name)

I really don't like this. I'd much rather we explicitly handle each item - we
can assume that the recipe and bbappend will be under the workspace (since
they have to be). For the sources which can potentially be anywhere, we can
just restore these under sources/ on the other side.

> +    def reset(tarinfo):
> +        tarinfo.uname = tarinfo.gname = "nobody"

Why "nobody"? Is there a reason it shouldn't be left as the user that did the
export?

> +        return tarinfo
> +
> +    def add(tar, value):
> +        # Get  arcnames
> +        arcnames = []
> +        for key in ['srctree', 'bbappend', 'recipefile']:
> +            if key in value and value[key]:
> +                arcnames.append(create_arcname(value[key]))
> +
> +        # Archive
> +        for name, arcname in arcnames:
> +            tar.add(name, arcname=arcname, filter=reset)
> +
> +    # include the default archiver filename if missing
> +    name = args.name
> +    if os.path.isdir(name):
> +        if name[-1] != '/':
> +            name = name + '/'
> +        name = name + default_arcname
> +
> +    if os.path.exists(name) and not args.force:
> +        logger.error('Tar archive %s exists. Use -f to force removal')

"to force removal" -> "to overwrite it" (also see below)

> +        return 1
> +
> +    included = []
> +    with tarfile.open(name, "w") as tar:
> +        if args.include:

So here I would much rather we check both args.include and args.exclude
to ensure that they are completely valid and if not then exit with an error.
That reduces the likelihood of users mistyping a name and assuming that
a recipe is in the export when it isn't (or vice versa).

> +            for recipe in args.include:
> +                if recipe in workspace:
> +                    add(tar, workspace[recipe])
> +                    included.append(recipe)
> +                else:
> +                    logger.warn('recipe %s not in workspace, not in archive
> file')
> +        else:
> +            for recipe, value in workspace.items():
> +                if recipe not in args.exclude:
> +                    add(tar, value)
> +                    included.append(recipe)
> +
> +    logger.info('Tar archive create at %s with the following recipes: %s' % (name, included))

s/create/created/

Also replace included with ', '.join(included) for a bit neater presentation.

> +
> +def register_commands(subparsers, context):
> +    """Register devtool export subcommands"""
> +    parser = subparsers.add_parser('export',
> +                                   help='Export workspace into a tar archive',
> +                                   description='Export one or more recipes from current workspace into a tar archive',
> +                                   group='advanced')
> +
> +    parser.add_argument('--name', '-n', default=default_arcname, help='Name of the tar archive')

Can you make this -f / --file to match tar?

> +    parser.add_argument('--force', '-f', action="store_true", help='Overwrite previous export tar archive')

Rather than "force" this should be "overwrite".

Thanks,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

[PATCH 2/2] import: new plugin to import the devtool workspace

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

Takes a tar archive created by 'devtool export' and export it (untar) to
workspace. By default, the whole tar archive is imported, thus there is no
way to limit what is imported.

https://bugzilla.yoctoproject.org/show_bug.cgi?id=10510

[YOCTO #10510]

Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
---
 scripts/lib/devtool/import.py | 99 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 99 insertions(+)
 create mode 100644 scripts/lib/devtool/import.py

diff --git a/scripts/lib/devtool/import.py b/scripts/lib/devtool/import.py
new file mode 100644
index 00000000000..ff770c321f4
--- /dev/null
+++ b/scripts/lib/devtool/import.py
@@ -0,0 +1,99 @@
+# Development tool - import command plugin
+#
+# Copyright (C) 2014-2017 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 import plugin"""
+
+import os
+import tarfile
+import logging
+import re
+from devtool import standard
+
+logger = logging.getLogger('devtool')
+
+def devimport(args, config, basepath, workspace):
+    """Entry point for the devtool 'import' subcommand"""
+    if not os.path.exists(args.name):
+        logger.error('Tar archive %s does not exist. The expected archive should be created with "devtool export"')
+        return 1
+
+    # match exported workspace folders
+    prog = re.compile('recipes|appends|sources')
+
+    def get_prefix(name):
+        """Prefix the workspace path or $HOME to the member name"""
+        _prefix = ""
+        if prog.match(name):
+            _prefix = config.workspace_path + '/'
+        else:
+            if not name.startswith('/'):
+                _prefix = os.environ['HOME'] + '/'
+
+        return _prefix, _prefix + name
+
+    # include the default archiver filename if missing
+    name = args.name
+    if os.path.isdir(name):
+        if name[-1] != '/':
+            name = name + '/'
+        name = name + default_arcname
+
+    if not os.path.exists(name):
+        logger.error('Tar archive %s does not exists. Export your workspace using "devtool export"')
+        return 1
+
+    included = []
+    with tarfile.open(name) as tar:
+        for member in tar.getmembers():
+            prefix, path = get_prefix(member.name)
+            if os.path.exists(path):
+                if args.force:
+                    try:
+                        tar.extract(member, path=prefix)
+                    except PermissionError as pe:
+                        logger.warn(pe)
+                else:
+                    logger.warn('File already present, add -f to overwrite: %s' % member.name)
+            else:
+                tar.extract(member, path=prefix)
+
+            # md5 creation just for recipes or appends
+            if member.name.startswith('recipes') or member.name.startswith('appends'):
+                dirpath, recipe = os.path.split(member.name)
+                recipename = ""
+                for sep in "_ .".split():
+                    if sep in recipe:
+                        recipename = recipe.split(sep)[0]
+                        break
+                if not recipename:
+                    logger.warn('Recipe name could not be extracted from %s' % member.name)
+                    recipename = recipe
+
+                standard._add_md5(config, recipename, path)
+                if recipename not in included:
+                    included.append(recipename)
+
+    logger.info('Imported recipes into workspace %s: %s' % (config.workspace_path, included))
+
+def register_commands(subparsers, context):
+    """Register devtool import subcommands"""
+    parser = subparsers.add_parser('import',
+                                   help='Import tar archive into workspace',
+                                   description='Import previously created tar archive into the workspace',
+                                   group='advanced')
+    parser.add_argument('--name', '-n', default='workspace.tar', help='Name of the tar archive to import')
+    parser.add_argument('--force', '-f', action="store_true", help='Overwrite previous files')
+    parser.set_defaults(func=devimport)
-- 
2.12.0

Re: [PATCH 2/2] import: new plugin to import the devtool workspace

[Paul Eggleton]

On Friday, 26 May 2017 9:31:48 AM NZST leonardo.sandoval.gonzalez@linux.intel.com wrote:
> From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
> 
> Takes a tar archive created by 'devtool export' and export it (untar) to
> workspace. By default, the whole tar archive is imported, thus there is no
> way to limit what is imported.
> 
> https://bugzilla.yoctoproject.org/show_bug.cgi?id=10510
> 
> [YOCTO #10510]
> 
> Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
> ---
>  scripts/lib/devtool/import.py | 99 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 99 insertions(+)
>  create mode 100644 scripts/lib/devtool/import.py
> 
> diff --git a/scripts/lib/devtool/import.py b/scripts/lib/devtool/import.py
> new file mode 100644
> index 00000000000..ff770c321f4
> --- /dev/null
> +++ b/scripts/lib/devtool/import.py
> @@ -0,0 +1,99 @@
> +# Development tool - import command plugin
> +#
> +# Copyright (C) 2014-2017 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 import plugin"""
> +
> +import os
> +import tarfile
> +import logging
> +import re
> +from devtool import standard
> +
> +logger = logging.getLogger('devtool')
> +
> +def devimport(args, config, basepath, workspace):
> +    """Entry point for the devtool 'import' subcommand"""
> +    if not os.path.exists(args.name):
> +        logger.error('Tar archive %s does not exist. The expected archive should be created with "devtool export"')
> +        return 1
> +
> +    # match exported workspace folders
> +    prog = re.compile('recipes|appends|sources')
> +
> +    def get_prefix(name):
> +        """Prefix the workspace path or $HOME to the member name"""
> +        _prefix = ""
> +        if prog.match(name):
> +            _prefix = config.workspace_path + '/'
> +        else:
> +            if not name.startswith('/'):
> +                _prefix = os.environ['HOME'] + '/'
> +
> +        return _prefix, _prefix + name
> +
> +    # include the default archiver filename if missing
> +    name = args.name
> +    if os.path.isdir(name):
> +        if name[-1] != '/':
> +            name = name + '/'
> +        name = name + default_arcname

If the date is in the export we can't use this default - see below.

> +
> +    if not os.path.exists(name):
> +        logger.error('Tar archive %s does not exists. Export your workspace using "devtool export"')

"exists" -> "exist". We should also drop the "export your workspace" bit.

> +        return 1
> +
> +    included = []
> +    with tarfile.open(name) as tar:
> +        for member in tar.getmembers():
> +            prefix, path = get_prefix(member.name)
> +            if os.path.exists(path):
> +                if args.force:
> +                    try:
> +                        tar.extract(member, path=prefix)
> +                    except PermissionError as pe:
> +                        logger.warn(pe)
> +                else:
> +                    logger.warn('File already present, add -f to overwrite: %s' % member.name)
> +            else:
> +                tar.extract(member, path=prefix)
> +
> +            # md5 creation just for recipes or appends
> +            if member.name.startswith('recipes') or member.name.startswith('appends'):
> +                dirpath, recipe = os.path.split(member.name)
> +                recipename = ""
> +                for sep in "_ .".split():
> +                    if sep in recipe:
> +                        recipename = recipe.split(sep)[0]
> +                        break
> +                if not recipename:
> +                    logger.warn('Recipe name could not be extracted from %s' % member.name)
> +                    recipename = recipe
> +
> +                standard._add_md5(config, recipename, path)
> +                if recipename not in included:
> +                    included.append(recipename)
> +
> +    logger.info('Imported recipes into workspace %s: %s' % (config.workspace_path, included))
> +
> +def register_commands(subparsers, context):
> +    """Register devtool import subcommands"""
> +    parser = subparsers.add_parser('import',
> +                                   help='Import tar archive into workspace',
> +                                   description='Import previously created tar archive into the workspace',
> +                                   group='advanced')
> +    parser.add_argument('--name', '-n', default='workspace.tar', help='Name of the tar archive to import')

Remove the default here - actually I suspect we should just make this a
mandatory positional argument.

> +    parser.add_argument('--force', '-f', action="store_true", help='Overwrite previous files')

As per export this should be "overwrite" rather than "force".

There are a couple of things missing from this:

1) You noted that it doesn't check if there are recipes to match the
bbappends - we do need to do this as part of the initial version.
tinfoil.cooker.recipecaches[''].pkg_fn.items() should give you a list of all
recipe files that you can use to verify this.

2) The bbappends contain absolute paths that will need to be updated e.g.
EXTERNALSRC / EXTERNALSRC_BUILD - these will almost certainly be different
between the export and import machine if this functionality is used to send
workspace content between different users.

Cheers,
Paul


-- 

Paul Eggleton
Intel Open Source Technology Centre

[PATCH v2 0/3] Two new devtool plugins: export and import

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

These two plugins can be used to share the devtool workspace between users.

Changes from v1 (mostly suggested by Paul E.)

Export:
    * metatada file created and included on tar, containing workspace data (to be consume by
    the import plugin)
    * tar archive contains a timestamp, making effectively unique for ever export
    * Exclude or include recipes are validated against current workspace
    * change on parameter naming, following the tar command args

Import:
    * the tar archive to be imported must be given from the commmand line
    * Before importing, bbappends must have its corresponding recipe
    * EXTERNALSRC correctly modified reflecting imported workspace

The following changes since commit ef506f58da3a95fba2696df749b2b81f9c118847:

  cve-check-tool: backport a patch to make CVE checking work (2017-05-18 14:01:48 +0100)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib lsandov1/devtool-import-export
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=lsandov1/devtool-import-export

Leonardo Sandoval (3):
  export: new plugin to export the devtool workspace
  ftools: new function to replace strings inside a text file
  import: new plugin to import the devtool workspace

 meta/lib/oeqa/utils/ftools.py |  22 +++++++
 scripts/lib/devtool/export.py | 120 ++++++++++++++++++++++++++++++++++++++
 scripts/lib/devtool/import.py | 130 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 272 insertions(+)
 create mode 100644 scripts/lib/devtool/export.py
 create mode 100644 scripts/lib/devtool/import.py

-- 
2.12.0

[PATCH v2 1/3] export: new plugin to export the devtool workspace

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

By default, exports the whole workspace (all recipes) including the source code.
User can also limit what is exported with --included/--excluded flags. As
a result of this operation, a tar archive containing only workspace metadata
and its corresponding source code is created, which can be properly imported
with 'devtool import'.

https://bugzilla.yoctoproject.org/show_bug.cgi?id=10510

[YOCTO #10510]

Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
---
 scripts/lib/devtool/export.py | 120 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 120 insertions(+)
 create mode 100644 scripts/lib/devtool/export.py

diff --git a/scripts/lib/devtool/export.py b/scripts/lib/devtool/export.py
new file mode 100644
index 0000000000..64ac85162b
--- /dev/null
+++ b/scripts/lib/devtool/export.py
@@ -0,0 +1,120 @@
+# Development tool - export command plugin
+#
+# Copyright (C) 2014-2017 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 export plugin"""
+
+import os
+import argparse
+import tarfile
+import logging
+import datetime
+import json
+
+logger = logging.getLogger('devtool')
+
+# output files
+default_arcname_prefix = "workspace-export"
+default_arcname_link = default_arcname_prefix + '.tar'
+metadata = '.export_metadata'
+
+def export(args, config, basepath, workspace):
+    """Entry point for the devtool 'export' subcommand"""
+
+    def add_metadata(tar):
+        """Archive the workspace object"""
+        # finally store the workspace metadata
+        with open(metadata, 'w') as fd:
+            fd.write(json.dumps((config.workspace_path, workspace)))
+        tar.add(metadata)
+        os.unlink(metadata)
+
+    def add_recipe(tar, recipe, data):
+        """Archive recipe with proper arcname"""
+        # Create a map of name/arcnames
+        arcnames = []
+        for key, name in data.items():
+            if name:
+                if key == 'srctree':
+                    # all sources, no matter where are located, goes into the sources directory
+                    arcname = 'sources/%s' % recipe
+                else:
+                    arcname = name.replace(config.workspace_path, '')
+                arcnames.append((name, arcname))
+
+        for name, arcname in arcnames:
+            tar.add(name, arcname=arcname)
+
+
+    # Make sure workspace is non-empty and possible listed include/excluded recipes are in workspace
+    if not workspace:
+        logger.info('Workspace has not recipes, nothing to export')
+        return 0
+    else:
+        for param, recipes in {'include':args.include,'exclude':args.exclude}.items():
+            for recipe in recipes:
+                if recipe not in workspace:
+                    logger.error('Recipe (%s) on %s argument not in the current workspace' % (recipe, param))
+                    return 1
+
+    name = args.file
+
+    default_name = "%s-%s.tar" % (default_arcname_prefix, datetime.datetime.now().strftime('%Y%m%d%H%M%S'))
+    if not name:
+        name = default_name
+    else:
+        # if name is a directory, append the default name
+        if os.path.isdir(name):
+            name = os.path.join(name, default_name)
+
+    if os.path.exists(name) and not args.overwrite:
+        logger.error('Tar archive %s exists. Use --overwrite/-o to overwrite it')
+        return 1
+
+    # if all workspace is excluded, quit
+    if not len(set(workspace.keys()).difference(set(args.exclude))):
+        logger.warn('All recipes in workspace excluded, no tar archive created')
+        return 0
+
+    exported = []
+    with tarfile.open(name, 'w') as tar:
+        if args.include:
+            for recipe in args.include:
+                add_recipe(tar, recipe, workspace[recipe])
+                exported.append(recipe)
+        else:
+            for recipe, data in workspace.items():
+                if recipe not in args.exclude:
+                    add_recipe(tar, recipe, data)
+                    exported.append(recipe)
+
+        add_metadata(tar)
+
+    logger.info('Tar archive created at %s with the following recipes: %s' % (name, ' '.join(exported)))
+    return 0
+
+def register_commands(subparsers, context):
+    """Register devtool export subcommands"""
+    parser = subparsers.add_parser('export',
+                                   help='Export workspace into a tar archive',
+                                   description='Export one or more recipes from current workspace into a tar archive',
+                                   group='advanced')
+
+    parser.add_argument('--file', '-f', help='Use archive file or device')
+    parser.add_argument('--overwrite', '-o', action="store_true", help='Overwrite previous export tar archive')
+    group = parser.add_mutually_exclusive_group()
+    group.add_argument('--include', '-i', nargs='+', default=[], help='Include recipes into the tar archive')
+    group.add_argument('--exclude', '-e', nargs='+', default=[], help='Exclude recipes into the tar archive')
+    parser.set_defaults(func=export)
-- 
2.12.0

Re: [PATCH v2 1/3] export: new plugin to export the devtool workspace

[Paul Eggleton]

Hi Leo,

This looks OK. However I just noticed - the output tarfile isn't compressed. Can we enable gzip compression? In practice that means mode should be 'w:gz' when writing out and also .tar.gz instead of .tar extension.

A couple of minor message notes below as well.


On Friday, 2 June 2017 7:12:59 PM CEST leonardo.sandoval.gonzalez@linux.intel.com wrote:
> From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
> 
> By default, exports the whole workspace (all recipes) including the source code.
> User can also limit what is exported with --included/--excluded flags. As
> a result of this operation, a tar archive containing only workspace metadata
> and its corresponding source code is created, which can be properly imported
> with 'devtool import'.
> 
> https://bugzilla.yoctoproject.org/show_bug.cgi?id=10510
> 
> [YOCTO #10510]
> 
> Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
> ---
>  scripts/lib/devtool/export.py | 120 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 120 insertions(+)
>  create mode 100644 scripts/lib/devtool/export.py
> 
> diff --git a/scripts/lib/devtool/export.py b/scripts/lib/devtool/export.py
> new file mode 100644
> index 0000000000..64ac85162b
> --- /dev/null
> +++ b/scripts/lib/devtool/export.py
> @@ -0,0 +1,120 @@
> +# Development tool - export command plugin
> +#
> +# Copyright (C) 2014-2017 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 export plugin"""
> +
> +import os
> +import argparse
> +import tarfile
> +import logging
> +import datetime
> +import json
> +
> +logger = logging.getLogger('devtool')
> +
> +# output files
> +default_arcname_prefix = "workspace-export"
> +default_arcname_link = default_arcname_prefix + '.tar'
> +metadata = '.export_metadata'
> +
> +def export(args, config, basepath, workspace):
> +    """Entry point for the devtool 'export' subcommand"""
> +
> +    def add_metadata(tar):
> +        """Archive the workspace object"""
> +        # finally store the workspace metadata
> +        with open(metadata, 'w') as fd:
> +            fd.write(json.dumps((config.workspace_path, workspace)))
> +        tar.add(metadata)
> +        os.unlink(metadata)
> +
> +    def add_recipe(tar, recipe, data):
> +        """Archive recipe with proper arcname"""
> +        # Create a map of name/arcnames
> +        arcnames = []
> +        for key, name in data.items():
> +            if name:
> +                if key == 'srctree':
> +                    # all sources, no matter where are located, goes into the sources directory
> +                    arcname = 'sources/%s' % recipe
> +                else:
> +                    arcname = name.replace(config.workspace_path, '')
> +                arcnames.append((name, arcname))
> +
> +        for name, arcname in arcnames:
> +            tar.add(name, arcname=arcname)
> +
> +
> +    # Make sure workspace is non-empty and possible listed include/excluded recipes are in workspace
> +    if not workspace:
> +        logger.info('Workspace has not recipes, nothing to export')

"Workspace has not recipes" -> "Workspace contains no recipes"


> +        return 0
> +    else:
> +        for param, recipes in {'include':args.include,'exclude':args.exclude}.items():
> +            for recipe in recipes:
> +                if recipe not in workspace:
> +                    logger.error('Recipe (%s) on %s argument not in the current workspace' % (recipe, param))
> +                    return 1
> +
> +    name = args.file
> +
> +    default_name = "%s-%s.tar" % (default_arcname_prefix, datetime.datetime.now().strftime('%Y%m%d%H%M%S'))
> +    if not name:
> +        name = default_name
> +    else:
> +        # if name is a directory, append the default name
> +        if os.path.isdir(name):
> +            name = os.path.join(name, default_name)
> +
> +    if os.path.exists(name) and not args.overwrite:
> +        logger.error('Tar archive %s exists. Use --overwrite/-o to overwrite it')
> +        return 1
> +
> +    # if all workspace is excluded, quit
> +    if not len(set(workspace.keys()).difference(set(args.exclude))):
> +        logger.warn('All recipes in workspace excluded, no tar archive created')

"no tar archive created" -> "nothing to export"


> +        return 0
> +
> +    exported = []
> +    with tarfile.open(name, 'w') as tar:
> +        if args.include:
> +            for recipe in args.include:
> +                add_recipe(tar, recipe, workspace[recipe])
> +                exported.append(recipe)
> +        else:
> +            for recipe, data in workspace.items():
> +                if recipe not in args.exclude:
> +                    add_recipe(tar, recipe, data)
> +                    exported.append(recipe)
> +
> +        add_metadata(tar)
> +
> +    logger.info('Tar archive created at %s with the following recipes: %s' % (name, ' '.join(exported)))

', '.join() makes for a slightly more readable output.

> +    return 0
> +
> +def register_commands(subparsers, context):
> +    """Register devtool export subcommands"""
> +    parser = subparsers.add_parser('export',
> +                                   help='Export workspace into a tar archive',
> +                                   description='Export one or more recipes from current workspace into a tar archive',
> +                                   group='advanced')
> +
> +    parser.add_argument('--file', '-f', help='Use archive file or device')

For the -f help text I think we ought to just say "Output archive file name".

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

[PATCH v2 2/3] ftools: new function to replace strings inside a text file

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
---
 meta/lib/oeqa/utils/ftools.py | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/meta/lib/oeqa/utils/ftools.py b/meta/lib/oeqa/utils/ftools.py
index a7233d4ca6..8bf2547d68 100644
--- a/meta/lib/oeqa/utils/ftools.py
+++ b/meta/lib/oeqa/utils/ftools.py
@@ -44,3 +44,25 @@ def remove_from_file(path, data):
         except ValueError:
             pass
     write_file(path, "\n".join(contents))
+
+def replace_from_file(path, old, new):
+    # In case old is None, return immediately
+    if old is None:
+        return
+    try:
+        rdata = read_file(path)
+    except IOError as e:
+        # if file does not exit, just quit, otherwise raise an exception
+        if e.errno == errno.ENOENT:
+            return
+        else:
+            raise
+
+    old_contents = rdata.splitlines()
+    new_contents = []
+    for old_content in old_contents:
+        try:
+            new_contents.append(old_content.replace(old, new))
+        except ValueError:
+            pass
+    write_file(path, "\n".join(new_contents))
-- 
2.12.0

[PATCH v2 3/3] import: new plugin to import the devtool workspace

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

Takes a tar archive created by 'devtool export' and export it (untar) to
workspace. By default, the whole tar archive is imported, thus there is no
way to limit what is imported.

https://bugzilla.yoctoproject.org/show_bug.cgi?id=10510

[YOCTO #10510]

Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
---
 scripts/lib/devtool/import.py | 130 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 130 insertions(+)
 create mode 100644 scripts/lib/devtool/import.py

diff --git a/scripts/lib/devtool/import.py b/scripts/lib/devtool/import.py
new file mode 100644
index 0000000000..5b85571669
--- /dev/null
+++ b/scripts/lib/devtool/import.py
@@ -0,0 +1,130 @@
+# Development tool - import command plugin
+#
+# Copyright (C) 2014-2017 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 import plugin"""
+
+import os
+import tarfile
+import logging
+import re
+from devtool import standard, setup_tinfoil
+from devtool import export
+
+import oeqa.utils.ftools as ftools
+import json
+
+logger = logging.getLogger('devtool')
+
+def devimport(args, config, basepath, workspace):
+    """Entry point for the devtool 'import' subcommand"""
+
+    def get_pn(name):
+        dirpath, recipe = os.path.split(name)
+        pn = ""
+        for sep in "_ .".split():
+            if sep in recipe:
+                pn = recipe.split(sep)[0]
+                break
+        return pn
+    
+    # match exported workspace folders
+    prog = re.compile('recipes|appends|sources')
+
+    if not os.path.exists(args.file):
+        logger.error('Tar archive %s does not exist. Export your workspace using "devtool export"')
+        return 1
+
+    # get current recipes
+    current_recipes = []
+    try:
+        tinfoil = setup_tinfoil(config_only=False, basepath=basepath)
+        current_recipes = [recipe[1] for recipe in tinfoil.cooker.recipecaches[''].pkg_fn.items()]
+    finally:
+        tinfoil.shutdown()
+
+    imported = []
+    with tarfile.open(args.file) as tar:
+
+        # get exported metadata so values containing paths can be automatically replaced it
+        export_workspace_path = export_workspace = None
+        try:
+            metadata = tar.getmember(export.metadata)
+            tar.extract(metadata)
+            with open(metadata.name) as fdm:
+                export_workspace_path, export_workspace = json.load(fdm)
+            os.unlink(metadata.name)
+        except KeyError as ke:
+            logger.warn('The export metadata file created by "devtool export" was not found')
+            logger.warn('Manual editing is needed to correct paths on imported recipes/appends')
+
+        members = [member for member in tar.getmembers() if member.name != export.metadata]
+        for member in members:
+            # make sure imported bbappend has its corresponding recipe (bb)
+            if member.name.startswith('appends'):
+                bbappend = get_pn(member.name)
+                if bbappend:
+                    if bbappend not in current_recipes:
+                        # check that the recipe is not in the tar archive being imported
+                        if bbappend not in export_workspace:
+                            logger.warn('No recipe to append %s, skipping' % bbappend)
+                            continue
+                else:
+                    logger.warn('bbappend name %s could not be detected' % member.name)
+                    continue
+
+            # extract file from tar
+            path = os.path.join(config.workspace_path, member.name)
+            if os.path.exists(path):
+                if args.overwrite:
+                    try:
+                        tar.extract(member, path=config.workspace_path)
+                    except PermissionError as pe:
+                        logger.warn(pe)
+                else:
+                    logger.warn('File already present. Use --overwrite/-o to overwrite it: %s' % member.name)
+            else:
+                tar.extract(member, path=config.workspace_path)
+
+            if member.name.startswith('appends'):
+                recipe = get_pn(member.name)
+
+                # Update EXTERNARLSRC
+                if export_workspace_path:
+                    # appends created by 'devtool modify' just need to update the workspace
+                    ftools.replace_from_file(path, export_workspace_path, config.workspace_path)
+
+                    # appends created by 'devtool add' need replacement of exported source tree
+                    exported_srctree = export_workspace[recipe]['srctree']
+                    if exported_srctree:
+                        ftools.replace_from_file(path, exported_srctree, os.path.join(config.workspace_path, 'sources', recipe))
+
+                # update .devtool_md5 file
+                standard._add_md5(config, recipe, path)
+                if recipe not in imported:
+                    imported.append(recipe)
+
+    logger.info('Imported recipes into workspace %s: %s' % (config.workspace_path, ' '.join(imported)))
+    return 0
+
+def register_commands(subparsers, context):
+    """Register devtool import subcommands"""
+    parser = subparsers.add_parser('import',
+                                   help='Import tar archive into workspace',
+                                   description='Import previously created tar archive into the workspace',
+                                   group='advanced')
+    parser.add_argument('file', metavar='FILE', help='Name of the tar archive to import')
+    parser.add_argument('--overwrite', '-o', action="store_true", help='Overwrite previous export tar archive')
+    parser.set_defaults(func=devimport)
-- 
2.12.0

Re: [PATCH v2 3/3] import: new plugin to import the devtool workspace

[Paul Eggleton]

Hi Leo,

A few comments below.


On Friday, 2 June 2017 7:13:01 PM CEST leonardo.sandoval.gonzalez@linux.intel.com wrote:
> From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
> 
> Takes a tar archive created by 'devtool export' and export it (untar) to
> workspace. By default, the whole tar archive is imported, thus there is no
> way to limit what is imported.
> 
> https://bugzilla.yoctoproject.org/show_bug.cgi?id=10510
> 
> [YOCTO #10510]
> 
> Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
> ---
>  scripts/lib/devtool/import.py | 130 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 130 insertions(+)
>  create mode 100644 scripts/lib/devtool/import.py
> 
> diff --git a/scripts/lib/devtool/import.py b/scripts/lib/devtool/import.py
> new file mode 100644
> index 0000000000..5b85571669
> --- /dev/null
> +++ b/scripts/lib/devtool/import.py
> @@ -0,0 +1,130 @@
> +# Development tool - import command plugin
> +#
> +# Copyright (C) 2014-2017 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 import plugin"""
> +
> +import os
> +import tarfile
> +import logging
> +import re
> +from devtool import standard, setup_tinfoil
> +from devtool import export
> +
> +import oeqa.utils.ftools as ftools

So, this is importing something from oeqa to use outside of the QA scripts.
Aside from that structural oddity, I have to be honest and say I'd rather we
did it the replacement in specific code here rather than creating and using
a generic function (especially seeing as we don't already have one) - then
we avoid the need to open the file and process all lines multiple times.


> +import json
> +
> +logger = logging.getLogger('devtool')
> +
> +def devimport(args, config, basepath, workspace):
> +    """Entry point for the devtool 'import' subcommand"""
> +
> +    def get_pn(name):
> +        dirpath, recipe = os.path.split(name)
> +        pn = ""
> +        for sep in "_ .".split():
> +            if sep in recipe:
> +                pn = recipe.split(sep)[0]
> +                break
> +        return pn

This function is a little worrying. Recipe names can legally have dots
in the PN part of the name. See below for a recommended alternative.

> +    
> +    # match exported workspace folders
> +    prog = re.compile('recipes|appends|sources')
> +
> +    if not os.path.exists(args.file):
> +        logger.error('Tar archive %s does not exist. Export your workspace using "devtool export"')
> +        return 1
> +
> +    # get current recipes
> +    current_recipes = []
> +    try:
> +        tinfoil = setup_tinfoil(config_only=False, basepath=basepath)

The setup_tinfoil() here should be outside of the try...finally or if it fails
to start it will still try to shut it down.


> +        current_recipes = [recipe[1] for recipe in tinfoil.cooker.recipecaches[''].pkg_fn.items()]
> +    finally:
> +        tinfoil.shutdown()
> +
> +    imported = []
> +    with tarfile.open(args.file) as tar:
> +
> +        # get exported metadata so values containing paths can be automatically replaced it
> +        export_workspace_path = export_workspace = None
> +        try:
> +            metadata = tar.getmember(export.metadata)
> +            tar.extract(metadata)
> +            with open(metadata.name) as fdm:
> +                export_workspace_path, export_workspace = json.load(fdm)
> +            os.unlink(metadata.name)
> +        except KeyError as ke:
> +            logger.warn('The export metadata file created by "devtool export" was not found')
> +            logger.warn('Manual editing is needed to correct paths on imported recipes/appends')

We should just fail if the metadata file isn't there - we don't need to
support users importing any random tarball; if the metadata file
isn't there we can safely assume that it wasn't exported with
devtool export.

> +
> +        members = [member for member in tar.getmembers() if member.name != export.metadata]
> +        for member in members:
> +            # make sure imported bbappend has its corresponding recipe (bb)
> +            if member.name.startswith('appends'):
> +                bbappend = get_pn(member.name)
> +                if bbappend:
> +                    if bbappend not in current_recipes:
> +                        # check that the recipe is not in the tar archive being imported
> +                        if bbappend not in export_workspace:
> +                            logger.warn('No recipe to append %s, skipping' % bbappend)
> +                            continue
> +                else:
> +                    logger.warn('bbappend name %s could not be detected' % member.name)
> +                    continue

This isn't the right way to check this - PN isn't guaranteed to be the same
as the name part of the filename, for one. It's much safer to iterate over
tinfoil.cooker.recipecaches[''].pkg_fn (keys, i.e. filenames, not PN values)
and see if the bbappend matches the item, breaking out on first match.
Remember that a bbappend might be the exact same name as the bb file or
it might use a % wildcard. (You could cheat and replace this % with a * and
then use fnmatch.fnmatch() to make this a bit easier). 

Given the moderate expense of computing this we should probably do it
once as part of the initial check and store it in a dict so we can use it later
(I see get_pn() is called again later on).

> +            # extract file from tar
> +            path = os.path.join(config.workspace_path, member.name)
> +            if os.path.exists(path):
> +                if args.overwrite:
> +                    try:
> +                        tar.extract(member, path=config.workspace_path)
> +                    except PermissionError as pe:
> +                        logger.warn(pe)

If a recipe is already in someone's workspace, is this going to leave the
user with a mix of files extracted from the tarball and whatever happens to
already be in the source tree? If so, that could be problematic. I think I
would first check if the recipe's source tree exists at all and if so, either
skip it with a warning or overwrite it entirely dependent on whether -o
has been specified.


> +                else:
> +                    logger.warn('File already present. Use --overwrite/-o to overwrite it: %s' % member.name)
> +            else:
> +                tar.extract(member, path=config.workspace_path)
> +
> +            if member.name.startswith('appends'):
> +                recipe = get_pn(member.name)
> +
> +                # Update EXTERNARLSRC

Typo - EXTERNALSRC.

> +                if export_workspace_path:
> +                    # appends created by 'devtool modify' just need to update the workspace
> +                    ftools.replace_from_file(path, export_workspace_path, config.workspace_path)
> +
> +                    # appends created by 'devtool add' need replacement of exported source tree
> +                    exported_srctree = export_workspace[recipe]['srctree']
> +                    if exported_srctree:
> +                        ftools.replace_from_file(path, exported_srctree, os.path.join(config.workspace_path, 'sources', recipe))
> +
> +                # update .devtool_md5 file
> +                standard._add_md5(config, recipe, path)
> +                if recipe not in imported:
> +                    imported.append(recipe)
> +
> +    logger.info('Imported recipes into workspace %s: %s' % (config.workspace_path, ' '.join(imported)))
> +    return 0
> +
> +def register_commands(subparsers, context):
> +    """Register devtool import subcommands"""
> +    parser = subparsers.add_parser('import',
> +                                   help='Import tar archive into workspace',
> +                                   description='Import previously created tar archive into the workspace',

We should be specific - 'Import exported tar archive into workspace' and
'Import tar archive previously created by "devtool export" into workspace'.

> +                                   group='advanced')
> +    parser.add_argument('file', metavar='FILE', help='Name of the tar archive to import')
> +    parser.add_argument('--overwrite', '-o', action="store_true", help='Overwrite previous export tar archive')

We're not writing out a tar archive so this should be "Overwrite files when
extracting" or similar.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

Re: [PATCH v2 3/3] import: new plugin to import the devtool workspace

[Leonardo Sandoval]

On Mon, 2017-06-12 at 16:19 +0200, Paul Eggleton wrote:
> Hi Leo,
> 
> A few comments below.
> 

Thanks Paul. Too late to provided a v3 before M1, so I will work on the
changes during M2.

Leo

> 
> On Friday, 2 June 2017 7:13:01 PM CEST leonardo.sandoval.gonzalez@linux.intel.com wrote:
> > From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
> > 
> > Takes a tar archive created by 'devtool export' and export it (untar) to
> > workspace. By default, the whole tar archive is imported, thus there is no
> > way to limit what is imported.
> > 
> > https://bugzilla.yoctoproject.org/show_bug.cgi?id=10510
> > 
> > [YOCTO #10510]
> > 
> > Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
> > ---
> >  scripts/lib/devtool/import.py | 130 ++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 130 insertions(+)
> >  create mode 100644 scripts/lib/devtool/import.py
> > 
> > diff --git a/scripts/lib/devtool/import.py b/scripts/lib/devtool/import.py
> > new file mode 100644
> > index 0000000000..5b85571669
> > --- /dev/null
> > +++ b/scripts/lib/devtool/import.py
> > @@ -0,0 +1,130 @@
> > +# Development tool - import command plugin
> > +#
> > +# Copyright (C) 2014-2017 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 import plugin"""
> > +
> > +import os
> > +import tarfile
> > +import logging
> > +import re
> > +from devtool import standard, setup_tinfoil
> > +from devtool import export
> > +
> > +import oeqa.utils.ftools as ftools
> 
> So, this is importing something from oeqa to use outside of the QA scripts.
> Aside from that structural oddity, I have to be honest and say I'd rather we
> did it the replacement in specific code here rather than creating and using
> a generic function (especially seeing as we don't already have one) - then
> we avoid the need to open the file and process all lines multiple times.
> 
> 
> > +import json
> > +
> > +logger = logging.getLogger('devtool')
> > +
> > +def devimport(args, config, basepath, workspace):
> > +    """Entry point for the devtool 'import' subcommand"""
> > +
> > +    def get_pn(name):
> > +        dirpath, recipe = os.path.split(name)
> > +        pn = ""
> > +        for sep in "_ .".split():
> > +            if sep in recipe:
> > +                pn = recipe.split(sep)[0]
> > +                break
> > +        return pn
> 
> This function is a little worrying. Recipe names can legally have dots
> in the PN part of the name. See below for a recommended alternative.
> 
> > +    
> > +    # match exported workspace folders
> > +    prog = re.compile('recipes|appends|sources')
> > +
> > +    if not os.path.exists(args.file):
> > +        logger.error('Tar archive %s does not exist. Export your workspace using "devtool export"')
> > +        return 1
> > +
> > +    # get current recipes
> > +    current_recipes = []
> > +    try:
> > +        tinfoil = setup_tinfoil(config_only=False, basepath=basepath)
> 
> The setup_tinfoil() here should be outside of the try...finally or if it fails
> to start it will still try to shut it down.
> 
> 
> > +        current_recipes = [recipe[1] for recipe in tinfoil.cooker.recipecaches[''].pkg_fn.items()]
> > +    finally:
> > +        tinfoil.shutdown()
> > +
> > +    imported = []
> > +    with tarfile.open(args.file) as tar:
> > +
> > +        # get exported metadata so values containing paths can be automatically replaced it
> > +        export_workspace_path = export_workspace = None
> > +        try:
> > +            metadata = tar.getmember(export.metadata)
> > +            tar.extract(metadata)
> > +            with open(metadata.name) as fdm:
> > +                export_workspace_path, export_workspace = json.load(fdm)
> > +            os.unlink(metadata.name)
> > +        except KeyError as ke:
> > +            logger.warn('The export metadata file created by "devtool export" was not found')
> > +            logger.warn('Manual editing is needed to correct paths on imported recipes/appends')
> 
> We should just fail if the metadata file isn't there - we don't need to
> support users importing any random tarball; if the metadata file
> isn't there we can safely assume that it wasn't exported with
> devtool export.
> 
> > +
> > +        members = [member for member in tar.getmembers() if member.name != export.metadata]
> > +        for member in members:
> > +            # make sure imported bbappend has its corresponding recipe (bb)
> > +            if member.name.startswith('appends'):
> > +                bbappend = get_pn(member.name)
> > +                if bbappend:
> > +                    if bbappend not in current_recipes:
> > +                        # check that the recipe is not in the tar archive being imported
> > +                        if bbappend not in export_workspace:
> > +                            logger.warn('No recipe to append %s, skipping' % bbappend)
> > +                            continue
> > +                else:
> > +                    logger.warn('bbappend name %s could not be detected' % member.name)
> > +                    continue
> 
> This isn't the right way to check this - PN isn't guaranteed to be the same
> as the name part of the filename, for one. It's much safer to iterate over
> tinfoil.cooker.recipecaches[''].pkg_fn (keys, i.e. filenames, not PN values)
> and see if the bbappend matches the item, breaking out on first match.
> Remember that a bbappend might be the exact same name as the bb file or
> it might use a % wildcard. (You could cheat and replace this % with a * and
> then use fnmatch.fnmatch() to make this a bit easier). 
> 
> Given the moderate expense of computing this we should probably do it
> once as part of the initial check and store it in a dict so we can use it later
> (I see get_pn() is called again later on).
> 
> > +            # extract file from tar
> > +            path = os.path.join(config.workspace_path, member.name)
> > +            if os.path.exists(path):
> > +                if args.overwrite:
> > +                    try:
> > +                        tar.extract(member, path=config.workspace_path)
> > +                    except PermissionError as pe:
> > +                        logger.warn(pe)
> 
> If a recipe is already in someone's workspace, is this going to leave the
> user with a mix of files extracted from the tarball and whatever happens to
> already be in the source tree? If so, that could be problematic. I think I
> would first check if the recipe's source tree exists at all and if so, either
> skip it with a warning or overwrite it entirely dependent on whether -o
> has been specified.
> 
> 
> > +                else:
> > +                    logger.warn('File already present. Use --overwrite/-o to overwrite it: %s' % member.name)
> > +            else:
> > +                tar.extract(member, path=config.workspace_path)
> > +
> > +            if member.name.startswith('appends'):
> > +                recipe = get_pn(member.name)
> > +
> > +                # Update EXTERNARLSRC
> 
> Typo - EXTERNALSRC.
> 
> > +                if export_workspace_path:
> > +                    # appends created by 'devtool modify' just need to update the workspace
> > +                    ftools.replace_from_file(path, export_workspace_path, config.workspace_path)
> > +
> > +                    # appends created by 'devtool add' need replacement of exported source tree
> > +                    exported_srctree = export_workspace[recipe]['srctree']
> > +                    if exported_srctree:
> > +                        ftools.replace_from_file(path, exported_srctree, os.path.join(config.workspace_path, 'sources', recipe))
> > +
> > +                # update .devtool_md5 file
> > +                standard._add_md5(config, recipe, path)
> > +                if recipe not in imported:
> > +                    imported.append(recipe)
> > +
> > +    logger.info('Imported recipes into workspace %s: %s' % (config.workspace_path, ' '.join(imported)))
> > +    return 0
> > +
> > +def register_commands(subparsers, context):
> > +    """Register devtool import subcommands"""
> > +    parser = subparsers.add_parser('import',
> > +                                   help='Import tar archive into workspace',
> > +                                   description='Import previously created tar archive into the workspace',
> 
> We should be specific - 'Import exported tar archive into workspace' and
> 'Import tar archive previously created by "devtool export" into workspace'.
> 
> > +                                   group='advanced')
> > +    parser.add_argument('file', metavar='FILE', help='Name of the tar archive to import')
> > +    parser.add_argument('--overwrite', '-o', action="store_true", help='Overwrite previous export tar archive')
> 
> We're not writing out a tar archive so this should be "Overwrite files when
> extracting" or similar.
> 
> Cheers,
> Paul
> 

[PATCH v3 0/3] Two new devtool plugins: export and import

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

These two plugins can be used to share the devtool workspace between users.

The following changes since commit ef506f58da3a95fba2696df749b2b81f9c118847:

  cve-check-tool: backport a patch to make CVE checking work (2017-05-18 14:01:48 +0100)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib lsandov1/devtool-import-export
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=lsandov1/devtool-import-export

Leonardo Sandoval (3):
  export: new plugin to export the devtool workspace
  devtool: function to replace strings inside a text file
  import: new plugin to import the devtool workspace

 scripts/lib/devtool/__init__.py |  37 +++++++++++
 scripts/lib/devtool/export.py   | 119 +++++++++++++++++++++++++++++++++
 scripts/lib/devtool/import.py   | 142 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 298 insertions(+)
 create mode 100644 scripts/lib/devtool/export.py
 create mode 100644 scripts/lib/devtool/import.py

-- 
2.12.0

[PATCH v3 1/3] export: new plugin to export the devtool workspace

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

By default, exports the whole workspace (all recipes) including the source code.
User can also limit what is exported with --included/--excluded flags. As
a result of this operation, a tar archive containing only workspace metadata
and its corresponding source code is created, which can be properly imported
with 'devtool import'.

https://bugzilla.yoctoproject.org/show_bug.cgi?id=10510

[YOCTO #10510]

Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
---
 scripts/lib/devtool/export.py | 119 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)
 create mode 100644 scripts/lib/devtool/export.py

diff --git a/scripts/lib/devtool/export.py b/scripts/lib/devtool/export.py
new file mode 100644
index 0000000000..13ee258e7a
--- /dev/null
+++ b/scripts/lib/devtool/export.py
@@ -0,0 +1,119 @@
+# Development tool - export command plugin
+#
+# Copyright (C) 2014-2017 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 export plugin"""
+
+import os
+import argparse
+import tarfile
+import logging
+import datetime
+import json
+
+logger = logging.getLogger('devtool')
+
+# output files
+default_arcname_prefix = "workspace-export"
+metadata = '.export_metadata'
+
+def export(args, config, basepath, workspace):
+    """Entry point for the devtool 'export' subcommand"""
+
+    def add_metadata(tar):
+        """Archive the workspace object"""
+        # finally store the workspace metadata
+        with open(metadata, 'w') as fd:
+            fd.write(json.dumps((config.workspace_path, workspace)))
+        tar.add(metadata)
+        os.unlink(metadata)
+
+    def add_recipe(tar, recipe, data):
+        """Archive recipe with proper arcname"""
+        # Create a map of name/arcnames
+        arcnames = []
+        for key, name in data.items():
+            if name:
+                if key == 'srctree':
+                    # all sources, no matter where are located, goes into the sources directory
+                    arcname = 'sources/%s' % recipe
+                else:
+                    arcname = name.replace(config.workspace_path, '')
+                arcnames.append((name, arcname))
+
+        for name, arcname in arcnames:
+            tar.add(name, arcname=arcname)
+
+
+    # Make sure workspace is non-empty and possible listed include/excluded recipes are in workspace
+    if not workspace:
+        logger.info('Workspace contains no recipes, nothing to export')
+        return 0
+    else:
+        for param, recipes in {'include':args.include,'exclude':args.exclude}.items():
+            for recipe in recipes:
+                if recipe not in workspace:
+                    logger.error('Recipe (%s) on %s argument not in the current workspace' % (recipe, param))
+                    return 1
+
+    name = args.file
+
+    default_name = "%s-%s.tar.gz" % (default_arcname_prefix, datetime.datetime.now().strftime('%Y%m%d%H%M%S'))
+    if not name:
+        name = default_name
+    else:
+        # if name is a directory, append the default name
+        if os.path.isdir(name):
+            name = os.path.join(name, default_name)
+
+    if os.path.exists(name) and not args.overwrite:
+        logger.error('Tar archive %s exists. Use --overwrite/-o to overwrite it')
+        return 1
+
+    # if all workspace is excluded, quit
+    if not len(set(workspace.keys()).difference(set(args.exclude))):
+        logger.warn('All recipes in workspace excluded, nothing to export')
+        return 0
+
+    exported = []
+    with tarfile.open(name, 'w:gz') as tar:
+        if args.include:
+            for recipe in args.include:
+                add_recipe(tar, recipe, workspace[recipe])
+                exported.append(recipe)
+        else:
+            for recipe, data in workspace.items():
+                if recipe not in args.exclude:
+                    add_recipe(tar, recipe, data)
+                    exported.append(recipe)
+
+        add_metadata(tar)
+
+    logger.info('Tar archive created at %s with the following recipes: %s' % (name, ', '.join(exported)))
+    return 0
+
+def register_commands(subparsers, context):
+    """Register devtool export subcommands"""
+    parser = subparsers.add_parser('export',
+                                   help='Export workspace into a tar archive',
+                                   description='Export one or more recipes from current workspace into a tar archive',
+                                   group='advanced')
+
+    parser.add_argument('--file', '-f', help='Output archive file name')
+    parser.add_argument('--overwrite', '-o', action="store_true", help='Overwrite previous export tar archive')
+    group = parser.add_mutually_exclusive_group()
+    group.add_argument('--include', '-i', nargs='+', default=[], help='Include recipes into the tar archive')
+    group.add_argument('--exclude', '-e', nargs='+', default=[], help='Exclude recipes into the tar archive')
+    parser.set_defaults(func=export)
-- 
2.12.0

[PATCH v3 2/3] devtool: function to replace strings inside a text file

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
---
 scripts/lib/devtool/__init__.py | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/scripts/lib/devtool/__init__.py b/scripts/lib/devtool/__init__.py
index d646b0cf63..95307e63fa 100644
--- a/scripts/lib/devtool/__init__.py
+++ b/scripts/lib/devtool/__init__.py
@@ -292,3 +292,40 @@ def ensure_npm(config, basepath, fixed_setup=False, check_exists=True):
                 raise DevtoolError(msg)
             else:
                 raise
+
+def replace_from_file(path, old, new):
+    """Replace strings on a file"""
+
+    def read_file(path):
+        data = None
+        with open(path) as f:
+            data = f.read()
+        return data
+
+    def write_file(path, data):
+        if data is None:
+            return
+        wdata = data.rstrip() + "\n"
+        with open(path, "w") as f:
+            f.write(wdata)
+
+    # In case old is None, return immediately
+    if old is None:
+        return
+    try:
+        rdata = read_file(path)
+    except IOError as e:
+        # if file does not exit, just quit, otherwise raise an exception
+        if e.errno == errno.ENOENT:
+            return
+        else:
+            raise
+
+    old_contents = rdata.splitlines()
+    new_contents = []
+    for old_content in old_contents:
+        try:
+            new_contents.append(old_content.replace(old, new))
+        except ValueError:
+            pass
+    write_file(path, "\n".join(new_contents))
-- 
2.12.0

[PATCH v3 3/3] import: new plugin to import the devtool workspace

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

Takes a tar archive created by 'devtool export' and export it (untar) to
workspace. By default, the whole tar archive is imported, thus there is no
way to limit what is imported.

https://bugzilla.yoctoproject.org/show_bug.cgi?id=10510

[YOCTO #10510]

Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
---
 scripts/lib/devtool/import.py | 142 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 142 insertions(+)
 create mode 100644 scripts/lib/devtool/import.py

diff --git a/scripts/lib/devtool/import.py b/scripts/lib/devtool/import.py
new file mode 100644
index 0000000000..26084ec3b8
--- /dev/null
+++ b/scripts/lib/devtool/import.py
@@ -0,0 +1,142 @@
+# Development tool - import command plugin
+#
+# Copyright (C) 2014-2017 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 import plugin"""
+
+import os
+import tarfile
+import logging
+import re
+import json
+import fnmatch
+
+from devtool import standard, setup_tinfoil, replace_from_file
+from devtool import export
+
+logger = logging.getLogger('devtool')
+
+def devimport(args, config, basepath, workspace):
+    """Entry point for the devtool 'import' subcommand"""
+
+    if not os.path.exists(args.file):
+        logger.error('Tar archive %s does not exist. Export your workspace using "devtool export"')
+        return 1
+
+
+    tinfoil = setup_tinfoil(config_only=False, basepath=basepath)
+
+    imported = []
+    tar = tarfile.open(args.file)
+    members = tar.getmembers()
+
+    # get exported metadata so values containing paths can be automatically replaced it
+    export_workspace_path = export_workspace = None
+    try:
+        metadata = tar.getmember(export.metadata)
+        tar.extract(metadata)
+        with open(metadata.name) as fdm:
+            export_workspace_path, export_workspace = json.load(fdm)
+        os.unlink(metadata.name)
+    except KeyError as ke:
+        logger.error('The export metadata file created by "devtool export" was not found')
+        return 1
+
+    try:
+        recipe_files = [os.path.basename(recipe[0]) for recipe in tinfoil.cooker.recipecaches[''].pkg_fn.items()]
+    finally:
+        if tinfoil:
+            tinfoil.shutdown()
+
+    for member in members:
+        # do not export the metadata
+        if member.name == export.metadata:
+            continue
+
+        is_bbappend = False
+
+        # check bbappend has its corresponding recipe, if not warn continue with next tar member
+        if member.name.startswith('appends'):
+            is_bbappend = True
+            append_root,_ = os.path.splitext(os.path.basename(member.name))
+
+            # check on new recipes introduced by the export
+            for exported_recipe in export_workspace.keys():
+                if append_root.startswith(exported_recipe):
+                    break
+            else:
+                # check on current recipes
+                for recipe_file in recipe_files:
+                    if fnmatch.fnmatch(recipe_file, append_root.replace('%', '') + '*'):
+                        break
+                else:
+                    logger.warn('No recipe to append %s, skipping' % append_root)
+                    continue
+
+        # extract
+        path = os.path.join(config.workspace_path, member.name)
+        if os.path.exists(path):
+            # by default, no file overwrite is done unless -o is given by the user
+            if args.overwrite:
+                try:
+                    tar.extract(member, path=config.workspace_path)
+                except PermissionError as pe:
+                    logger.warn(pe)
+            else:
+                logger.warn('File already present. Use --overwrite/-o to overwrite it: %s' % member.name)
+                continue
+        else:
+            tar.extract(member, path=config.workspace_path)
+
+        # bbappend require extra handling
+        if is_bbappend:
+
+            # we need to get the exported PN from the exported metadata,
+            for exported_recipe in export_workspace.keys():
+                if exported_recipe in member.name:
+                    pn = exported_recipe
+                    break
+            else:
+                logger.error('Exported metadata does not correspond to data')
+
+            # Update EXTERNALSRC
+            if export_workspace_path:
+                # appends created by 'devtool modify' just need to update the workspace
+                replace_from_file(path, export_workspace_path, config.workspace_path)
+
+                # appends created by 'devtool add' need replacement of exported source tree
+                exported_srctree = export_workspace[pn]['srctree']
+                if exported_srctree:
+                    replace_from_file(path, exported_srctree, os.path.join(config.workspace_path, 'sources', pn))
+
+            # update .devtool_md5 file
+            standard._add_md5(config, pn, path)
+            if pn not in imported:
+                imported.append(pn)
+
+    tar.close()
+
+    logger.info('Imported recipes into workspace %s: %s' % (config.workspace_path, ' '.join(imported)))
+    return 0
+
+def register_commands(subparsers, context):
+    """Register devtool import subcommands"""
+    parser = subparsers.add_parser('import',
+                                   help='Import exported tar archive into workspace',
+                                   description='Import tar archive previously created by "devtool export" into workspace',
+                                   group='advanced')
+    parser.add_argument('file', metavar='FILE', help='Name of the tar archive to import')
+    parser.add_argument('--overwrite', '-o', action="store_true", help='Overwrite files when extracting')
+    parser.set_defaults(func=devimport)
-- 
2.12.0

Re: [PATCH v3 3/3] import: new plugin to import the devtool workspace

[Paul Eggleton]

Hi Leo,

On Tuesday, 20 June 2017 8:30:05 PM CEST leonardo.sandoval.gonzalez@linux.intel.com wrote:
> From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
> 
> Takes a tar archive created by 'devtool export' and export it (untar) to
> workspace. By default, the whole tar archive is imported, thus there is no
> way to limit what is imported.
> 
> https://bugzilla.yoctoproject.org/show_bug.cgi?id=10510
> 
> [YOCTO #10510]
> 
> Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
> ---
>  scripts/lib/devtool/import.py | 142 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 142 insertions(+)
>  create mode 100644 scripts/lib/devtool/import.py
> 
> diff --git a/scripts/lib/devtool/import.py b/scripts/lib/devtool/import.py
> new file mode 100644
> index 0000000000..26084ec3b8
> --- /dev/null
> +++ b/scripts/lib/devtool/import.py
> @@ -0,0 +1,142 @@
> +# Development tool - import command plugin
> +#
> +# Copyright (C) 2014-2017 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 import plugin"""
> +
> +import os
> +import tarfile
> +import logging
> +import re
> +import json
> +import fnmatch
> +
> +from devtool import standard, setup_tinfoil, replace_from_file
> +from devtool import export
> +
> +logger = logging.getLogger('devtool')
> +
> +def devimport(args, config, basepath, workspace):
> +    """Entry point for the devtool 'import' subcommand"""
> +
> +    if not os.path.exists(args.file):
> +        logger.error('Tar archive %s does not exist. Export your workspace using "devtool export"')
> +        return 1
> +
> +
> +    tinfoil = setup_tinfoil(config_only=False, basepath=basepath)

So, when I said move this outside of the try...finally, try still needs to
immediately follow it or there is a chance that an error will occur some
time between tinfoil being created and the try...finally block that calls
shutdown(), with the result that tinfoil will not get shut down properly.


> +
> +    imported = []
> +    tar = tarfile.open(args.file)
> +    members = tar.getmembers()
> +
> +    # get exported metadata so values containing paths can be automatically replaced it
> +    export_workspace_path = export_workspace = None
> +    try:
> +        metadata = tar.getmember(export.metadata)
> +        tar.extract(metadata)
> +        with open(metadata.name) as fdm:
> +            export_workspace_path, export_workspace = json.load(fdm)
> +        os.unlink(metadata.name)
> +    except KeyError as ke:
> +        logger.error('The export metadata file created by "devtool export" was not found')
> +        return 1

I neglected to mention in my previous review that as a matter of best
practice the try...except should span only the calls where you expect 
this particular error to occur (I would assume just tar.getmember()?) 
in case KeyError happens for different reasons.

We should also mention in the message that "devtool import can only be used
to import tar archives created by devtool export", since that is the most
likely reason for receiving this error.


> +    try:
> +        recipe_files = [os.path.basename(recipe[0]) for recipe in tinfoil.cooker.recipecaches[''].pkg_fn.items()]
> +    finally:
> +        if tinfoil:
> +            tinfoil.shutdown()

Why would tinfoil not be assigned here?


> +
> +    for member in members:
> +        # do not export the metadata
> +        if member.name == export.metadata:
> +            continue
> +
> +        is_bbappend = False
> +
> +        # check bbappend has its corresponding recipe, if not warn continue with next tar member
> +        if member.name.startswith('appends'):
> +            is_bbappend = True
> +            append_root,_ = os.path.splitext(os.path.basename(member.name))
> +
> +            # check on new recipes introduced by the export
> +            for exported_recipe in export_workspace.keys():
> +                if append_root.startswith(exported_recipe):
> +                    break
> +            else:
> +                # check on current recipes
> +                for recipe_file in recipe_files:
> +                    if fnmatch.fnmatch(recipe_file, append_root.replace('%', '') + '*'):
> +                        break
> +                else:
> +                    logger.warn('No recipe to append %s, skipping' % append_root)
> +                    continue

Could you also please ensure that the other files associated with this
bbappend (recipe + associated files and sources) don't get imported if the
bbappend doesn't apply? I think we should have the information needed for
that in the workspace object retrieved from the exported metadata.

Cheers,
Paul

-- 

Paul Eggleton
Intel Open Source Technology Centre

[PATCH v4 0/4] Two new devtool plugins: export and import

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

These two plugins can be used to share the devtool workspace between users.

The following changes since commit c3058ec4a4f2f4c57116816a5bede1e61a5a4cc4:

  meta/conf/layer.conf: bump layer version for LSB changes (2017-06-28 15:52:19 +0100)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib lsandov1/devtool-import-export
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=lsandov1/devtool-import-export

Leonardo Sandoval (4):
  export: new plugin to export the devtool workspace
  devtool: function to replace strings inside a text file
  standard: append md5 tag only if not already present
  import: new plugin to import the devtool workspace

 scripts/lib/devtool/__init__.py |  37 ++++++++++
 scripts/lib/devtool/export.py   | 119 +++++++++++++++++++++++++++++++
 scripts/lib/devtool/import.py   | 150 ++++++++++++++++++++++++++++++++++++++++
 scripts/lib/devtool/standard.py |   7 +-
 4 files changed, 311 insertions(+), 2 deletions(-)
 create mode 100644 scripts/lib/devtool/export.py
 create mode 100644 scripts/lib/devtool/import.py

-- 
2.12.0

[PATCH v4 1/4] export: new plugin to export the devtool workspace

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

By default, exports the whole workspace (all recipes) including the source code.
User can also limit what is exported with --included/--excluded flags. As
a result of this operation, a tar archive containing only workspace metadata
and its corresponding source code is created, which can be properly imported
with 'devtool import'.

https://bugzilla.yoctoproject.org/show_bug.cgi?id=10510

[YOCTO #10510]

Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
---
 scripts/lib/devtool/export.py | 119 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)
 create mode 100644 scripts/lib/devtool/export.py

diff --git a/scripts/lib/devtool/export.py b/scripts/lib/devtool/export.py
new file mode 100644
index 0000000000..13ee258e7a
--- /dev/null
+++ b/scripts/lib/devtool/export.py
@@ -0,0 +1,119 @@
+# Development tool - export command plugin
+#
+# Copyright (C) 2014-2017 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 export plugin"""
+
+import os
+import argparse
+import tarfile
+import logging
+import datetime
+import json
+
+logger = logging.getLogger('devtool')
+
+# output files
+default_arcname_prefix = "workspace-export"
+metadata = '.export_metadata'
+
+def export(args, config, basepath, workspace):
+    """Entry point for the devtool 'export' subcommand"""
+
+    def add_metadata(tar):
+        """Archive the workspace object"""
+        # finally store the workspace metadata
+        with open(metadata, 'w') as fd:
+            fd.write(json.dumps((config.workspace_path, workspace)))
+        tar.add(metadata)
+        os.unlink(metadata)
+
+    def add_recipe(tar, recipe, data):
+        """Archive recipe with proper arcname"""
+        # Create a map of name/arcnames
+        arcnames = []
+        for key, name in data.items():
+            if name:
+                if key == 'srctree':
+                    # all sources, no matter where are located, goes into the sources directory
+                    arcname = 'sources/%s' % recipe
+                else:
+                    arcname = name.replace(config.workspace_path, '')
+                arcnames.append((name, arcname))
+
+        for name, arcname in arcnames:
+            tar.add(name, arcname=arcname)
+
+
+    # Make sure workspace is non-empty and possible listed include/excluded recipes are in workspace
+    if not workspace:
+        logger.info('Workspace contains no recipes, nothing to export')
+        return 0
+    else:
+        for param, recipes in {'include':args.include,'exclude':args.exclude}.items():
+            for recipe in recipes:
+                if recipe not in workspace:
+                    logger.error('Recipe (%s) on %s argument not in the current workspace' % (recipe, param))
+                    return 1
+
+    name = args.file
+
+    default_name = "%s-%s.tar.gz" % (default_arcname_prefix, datetime.datetime.now().strftime('%Y%m%d%H%M%S'))
+    if not name:
+        name = default_name
+    else:
+        # if name is a directory, append the default name
+        if os.path.isdir(name):
+            name = os.path.join(name, default_name)
+
+    if os.path.exists(name) and not args.overwrite:
+        logger.error('Tar archive %s exists. Use --overwrite/-o to overwrite it')
+        return 1
+
+    # if all workspace is excluded, quit
+    if not len(set(workspace.keys()).difference(set(args.exclude))):
+        logger.warn('All recipes in workspace excluded, nothing to export')
+        return 0
+
+    exported = []
+    with tarfile.open(name, 'w:gz') as tar:
+        if args.include:
+            for recipe in args.include:
+                add_recipe(tar, recipe, workspace[recipe])
+                exported.append(recipe)
+        else:
+            for recipe, data in workspace.items():
+                if recipe not in args.exclude:
+                    add_recipe(tar, recipe, data)
+                    exported.append(recipe)
+
+        add_metadata(tar)
+
+    logger.info('Tar archive created at %s with the following recipes: %s' % (name, ', '.join(exported)))
+    return 0
+
+def register_commands(subparsers, context):
+    """Register devtool export subcommands"""
+    parser = subparsers.add_parser('export',
+                                   help='Export workspace into a tar archive',
+                                   description='Export one or more recipes from current workspace into a tar archive',
+                                   group='advanced')
+
+    parser.add_argument('--file', '-f', help='Output archive file name')
+    parser.add_argument('--overwrite', '-o', action="store_true", help='Overwrite previous export tar archive')
+    group = parser.add_mutually_exclusive_group()
+    group.add_argument('--include', '-i', nargs='+', default=[], help='Include recipes into the tar archive')
+    group.add_argument('--exclude', '-e', nargs='+', default=[], help='Exclude recipes into the tar archive')
+    parser.set_defaults(func=export)
-- 
2.12.0

[PATCH v4 2/4] devtool: function to replace strings inside a text file

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
---
 scripts/lib/devtool/__init__.py | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/scripts/lib/devtool/__init__.py b/scripts/lib/devtool/__init__.py
index 29c4c05071..7f4f817a8e 100644
--- a/scripts/lib/devtool/__init__.py
+++ b/scripts/lib/devtool/__init__.py
@@ -292,3 +292,40 @@ def ensure_npm(config, basepath, fixed_setup=False, check_exists=True):
                 raise DevtoolError(msg)
             else:
                 raise
+
+def replace_from_file(path, old, new):
+    """Replace strings on a file"""
+
+    def read_file(path):
+        data = None
+        with open(path) as f:
+            data = f.read()
+        return data
+
+    def write_file(path, data):
+        if data is None:
+            return
+        wdata = data.rstrip() + "\n"
+        with open(path, "w") as f:
+            f.write(wdata)
+
+    # In case old is None, return immediately
+    if old is None:
+        return
+    try:
+        rdata = read_file(path)
+    except IOError as e:
+        # if file does not exit, just quit, otherwise raise an exception
+        if e.errno == errno.ENOENT:
+            return
+        else:
+            raise
+
+    old_contents = rdata.splitlines()
+    new_contents = []
+    for old_content in old_contents:
+        try:
+            new_contents.append(old_content.replace(old, new))
+        except ValueError:
+            pass
+    write_file(path, "\n".join(new_contents))
-- 
2.12.0

[PATCH v4 3/4] standard: append md5 tag only if not already present

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

In case the proposed md5 tag to be appended is already present,
do not append it.

Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
---
 scripts/lib/devtool/standard.py | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/scripts/lib/devtool/standard.py b/scripts/lib/devtool/standard.py
index 7e342e7687..8b6ab9ff16 100644
--- a/scripts/lib/devtool/standard.py
+++ b/scripts/lib/devtool/standard.py
@@ -700,8 +700,11 @@ def _add_md5(config, recipename, filename):
 
     def addfile(fn):
         md5 = bb.utils.md5_file(fn)
-        with open(os.path.join(config.workspace_path, '.devtool_md5'), 'a') as f:
-            f.write('%s|%s|%s\n' % (recipename, os.path.relpath(fn, config.workspace_path), md5))
+        with open(os.path.join(config.workspace_path, '.devtool_md5'), 'a+') as f:
+            md5_str = '%s|%s|%s\n' % (recipename, os.path.relpath(fn, config.workspace_path), md5)
+            f.seek(0, os.SEEK_SET)
+            if not md5_str in f.read():
+                f.write(md5_str)
 
     if os.path.isdir(filename):
         for root, _, files in os.walk(filename):
-- 
2.12.0

[PATCH v4 4/4] import: new plugin to import the devtool workspace

[leonardo.sandoval.gonzalez]

From: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>

Takes a tar archive created by 'devtool export' and export it (untar) to
workspace. By default, the whole tar archive is imported, thus there is no
way to limit what is imported.

https://bugzilla.yoctoproject.org/show_bug.cgi?id=10510

[YOCTO #10510]

Signed-off-by: Leonardo Sandoval <leonardo.sandoval.gonzalez@linux.intel.com>
---
 scripts/lib/devtool/import.py | 150 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 150 insertions(+)
 create mode 100644 scripts/lib/devtool/import.py

diff --git a/scripts/lib/devtool/import.py b/scripts/lib/devtool/import.py
new file mode 100644
index 0000000000..21eb90820b
--- /dev/null
+++ b/scripts/lib/devtool/import.py
@@ -0,0 +1,150 @@
+# Development tool - import command plugin
+#
+# Copyright (C) 2014-2017 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 import plugin"""
+
+import os
+import tarfile
+import logging
+import collections
+import json
+import fnmatch
+
+from devtool import standard, setup_tinfoil, replace_from_file
+from devtool import export
+
+logger = logging.getLogger('devtool')
+
+def devimport(args, config, basepath, workspace):
+    """Entry point for the devtool 'import' subcommand"""
+
+    def get_pn(name):
+        """ Returns the filename of a workspace recipe/append"""
+        metadata = name.split('/')[-1]
+        fn, _ = os.path.splitext(metadata)
+        return fn
+
+    if not os.path.exists(args.file):
+        logger.error('Tar archive %s does not exist. Export your workspace using "devtool export"' % args.file)
+        return 1
+
+    tar = tarfile.open(args.file)
+
+    # Get exported metadata
+    export_workspace_path = export_workspace = None
+    try:
+        metadata = tar.getmember(export.metadata)
+    except KeyError as ke:
+        logger.error('The export metadata file created by "devtool export" was not found')
+        logger.error('The devtool import plugin can only be used to import tar archives created by devtool export')
+        return 1
+
+    tar.extract(metadata)
+    with open(metadata.name) as fdm:
+        export_workspace_path, export_workspace = json.load(fdm)
+    os.unlink(metadata.name)
+
+    members = tar.getmembers()
+
+    # Get appends and recipes from the exported archive, these
+    # will be needed to find out those appends without corresponding
+    # recipe pair
+    append_fns, recipe_fns = set(), set()
+    for member in members:
+        if member.name.startswith('appends'):
+            append_fns.add(get_pn(member.name))
+        elif member.name.startswith('recipes'):
+            recipe_fns.add(get_pn(member.name))
+
+    # Setup tinfoil, get required data and shutdown
+    tinfoil = setup_tinfoil(config_only=False, basepath=basepath)
+    try:
+        current_fns = [os.path.basename(recipe[0]) for recipe in tinfoil.cooker.recipecaches[''].pkg_fn.items()]
+    finally:
+        tinfoil.shutdown()
+
+    # Find those appends that do not have recipes in current metadata
+    non_importables = []
+    for fn in append_fns - recipe_fns:
+        # Check on current metadata (covering those layers indicated in bblayers.conf)
+        for current_fn in current_fns:
+            if fnmatch.fnmatch(current_fn, '*' + fn.replace('%', '') + '*'):
+                break
+        else:
+            non_importables.append(fn)
+            logger.warn('No recipe to append %s.bbapppend, skipping' % fn)
+
+    # Extract
+    imported = []
+    for member in members:
+        if member.name == export.metadata:
+            continue
+
+        for nonimp in non_importables:
+            pn = nonimp.split('_')[0]
+            # do not extract data from non-importable recipes or metadata
+            if member.name.startswith('appends/%s' % nonimp) or \
+               member.name.startswith('recipes/%s' % nonimp) or \
+               member.name.startswith('sources/%s' % pn):
+               break
+        else:
+            path = os.path.join(config.workspace_path, member.name)
+            if os.path.exists(path):
+                # by default, no file overwrite is done unless -o is given by the user
+                if args.overwrite:
+                    try:
+                        tar.extract(member, path=config.workspace_path)
+                    except PermissionError as pe:
+                        logger.warn(pe)
+                else:
+                    logger.warn('File already present. Use --overwrite/-o to overwrite it: %s' % member.name)
+                    continue
+            else:
+                tar.extract(member, path=config.workspace_path)
+
+            # Update EXTERNALSRC and the devtool md5 file
+            if member.name.startswith('appends'):
+                if export_workspace_path:
+                    # appends created by 'devtool modify' just need to update the workspace
+                    replace_from_file(path, export_workspace_path, config.workspace_path)
+
+                    # appends created by 'devtool add' need replacement of exported source tree
+                    pn = get_pn(member.name).split('_')[0]
+                    exported_srctree = export_workspace[pn]['srctree']
+                    if exported_srctree:
+                        replace_from_file(path, exported_srctree, os.path.join(config.workspace_path, 'sources', pn))
+
+                standard._add_md5(config, pn, path)
+                imported.append(pn)
+
+    tar.close()
+
+    if  imported:
+        logger.info('Imported recipes into workspace %s: %s' % (config.workspace_path, ', '.join(imported)))
+    else:
+        logger.warn('No recipes imported into the workspace')
+
+    return 0
+
+def register_commands(subparsers, context):
+    """Register devtool import subcommands"""
+    parser = subparsers.add_parser('import',
+                                   help='Import exported tar archive into workspace',
+                                   description='Import tar archive previously created by "devtool export" into workspace',
+                                   group='advanced')
+    parser.add_argument('file', metavar='FILE', help='Name of the tar archive to import')
+    parser.add_argument('--overwrite', '-o', action="store_true", help='Overwrite files when extracting')
+    parser.set_defaults(func=devimport)
-- 
2.12.0