[wic][PATCH v2 0/9] wic: implement 'include' command

[wic][PATCH v2 0/9] wic: implement 'include' command

[Ed Bartosh]

Hi,

Please review the implementation of 'include' command in
wks parser.

This patchset depends on my previous patchset 'wic: kickstart fixes' which
is in toaster-next, so I rebased this one on top of toaster-next.

Changes in V2: Fixed forgotten import. Pylinted kspareser.py

The following changes since commit 429137c9d785be79816029b987d6102978e936e0:

  classes/populate_sdk_ext: fix task dependency regression (2016-01-18 15:06:01 +0000)

are available in the git repository at:

  git://git.yoctoproject.org/poky-contrib ed/wic/kickstart-include-8848
  http://git.yoctoproject.org/cgit.cgi/poky-contrib/log/?h=ed/wic/kickstart-include-8848

Ed Bartosh (9):
  wic: move wks parsing code to KickStart._parse
  wic: use unique partition number
  wic: do not remove build dir in source plugins
  wic: ksparser: add support for include
  wic: refactor get_boot_config
  wic: implement search of includes
  wic: move parts of canned .wks into common.wks.inc
  wic: add help for 'include' command
  wic: pylinted ksparser module

 scripts/lib/wic/canned-wks/common.wks.inc          |  3 ++
 .../canned-wks/directdisk-bootloader-config.wks    |  4 +-
 scripts/lib/wic/canned-wks/directdisk.wks          |  4 +-
 scripts/lib/wic/canned-wks/qemux86-directdisk.wks  |  4 +-
 scripts/lib/wic/help.py                            | 14 +++++++
 scripts/lib/wic/imager/direct.py                   |  3 ++
 scripts/lib/wic/ksparser.py                        | 43 ++++++++++++++++++----
 scripts/lib/wic/plugins/source/bootimg-efi.py      |  2 -
 .../lib/wic/plugins/source/bootimg-partition.py    |  3 --
 scripts/lib/wic/plugins/source/bootimg-pcbios.py   |  2 -
 scripts/lib/wic/utils/misc.py                      | 16 ++++----
 11 files changed, 66 insertions(+), 32 deletions(-)
 create mode 100644 scripts/lib/wic/canned-wks/common.wks.inc

-- 
2.1.4

[wic][PATCH v2 1/9] wic: move wks parsing code to KickStart._parse

[Ed Bartosh]

This is a preparation for implementation of include statement.
Parser will be called recursively to parse included .wks files,
so it should be available as a method.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/ksparser.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
index 7dbe052..e366f61 100644
--- a/scripts/lib/wic/ksparser.py
+++ b/scripts/lib/wic/ksparser.py
@@ -115,6 +115,9 @@ class KickStart(object):
         bootloader.add_argument('--timeout', type=int)
         bootloader.add_argument('--source')
 
+        self._parse(parser, confpath)
+
+    def _parse(self, parser, confpath):
         with open(confpath) as conf:
             lineno = 0
             for line in conf:
-- 
2.1.4

[wic][PATCH v2 2/9] wic: use unique partition number

[Ed Bartosh]

This is a preparation for 'include' support.

Used unique counter instead of line number for partitions
in .ks file. Line numbers can be equal for different .ks files,
which can cause problems if one .ks file is included into
another.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/ksparser.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
index e366f61..0191a84 100644
--- a/scripts/lib/wic/ksparser.py
+++ b/scripts/lib/wic/ksparser.py
@@ -84,6 +84,7 @@ class KickStart(object):
         self.partitions = []
         self.bootloader = None
         self.lineno = 0
+        self.partnum = 0
 
         parser = KickStartParser()
         subparsers = parser.add_subparsers()
@@ -130,7 +131,8 @@ class KickStart(object):
                         raise KickStartError('%s:%d: %s' % \
                                              (confpath, lineno, err))
                     if line.startswith('part'):
-                        self.partitions.append(Partition(parsed, lineno))
+                        self.partnum += 1
+                        self.partitions.append(Partition(parsed, self.partnum))
                     else:
                         if not self.bootloader:
                              self.bootloader = parsed
-- 
2.1.4

[wic][PATCH v2 3/9] wic: do not remove build dir in source plugins

[Ed Bartosh]

Interesting bug was found during implementation of 'include'
parser command.

Build directory was removed in do_configure_partition method of
bootimg- source plugins. This can cause removal of previously
prepared partition images if /boot partition is mentioned after
other partitions in .ks file.

Moved work directory removal to direct.py before processing
partitions.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/imager/direct.py                    | 3 +++
 scripts/lib/wic/plugins/source/bootimg-efi.py       | 2 --
 scripts/lib/wic/plugins/source/bootimg-partition.py | 3 ---
 scripts/lib/wic/plugins/source/bootimg-pcbios.py    | 2 --
 4 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/scripts/lib/wic/imager/direct.py b/scripts/lib/wic/imager/direct.py
index 8d4daec..a1b4249 100644
--- a/scripts/lib/wic/imager/direct.py
+++ b/scripts/lib/wic/imager/direct.py
@@ -229,6 +229,9 @@ class DirectImageCreator(BaseImageCreator):
 
         fstab_path = self._write_fstab(self.rootfs_dir.get("ROOTFS_DIR"))
 
+        shutil.rmtree(self.workdir)
+        os.mkdir(self.workdir)
+
         for part in parts:
             # get rootfs size from bitbake variable if it's not set in .ks file
             if not part.size:
diff --git a/scripts/lib/wic/plugins/source/bootimg-efi.py b/scripts/lib/wic/plugins/source/bootimg-efi.py
index 125c943..a4734c9 100644
--- a/scripts/lib/wic/plugins/source/bootimg-efi.py
+++ b/scripts/lib/wic/plugins/source/bootimg-efi.py
@@ -142,8 +142,6 @@ class BootimgEFIPlugin(SourcePlugin):
         Called before do_prepare_partition(), creates loader-specific config
         """
         hdddir = "%s/hdd/boot" % cr_workdir
-        rm_cmd = "rm -rf %s" % cr_workdir
-        exec_cmd(rm_cmd)
 
         install_cmd = "install -d %s/EFI/BOOT" % hdddir
         exec_cmd(install_cmd)
diff --git a/scripts/lib/wic/plugins/source/bootimg-partition.py b/scripts/lib/wic/plugins/source/bootimg-partition.py
index bc2ca0f..b76c121 100644
--- a/scripts/lib/wic/plugins/source/bootimg-partition.py
+++ b/scripts/lib/wic/plugins/source/bootimg-partition.py
@@ -71,9 +71,6 @@ class BootimgPartitionPlugin(SourcePlugin):
         - copies all files listed in IMAGE_BOOT_FILES variable
         """
         hdddir = "%s/boot" % cr_workdir
-        rm_cmd = "rm -rf %s/boot" % cr_workdir
-        exec_cmd(rm_cmd)
-
         install_cmd = "install -d %s" % hdddir
         exec_cmd(install_cmd)
 
diff --git a/scripts/lib/wic/plugins/source/bootimg-pcbios.py b/scripts/lib/wic/plugins/source/bootimg-pcbios.py
index a1bfa26..5b719bf 100644
--- a/scripts/lib/wic/plugins/source/bootimg-pcbios.py
+++ b/scripts/lib/wic/plugins/source/bootimg-pcbios.py
@@ -78,8 +78,6 @@ class BootimgPcbiosPlugin(SourcePlugin):
         Called before do_prepare_partition(), creates syslinux config
         """
         hdddir = "%s/hdd/boot" % cr_workdir
-        rm_cmd = "rm -rf " + cr_workdir
-        exec_cmd(rm_cmd)
 
         install_cmd = "install -d %s" % hdddir
         exec_cmd(install_cmd)
-- 
2.1.4

[wic][PATCH v2 4/9] wic: ksparser: add support for include

[Ed Bartosh]

Extended parser to support inclusion of .ks files:
    recursively called self._parse to parse included .ks

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/ksparser.py | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
index 0191a84..c73a456 100644
--- a/scripts/lib/wic/ksparser.py
+++ b/scripts/lib/wic/ksparser.py
@@ -116,6 +116,9 @@ class KickStart(object):
         bootloader.add_argument('--timeout', type=int)
         bootloader.add_argument('--source')
 
+        include = subparsers.add_parser('include')
+        include.add_argument('path')
+
         self._parse(parser, confpath)
 
     def _parse(self, parser, confpath):
@@ -133,7 +136,9 @@ class KickStart(object):
                     if line.startswith('part'):
                         self.partnum += 1
                         self.partitions.append(Partition(parsed, self.partnum))
-                    else:
+                    elif line.startswith('include'):
+                        self._parse(parser, parsed.path)
+                    elif line.startswith('bootloader'):
                         if not self.bootloader:
                              self.bootloader = parsed
                         else:
-- 
2.1.4

[wic][PATCH v2 5/9] wic: refactor get_boot_config

[Ed Bartosh]

This function is going to be used by ks parser to find include .wks
files. get_boot_config name is a bit confusing as function is quite
generic. It looks if file is present in the canned wks directories.

Renamed get_boot_config -> get_canned.
Renamed parameter file_boot -> file_name.
Updated description.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/utils/misc.py | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/scripts/lib/wic/utils/misc.py b/scripts/lib/wic/utils/misc.py
index d886d75..1415ae9 100644
--- a/scripts/lib/wic/utils/misc.py
+++ b/scripts/lib/wic/utils/misc.py
@@ -58,25 +58,23 @@ def build_name(kscfg, release=None, prefix=None, suffix=None):
 
     return ret
 
-def find_boot_config(scripts_path, boot_file):
+def find_canned(scripts_path, file_name):
     """
-    Find a config file with the given name in the canned files dir.
+    Find a file either by its path or by name in the canned files dir.
 
-    Return False if not found
+    Return None if not found
     """
-    if os.path.exists(boot_file):
-        return boot_file
+    if os.path.exists(file_name):
+        return file_name
 
     layers_canned_wks_dir = wic.engine.build_canned_image_list(scripts_path)
     for canned_wks_dir in layers_canned_wks_dir:
         for root, dirs, files in os.walk(canned_wks_dir):
             for fname in files:
-                if fname == boot_file:
+                if fname == file_name:
                     fullpath = os.path.join(canned_wks_dir, fname)
                     return fullpath
 
-    return None
-
 def get_custom_config(boot_file):
     """
     Get the custom configuration to be used for the bootloader.
@@ -88,7 +86,7 @@ def get_custom_config(boot_file):
     for x in range(0, 3):
         scripts_path = os.path.dirname(scripts_path)
 
-    cfg_file = find_boot_config(scripts_path, boot_file)
+    cfg_file = find_canned(scripts_path, boot_file)
     if cfg_file:
         with open(cfg_file, "r") as f:
             config = f.read()
-- 
2.1.4

[wic][PATCH v2 6/9] wic: implement search of includes

[Ed Bartosh]

Used custom argument type to implement search of include
.wks files in canned wks paths. Include files can be
specified either by full path or by name.

[YOCTO #8848]

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/ksparser.py | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
index c73a456..3722799 100644
--- a/scripts/lib/wic/ksparser.py
+++ b/scripts/lib/wic/ksparser.py
@@ -25,11 +25,12 @@
 # Ed Bartosh <ed.bartosh> (at] linux.intel.com>
 
 
-
+import os
 import shlex
 from argparse import ArgumentParser, ArgumentError, ArgumentTypeError
 
 from wic.partition import Partition
+from wic.utils.misc import find_canned
 
 class KickStartError(Exception):
     pass
@@ -78,6 +79,17 @@ def overheadtype(arg):
 
     return result
 
+def cannedpathtype(arg):
+    """
+    Custom type for ArgumentParser
+    Tries to find file in the list of canned wks paths
+    """
+    scripts_path = os.path.abspath(os.path.dirname(__file__) + '../../..')
+    result = find_canned(scripts_path, arg)
+    if not result:
+        raise ArgumentTypeError("file not found: %s" % arg)
+    return result
+
 class KickStart(object):
     def __init__(self, confpath):
 
@@ -117,7 +129,7 @@ class KickStart(object):
         bootloader.add_argument('--source')
 
         include = subparsers.add_parser('include')
-        include.add_argument('path')
+        include.add_argument('path', type=cannedpathtype)
 
         self._parse(parser, confpath)
 
-- 
2.1.4

[wic][PATCH v2 7/9] wic: move parts of canned .wks into common.wks.inc

[Ed Bartosh]

In order to give and example of 'include' feature of ks parser
and for testing purposes common parts of 3 canned wks files were
moved into common.wks.inc

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/canned-wks/common.wks.inc                   | 3 +++
 scripts/lib/wic/canned-wks/directdisk-bootloader-config.wks | 4 +---
 scripts/lib/wic/canned-wks/directdisk.wks                   | 4 +---
 scripts/lib/wic/canned-wks/qemux86-directdisk.wks           | 4 +---
 4 files changed, 6 insertions(+), 9 deletions(-)
 create mode 100644 scripts/lib/wic/canned-wks/common.wks.inc

diff --git a/scripts/lib/wic/canned-wks/common.wks.inc b/scripts/lib/wic/canned-wks/common.wks.inc
new file mode 100644
index 0000000..5cf2fd1
--- /dev/null
+++ b/scripts/lib/wic/canned-wks/common.wks.inc
@@ -0,0 +1,3 @@
+# This file is included into 3 canned wks files from this directory
+part /boot --source bootimg-pcbios --ondisk sda --label boot --active --align 1024
+part / --source rootfs --ondisk sda --fstype=ext4 --label platform --align 1024
diff --git a/scripts/lib/wic/canned-wks/directdisk-bootloader-config.wks b/scripts/lib/wic/canned-wks/directdisk-bootloader-config.wks
index 4808d04..3529e05 100644
--- a/scripts/lib/wic/canned-wks/directdisk-bootloader-config.wks
+++ b/scripts/lib/wic/canned-wks/directdisk-bootloader-config.wks
@@ -2,9 +2,7 @@
 # long-description: Creates a partitioned legacy BIOS disk image that the user
 # can directly dd to boot media. The bootloader configuration source is a user file.
 
-
-part /boot --source bootimg-pcbios --ondisk sda --label boot --active --align 1024
-part / --source rootfs --ondisk sda --fstype=ext4 --label platform --align 1024
+include common.wks.inc
 
 bootloader --configfile="directdisk-bootloader-config.cfg"
 
diff --git a/scripts/lib/wic/canned-wks/directdisk.wks b/scripts/lib/wic/canned-wks/directdisk.wks
index af4c9ea..6db74a7 100644
--- a/scripts/lib/wic/canned-wks/directdisk.wks
+++ b/scripts/lib/wic/canned-wks/directdisk.wks
@@ -2,9 +2,7 @@
 # long-description: Creates a partitioned legacy BIOS disk image that the user
 # can directly dd to boot media.
 
-
-part /boot --source bootimg-pcbios --ondisk sda --label boot --active --align 1024
-part / --source rootfs --ondisk sda --fstype=ext4 --label platform --align 1024
+include common.wks.inc
 
 bootloader  --timeout=0  --append="rootwait rootfstype=ext4 video=vesafb vga=0x318 console=tty0"
 
diff --git a/scripts/lib/wic/canned-wks/qemux86-directdisk.wks b/scripts/lib/wic/canned-wks/qemux86-directdisk.wks
index 8fc38b5..a6518a0 100644
--- a/scripts/lib/wic/canned-wks/qemux86-directdisk.wks
+++ b/scripts/lib/wic/canned-wks/qemux86-directdisk.wks
@@ -2,9 +2,7 @@
 # long-description: Creates a partitioned legacy BIOS disk image that the user
 # can directly use to boot a qemu machine.
 
-
-part /boot --source bootimg-pcbios --ondisk sda --label boot --active --align 1024
-part / --source rootfs --ondisk sda --fstype=ext4 --label platform --align 1024
+include common.wks.inc
 
 bootloader  --timeout=0  --append="vga=0 uvesafb.mode_option=640x480-32 root=/dev/vda2 rw mem=256M ip=192.168.7.2::192.168.7.1:255.255.255.0 oprofile.timer=1 rootfstype=ext4 "
 
-- 
2.1.4

[wic][PATCH v2 8/9] wic: add help for 'include' command

[Ed Bartosh]

Added description of 'include' parser command to the
'wic help kickstart' output.

Signed-off-by: Ed Bartosh <ed.bartosh@linux.intel.com>
---
 scripts/lib/wic/help.py | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
index f5587bb..405d25a 100644
--- a/scripts/lib/wic/help.py
+++ b/scripts/lib/wic/help.py
@@ -760,4 +760,18 @@ DESCRIPTION
       implemented by the various --source plugins that implement
       bootloader functionality; the bootloader command essentially
       provides a means of modifying bootloader configuration.
+
+    * include
+
+      This command allows the user to include the content of .wks file
+      into original .wks file.
+
+      Command uses the following syntax:
+
+         include <file>
+
+      The <file> is either path to the file or its name. If name is
+      specified wic will try to find file in the directories with canned
+      .wks files.
+
 """
-- 
2.1.4

[wic][PATCH v2 9/9] wic: pylinted ksparser module

[Ed Bartosh]

Added missing docstrings, fixed wrong indentation and long lines.
Final pylint score is 9.89/10

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

diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
index 3722799..f2a0e04 100644
--- a/scripts/lib/wic/ksparser.py
+++ b/scripts/lib/wic/ksparser.py
@@ -21,9 +21,9 @@
 # This module provides parser for kickstart format
 #
 # AUTHORS
-# Tom Zanussi <tom.zanussi (at] linux.intel.com>
 # Ed Bartosh <ed.bartosh> (at] linux.intel.com>
 
+"""Kickstart parser module."""
 
 import os
 import shlex
@@ -33,6 +33,7 @@ from wic.partition import Partition
 from wic.utils.misc import find_canned
 
 class KickStartError(Exception):
+    """Custom exception."""
     pass
 
 class KickStartParser(ArgumentParser):
@@ -91,6 +92,8 @@ def cannedpathtype(arg):
     return result
 
 class KickStart(object):
+    """"Kickstart parser implementation."""
+
     def __init__(self, confpath):
 
         self.partitions = []
@@ -134,6 +137,9 @@ class KickStart(object):
         self._parse(parser, confpath)
 
     def _parse(self, parser, confpath):
+        """
+        Parse file in .wks format using provided parser.
+        """
         with open(confpath) as conf:
             lineno = 0
             for line in conf:
@@ -152,7 +158,8 @@ class KickStart(object):
                         self._parse(parser, parsed.path)
                     elif line.startswith('bootloader'):
                         if not self.bootloader:
-                             self.bootloader = parsed
+                            self.bootloader = parsed
                         else:
-                             raise KickStartError("%s:%d: more than one bootloader "\
-                                                  "specified" % (confpath, lineno))
+                            err = "%s:%d: more than one bootloader specified" \
+                                      % (confpath, lineno)
+                            raise KickStartError(err)
-- 
2.1.4