[PATCH 0/4] wic: fix for #11633 and code cleanup

[PATCH 0/4] wic: fix for #11633 and code cleanup

[Ed Bartosh]

Hi,

This patchset contains a fix for long standing wic bug: in some cases wic
can leave updated fstab in a rootfs directory and produce duplicate fstab
entries.

A bit of code cleanup is also included into the patchset.

The following changes since commit 27917fa9e14c1f0b0c3ed20c504772cf7d07b9df:

  image.bbclass: use prependVarFlag for postfuncs (2017-06-16 15:11:22 +0300)

are available in the git repository at:

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

Ed Bartosh (4):
  runner.py: move runtool API to misc.py
  wic: flatten directory structure
  wic: code cleanup
  wic: fix restoring of original fstab

 scripts/lib/wic/engine.py                          |  2 +-
 scripts/lib/wic/filemap.py                         |  8 +---
 scripts/lib/wic/{utils => }/misc.py                | 43 +++++++++++++++---
 scripts/lib/wic/partition.py                       |  3 +-
 scripts/lib/wic/pluginbase.py                      |  3 +-
 scripts/lib/wic/plugins/imager/direct.py           | 11 ++---
 scripts/lib/wic/plugins/source/bootimg-efi.py      |  4 +-
 .../lib/wic/plugins/source/bootimg-partition.py    |  2 +-
 scripts/lib/wic/plugins/source/bootimg-pcbios.py   |  5 +--
 .../lib/wic/plugins/source/isoimage-isohybrid.py   |  2 +-
 scripts/lib/wic/plugins/source/rawcopy.py          |  2 +-
 scripts/lib/wic/plugins/source/rootfs.py           |  2 +-
 scripts/lib/wic/utils/__init__.py                  |  0
 scripts/lib/wic/utils/runner.py                    | 52 ----------------------
 scripts/wic                                        |  2 +-
 15 files changed, 58 insertions(+), 83 deletions(-)
 rename scripts/lib/wic/{utils => }/misc.py (86%)
 delete mode 100644 scripts/lib/wic/utils/__init__.py
 delete mode 100644 scripts/lib/wic/utils/runner.py

--
Regards,
Ed

[PATCH 1/4] runner.py: move runtool API to misc.py

[Ed Bartosh]

Moved remaining API to misc.py.
Removed runner.py.

Now misc.py is ready to be moved to the scripts/lib/wic and
utils directory can be removed.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/utils/misc.py   | 39 ++++++++++++++++++++++++++++---
 scripts/lib/wic/utils/runner.py | 52 -----------------------------------------
 2 files changed, 36 insertions(+), 55 deletions(-)
 delete mode 100644 scripts/lib/wic/utils/runner.py

diff --git a/scripts/lib/wic/utils/misc.py b/scripts/lib/wic/utils/misc.py
index 4609984..e1df824 100644
--- a/scripts/lib/wic/utils/misc.py
+++ b/scripts/lib/wic/utils/misc.py
@@ -29,12 +29,12 @@
 import logging
 import os
 import re
+import subprocess
 
 from collections import defaultdict
 from distutils import spawn
 
 from wic import WicError
-from wic.utils import runner
 
 logger = logging.getLogger('wic')
 
@@ -59,6 +59,39 @@ NATIVE_RECIPES = {"bmaptool": "bmap-tools",
                   "syslinux": "syslinux"
                  }
 
+def runtool(cmdln_or_args):
+    """ wrapper for most of the subprocess calls
+    input:
+        cmdln_or_args: can be both args and cmdln str (shell=True)
+    return:
+        rc, output
+    """
+    if isinstance(cmdln_or_args, list):
+        cmd = cmdln_or_args[0]
+        shell = False
+    else:
+        import shlex
+        cmd = shlex.split(cmdln_or_args)[0]
+        shell = True
+
+    sout = subprocess.PIPE
+    serr = subprocess.STDOUT
+
+    try:
+        process = subprocess.Popen(cmdln_or_args, stdout=sout,
+                                   stderr=serr, shell=shell)
+        sout, serr = process.communicate()
+        # combine stdout and stderr, filter None out and decode
+        out = ''.join([out.decode('utf-8') for out in [sout, serr] if out])
+    except OSError as err:
+        if err.errno == 2:
+            # [Errno 2] No such file or directory
+            raise WicError('Cannot run command: %s, lost dependency?' % cmd)
+        else:
+            raise # relay
+
+    return process.returncode, out
+
 def _exec_cmd(cmd_and_args, as_shell=False):
     """
     Execute command, catching stderr, stdout
@@ -70,9 +103,9 @@ def _exec_cmd(cmd_and_args, as_shell=False):
     logger.debug(args)
 
     if as_shell:
-        ret, out = runner.runtool(cmd_and_args)
+        ret, out = runtool(cmd_and_args)
     else:
-        ret, out = runner.runtool(args)
+        ret, out = runtool(args)
     out = out.strip()
     if ret != 0:
         raise WicError("_exec_cmd: %s returned '%s' instead of 0\noutput: %s" % \
diff --git a/scripts/lib/wic/utils/runner.py b/scripts/lib/wic/utils/runner.py
deleted file mode 100644
index 4aa00fb..0000000
--- a/scripts/lib/wic/utils/runner.py
+++ /dev/null
@@ -1,52 +0,0 @@
-#!/usr/bin/env python -tt
-#
-# Copyright (c) 2011 Intel, Inc.
-#
-# This program is free software; you can redistribute it and/or modify it
-# under the terms of the GNU General Public License as published by the Free
-# Software Foundation; version 2 of the License
-#
-# 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., 59
-# Temple Place - Suite 330, Boston, MA 02111-1307, USA.
-import subprocess
-
-from wic import WicError
-
-def runtool(cmdln_or_args):
-    """ wrapper for most of the subprocess calls
-    input:
-        cmdln_or_args: can be both args and cmdln str (shell=True)
-    return:
-        rc, output
-    """
-    if isinstance(cmdln_or_args, list):
-        cmd = cmdln_or_args[0]
-        shell = False
-    else:
-        import shlex
-        cmd = shlex.split(cmdln_or_args)[0]
-        shell = True
-
-    sout = subprocess.PIPE
-    serr = subprocess.STDOUT
-
-    try:
-        process = subprocess.Popen(cmdln_or_args, stdout=sout,
-                                   stderr=serr, shell=shell)
-        sout, serr = process.communicate()
-        # combine stdout and stderr, filter None out and decode
-        out = ''.join([out.decode('utf-8') for out in [sout, serr] if out])
-    except OSError as err:
-        if err.errno == 2:
-            # [Errno 2] No such file or directory
-            raise WicError('Cannot run command: %s, lost dependency?' % cmd)
-        else:
-            raise # relay
-
-    return process.returncode, out
-- 
2.1.4

[PATCH 2/4] wic: flatten directory structure

[Ed Bartosh]

Moved misc.py from wic/utils/ to wic/
Removed wic/utils directory

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/engine.py                            | 2 +-
 scripts/lib/wic/{utils => }/misc.py                  | 0
 scripts/lib/wic/partition.py                         | 2 +-
 scripts/lib/wic/pluginbase.py                        | 2 +-
 scripts/lib/wic/plugins/imager/direct.py             | 2 +-
 scripts/lib/wic/plugins/source/bootimg-efi.py        | 4 ++--
 scripts/lib/wic/plugins/source/bootimg-partition.py  | 2 +-
 scripts/lib/wic/plugins/source/bootimg-pcbios.py     | 5 ++---
 scripts/lib/wic/plugins/source/isoimage-isohybrid.py | 2 +-
 scripts/lib/wic/plugins/source/rawcopy.py            | 2 +-
 scripts/lib/wic/plugins/source/rootfs.py             | 2 +-
 scripts/lib/wic/utils/__init__.py                    | 0
 scripts/wic                                          | 2 +-
 13 files changed, 13 insertions(+), 14 deletions(-)
 rename scripts/lib/wic/{utils => }/misc.py (100%)
 delete mode 100644 scripts/lib/wic/utils/__init__.py

diff --git a/scripts/lib/wic/engine.py b/scripts/lib/wic/engine.py
index 2c899dd..2dc2fd5 100644
--- a/scripts/lib/wic/engine.py
+++ b/scripts/lib/wic/engine.py
@@ -38,7 +38,7 @@ from distutils.spawn import find_executable
 from wic import WicError
 from wic.filemap import sparse_copy
 from wic.pluginbase import PluginMgr
-from wic.utils.misc import get_bitbake_var, exec_cmd
+from wic.misc import get_bitbake_var, exec_cmd
 
 logger = logging.getLogger('wic')
 
diff --git a/scripts/lib/wic/utils/misc.py b/scripts/lib/wic/misc.py
similarity index 100%
rename from scripts/lib/wic/utils/misc.py
rename to scripts/lib/wic/misc.py
diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index 939e667..1851c4a 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -29,7 +29,7 @@ import os
 import tempfile
 
 from wic import WicError
-from wic.utils.misc import exec_cmd, exec_native_cmd, get_bitbake_var
+from wic.misc import exec_cmd, exec_native_cmd, get_bitbake_var
 from wic.pluginbase import PluginMgr
 
 logger = logging.getLogger('wic')
diff --git a/scripts/lib/wic/pluginbase.py b/scripts/lib/wic/pluginbase.py
index fb3d179..08f9979 100644
--- a/scripts/lib/wic/pluginbase.py
+++ b/scripts/lib/wic/pluginbase.py
@@ -24,7 +24,7 @@ from collections import defaultdict
 from importlib.machinery import SourceFileLoader
 
 from wic import WicError
-from wic.utils.misc import get_bitbake_var
+from wic.misc import get_bitbake_var
 
 PLUGIN_TYPES = ["imager", "source"]
 
diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py
index f252697..e09350b 100644
--- a/scripts/lib/wic/plugins/imager/direct.py
+++ b/scripts/lib/wic/plugins/imager/direct.py
@@ -36,7 +36,7 @@ from wic import WicError
 from wic.filemap import sparse_copy
 from wic.ksparser import KickStart, KickStartError
 from wic.pluginbase import PluginMgr, ImagerPlugin
-from wic.utils.misc import get_bitbake_var, exec_cmd, exec_native_cmd
+from wic.misc import get_bitbake_var, exec_cmd, exec_native_cmd
 
 logger = logging.getLogger('wic')
 
diff --git a/scripts/lib/wic/plugins/source/bootimg-efi.py b/scripts/lib/wic/plugins/source/bootimg-efi.py
index 9879cb9..4c4f36a 100644
--- a/scripts/lib/wic/plugins/source/bootimg-efi.py
+++ b/scripts/lib/wic/plugins/source/bootimg-efi.py
@@ -31,8 +31,8 @@ import shutil
 from wic import WicError
 from wic.engine import get_custom_config
 from wic.pluginbase import SourcePlugin
-from wic.utils.misc import (exec_cmd, exec_native_cmd, get_bitbake_var,
-                            BOOTDD_EXTRA_SPACE)
+from wic.misc import (exec_cmd, exec_native_cmd,
+                      get_bitbake_var, BOOTDD_EXTRA_SPACE)
 
 logger = logging.getLogger('wic')
 
diff --git a/scripts/lib/wic/plugins/source/bootimg-partition.py b/scripts/lib/wic/plugins/source/bootimg-partition.py
index 13fddbd..ca074a3 100644
--- a/scripts/lib/wic/plugins/source/bootimg-partition.py
+++ b/scripts/lib/wic/plugins/source/bootimg-partition.py
@@ -31,7 +31,7 @@ from glob import glob
 
 from wic import WicError
 from wic.pluginbase import SourcePlugin
-from wic.utils.misc import exec_cmd, get_bitbake_var
+from wic.misc import exec_cmd, get_bitbake_var
 
 logger = logging.getLogger('wic')
 
diff --git a/scripts/lib/wic/plugins/source/bootimg-pcbios.py b/scripts/lib/wic/plugins/source/bootimg-pcbios.py
index 98ad88b..e3518d2 100644
--- a/scripts/lib/wic/plugins/source/bootimg-pcbios.py
+++ b/scripts/lib/wic/plugins/source/bootimg-pcbios.py
@@ -29,10 +29,9 @@ import os
 
 from wic import WicError
 from wic.engine import get_custom_config
-from wic.utils import runner
 from wic.pluginbase import SourcePlugin
-from wic.utils.misc import (exec_cmd, exec_native_cmd,
-                            get_bitbake_var, BOOTDD_EXTRA_SPACE)
+from wic.misc import (exec_cmd, exec_native_cmd,
+                      get_bitbake_var, BOOTDD_EXTRA_SPACE)
 
 logger = logging.getLogger('wic')
 
diff --git a/scripts/lib/wic/plugins/source/isoimage-isohybrid.py b/scripts/lib/wic/plugins/source/isoimage-isohybrid.py
index b464263..db77113 100644
--- a/scripts/lib/wic/plugins/source/isoimage-isohybrid.py
+++ b/scripts/lib/wic/plugins/source/isoimage-isohybrid.py
@@ -29,7 +29,7 @@ import shutil
 from wic import WicError
 from wic.engine import get_custom_config
 from wic.pluginbase import SourcePlugin
-from wic.utils.misc import exec_cmd, exec_native_cmd, get_bitbake_var
+from wic.misc import exec_cmd, exec_native_cmd, get_bitbake_var
 
 logger = logging.getLogger('wic')
 
diff --git a/scripts/lib/wic/plugins/source/rawcopy.py b/scripts/lib/wic/plugins/source/rawcopy.py
index e1c4f5e..424ed26 100644
--- a/scripts/lib/wic/plugins/source/rawcopy.py
+++ b/scripts/lib/wic/plugins/source/rawcopy.py
@@ -20,7 +20,7 @@ import os
 
 from wic import WicError
 from wic.pluginbase import SourcePlugin
-from wic.utils.misc import exec_cmd, get_bitbake_var
+from wic.misc import exec_cmd, get_bitbake_var
 from wic.filemap import sparse_copy
 
 logger = logging.getLogger('wic')
diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
index 944ec5f..1b9f68b 100644
--- a/scripts/lib/wic/plugins/source/rootfs.py
+++ b/scripts/lib/wic/plugins/source/rootfs.py
@@ -34,7 +34,7 @@ from oe.path import copyhardlinktree
 
 from wic import WicError
 from wic.pluginbase import SourcePlugin
-from wic.utils.misc import get_bitbake_var, exec_cmd
+from wic.misc import get_bitbake_var, exec_cmd
 
 logger = logging.getLogger('wic')
 
diff --git a/scripts/lib/wic/utils/__init__.py b/scripts/lib/wic/utils/__init__.py
deleted file mode 100644
index e69de29..0000000
diff --git a/scripts/wic b/scripts/wic
index da14f47..81fc85d 100755
--- a/scripts/wic
+++ b/scripts/wic
@@ -56,7 +56,7 @@ else:
     bitbake_main = None
 
 from wic import WicError
-from wic.utils.misc import get_bitbake_var, BB_VARS
+from wic.misc import get_bitbake_var, BB_VARS
 from wic import engine
 from wic import help as hlp
 
-- 
2.1.4

[PATCH 3/4] wic: code cleanup

[Ed Bartosh]

Split long lines.
Removed unused imports.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/filemap.py               | 8 ++------
 scripts/lib/wic/misc.py                  | 4 ++--
 scripts/lib/wic/partition.py             | 1 -
 scripts/lib/wic/pluginbase.py            | 1 -
 scripts/lib/wic/plugins/source/rootfs.py | 2 +-
 5 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/scripts/lib/wic/filemap.py b/scripts/lib/wic/filemap.py
index 6d11355..77e32b9 100644
--- a/scripts/lib/wic/filemap.py
+++ b/scripts/lib/wic/filemap.py
@@ -34,13 +34,9 @@ def get_block_size(file_obj):
     Returns block size for file object 'file_obj'. Errors are indicated by the
     'IOError' exception.
     """
-
-    from fcntl import ioctl
-    import struct
-
     # Get the block size of the host file-system for the image file by calling
     # the FIGETBSZ ioctl (number 2).
-    binary_data = ioctl(file_obj, 2, struct.pack('I', 0))
+    binary_data = fcntl.ioctl(file_obj, 2, struct.pack('I', 0))
     return struct.unpack('I', binary_data)[0]
 
 class ErrorNotSupp(Exception):
@@ -228,7 +224,7 @@ class FilemapSeek(_FilemapBase):
         try:
             tmp_obj = tempfile.TemporaryFile("w+", dir=directory)
         except IOError as err:
-            raise ErrorNotSupp("cannot create a temporary in \"%s\": %s"
+            raise ErrorNotSupp("cannot create a temporary in \"%s\": %s" \
                               % (directory, err))
 
         try:
diff --git a/scripts/lib/wic/misc.py b/scripts/lib/wic/misc.py
index e1df824..3ebae0a 100644
--- a/scripts/lib/wic/misc.py
+++ b/scripts/lib/wic/misc.py
@@ -164,8 +164,8 @@ def exec_native_cmd(cmd_and_args, native_sysroot, pseudo=""):
               "was not found (see details above).\n\n" % prog
         recipe = NATIVE_RECIPES.get(prog)
         if recipe:
-            msg += "Please make sure wic-tools have %s-native in its DEPENDS, bake it with 'bitbake wic-tools' "\
-                   "and try again.\n" % recipe
+            msg += "Please make sure wic-tools have %s-native in its DEPENDS, "\
+                   "build it with 'bitbake wic-tools' and try again.\n" % recipe
         else:
             msg += "Wic failed to find a recipe to build native %s. Please "\
                    "file a bug against wic.\n" % prog
diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index 1851c4a..5aa68c9 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -26,7 +26,6 @@
 
 import logging
 import os
-import tempfile
 
 from wic import WicError
 from wic.misc import exec_cmd, exec_native_cmd, get_bitbake_var
diff --git a/scripts/lib/wic/pluginbase.py b/scripts/lib/wic/pluginbase.py
index 08f9979..c009820 100644
--- a/scripts/lib/wic/pluginbase.py
+++ b/scripts/lib/wic/pluginbase.py
@@ -137,4 +137,3 @@ class SourcePlugin(metaclass=PluginMeta):
         'prepares' the partition to be incorporated into the image.
         """
         logger.debug("SourcePlugin: do_prepare_partition: part: %s", part)
-
diff --git a/scripts/lib/wic/plugins/source/rootfs.py b/scripts/lib/wic/plugins/source/rootfs.py
index 1b9f68b..4dc4cb8 100644
--- a/scripts/lib/wic/plugins/source/rootfs.py
+++ b/scripts/lib/wic/plugins/source/rootfs.py
@@ -34,7 +34,7 @@ from oe.path import copyhardlinktree
 
 from wic import WicError
 from wic.pluginbase import SourcePlugin
-from wic.misc import get_bitbake_var, exec_cmd
+from wic.misc import get_bitbake_var
 
 logger = logging.getLogger('wic')
 
-- 
2.1.4

[PATCH 4/4] wic: fix restoring of original fstab

[Ed Bartosh]

Wic updates fstab if mount points are specified in .wks.
After partition images are created the original fstab is restored.
However, if exception is raised when partition image being prepared
wic doesn't restore original fstab. This can cause duplication of
added lines in fstab when 'wic create' runs next time.

Wrapping call of 'prepare' method and restoring original fstab
even if exception occurs should fix this.

[YOCTO #11633]

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/plugins/imager/direct.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/scripts/lib/wic/plugins/imager/direct.py b/scripts/lib/wic/plugins/imager/direct.py
index e09350b..e26defb 100644
--- a/scripts/lib/wic/plugins/imager/direct.py
+++ b/scripts/lib/wic/plugins/imager/direct.py
@@ -172,10 +172,11 @@ class DirectPlugin(ImagerPlugin):
                     if rsize_bb:
                         part.size = int(round(float(rsize_bb)))
 
-        self._image.prepare(self)
-
-        if fstab_path:
-            shutil.move(fstab_path + ".orig", fstab_path)
+        try:
+            self._image.prepare(self)
+        finally:
+            if fstab_path:
+                shutil.move(fstab_path + ".orig", fstab_path)
 
         self._image.layout_partitions()
         self._image.create()
-- 
2.1.4