[PATCH v2 00/10] wic: bugfixes & --fixed-size support, tests, selftest: minor fixes

[PATCH v2 00/10] wic: bugfixes & --fixed-size support, tests, selftest: minor fixes

[Maciej Borzecki]

v2 of patch series previously posted here [1].

The series introduces anumber of fixes to wic, as well as a new --fixed-size
option applicable to `part` entries in kickstart files. The option makes it
possible to have a fixed size partition, with additional verification that the
file system image fits into the allocated disk space. This is in cotrast with
--size option, which the minimum size, and partition may in fact be larger in
the final disk image.

The series introduces two tests that verify if wic has created the image with
properly sized partition and that partition size checks work as expected.

Patch `oe-selftest: fix handling of test cases without ID in --list-tests-by` is
a small bugfix for oe-selftes tool which resolves an issue triggered by Python
3.x being more strict than 2.x.

[1]. http://lists.openembedded.org/pipermail/openembedded-core/2016-November/128540.html

Maciej Borzecki (10):
  wic: make sure that partition size is always an integer in internal
    processing
  wic: use partition size when creating empty partition files
  wic: check that filesystem is specified for a rootfs partition
  wic: fix function comment typos
  wic: add --fixed-size wks option
  oe-selftest: fix handling of test cases without ID in --list-tests-by
  wic: selftest: avoid COMPATIBLE_HOST issues
  wic: selftest: do not repeat core-image-minimal
  wic: selftest: do not assume bzImage kernel image
  wic: selftest: add tests for --fixed-size partition flags

 meta/lib/oeqa/selftest/wic.py                 | 193 ++++++++++++++++++++------
 scripts/lib/wic/help.py                       |  14 +-
 scripts/lib/wic/imager/direct.py              |   2 +-
 scripts/lib/wic/ksparser.py                   |  41 +++++-
 scripts/lib/wic/partition.py                  | 104 +++++++++-----
 scripts/lib/wic/plugins/source/bootimg-efi.py |   2 +-
 scripts/lib/wic/plugins/source/rawcopy.py     |   4 +-
 scripts/lib/wic/utils/partitionedfs.py        |   6 +-
 scripts/oe-selftest                           |  13 +-
 9 files changed, 282 insertions(+), 97 deletions(-)

-- 
2.5.0

[PATCH v2 01/10] wic: make sure that partition size is always an integer in internal processing

[Maciej Borzecki]

The size field of Partition class is expected to be an integer and ought
to be set inside prepare_*() method. Make sure that this is always the
case.

Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
---
 scripts/lib/wic/partition.py                  | 12 +++++++++---
 scripts/lib/wic/plugins/source/bootimg-efi.py |  2 +-
 scripts/lib/wic/plugins/source/rawcopy.py     |  4 ++--
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index 89c33ab8b7d54bb14678b2e07e706e3feb6ae57a..959035a97110244ffe56e95a886e122c400d4779 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -146,6 +146,12 @@ class Partition():
                                                      oe_builddir,
                                                      bootimg_dir, kernel_dir, rootfs_dir,
                                                      native_sysroot)
+        # further processing required Partition.size to be an integer, make
+        # sure that it is one
+        if type(self.size) is not int:
+            msger.error("Partition %s internal size is not an integer. " \
+                          "This a bug in source plugin %s and needs to be fixed." \
+                          % (self.mountpoint, self.source))
 
     def prepare_rootfs_from_fs_image(self, cr_workdir, oe_builddir,
                                      rootfs_dir):
@@ -157,7 +163,7 @@ class Partition():
         out = exec_cmd(du_cmd)
         rootfs_size = out.split()[0]
 
-        self.size = rootfs_size
+        self.size = int(rootfs_size)
         self.source_file = rootfs
 
     def prepare_rootfs(self, cr_workdir, oe_builddir, rootfs_dir,
@@ -194,7 +200,7 @@ class Partition():
                 # get the rootfs size in the right units for kickstart (kB)
                 du_cmd = "du -Lbks %s" % rootfs
                 out = exec_cmd(du_cmd)
-                self.size = out.split()[0]
+                self.size = int(out.split()[0])
 
                 break
 
@@ -379,7 +385,7 @@ class Partition():
         out = exec_cmd(du_cmd)
         fs_size = out.split()[0]
 
-        self.size = fs_size
+        self.size = int(fs_size)
 
     def prepare_swap_partition(self, cr_workdir, oe_builddir, native_sysroot):
         """
diff --git a/scripts/lib/wic/plugins/source/bootimg-efi.py b/scripts/lib/wic/plugins/source/bootimg-efi.py
index 8bc362254d8c06511aa2cf0d5e1bf6f5aa93804b..4adb80becc11a6d30ffeae64ff87ebeb959dde86 100644
--- a/scripts/lib/wic/plugins/source/bootimg-efi.py
+++ b/scripts/lib/wic/plugins/source/bootimg-efi.py
@@ -234,5 +234,5 @@ class BootimgEFIPlugin(SourcePlugin):
         out = exec_cmd(du_cmd)
         bootimg_size = out.split()[0]
 
-        part.size = bootimg_size
+        part.size = int(bootimg_size)
         part.source_file = bootimg
diff --git a/scripts/lib/wic/plugins/source/rawcopy.py b/scripts/lib/wic/plugins/source/rawcopy.py
index e0b11f95adb5a2c55fdc3b5e3ff1f4b463e2be9d..5bd22fdeb55bc2f0b38ffcc2a46cf18ade5425ef 100644
--- a/scripts/lib/wic/plugins/source/rawcopy.py
+++ b/scripts/lib/wic/plugins/source/rawcopy.py
@@ -78,9 +78,9 @@ class RawCopyPlugin(SourcePlugin):
         # get the size in the right units for kickstart (kB)
         du_cmd = "du -Lbks %s" % dst
         out = exec_cmd(du_cmd)
-        filesize = out.split()[0]
+        filesize = int(out.split()[0])
 
-        if int(filesize) > int(part.size):
+        if filesize > part.size:
             part.size = filesize
 
         part.source_file = dst
-- 
2.5.0

[PATCH v2 02/10] wic: use partition size when creating empty partition files

[Maciej Borzecki]

It seems that prepare_empty_partition_ext() and
prepare_empty_partition_btrfs() got broken in commit
c8669749e37fe865c197c98d5671d9de176ff4dd, thus one could observe the
following backtrace:

Backtrace:
  File "<snip>/poky/scripts/lib/wic/plugins/imager/direct_plugin.py", line 93, in do_create
    creator.create()
  File "<snip>/poky/scripts/lib/wic/imager/baseimager.py", line 159, in create
    self._create()
  File "<snip>/poky/scripts/lib/wic/imager/direct.py", line 290, in _create
    self.bootimg_dir, self.kernel_dir, self.native_sysroot)
  File "<snip>/poky/scripts/lib/wic/partition.py", line 146, in prepare
    method(rootfs, oe_builddir, native_sysroot)
  File "<snip>/poky/scripts/lib/wic/partition.py", line 325, in prepare_empty_partition_ext
    os.ftruncate(sparse.fileno(), rootfs_size * 1024)
NameError: name 'rootfs_size' is not defined

Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
---
 scripts/lib/wic/partition.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index 959035a97110244ffe56e95a886e122c400d4779..f3835339afc5091604ffd7f0d0acf1d1ad4351cc 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -314,7 +314,7 @@ class Partition():
         Prepare an empty ext2/3/4 partition.
         """
         with open(rootfs, 'w') as sparse:
-            os.ftruncate(sparse.fileno(), rootfs_size * 1024)
+            os.ftruncate(sparse.fileno(), self.size * 1024)
 
         extra_imagecmd = "-i 8192"
 
@@ -332,7 +332,7 @@ class Partition():
         Prepare an empty btrfs partition.
         """
         with open(rootfs, 'w') as sparse:
-            os.ftruncate(sparse.fileno(), rootfs_size * 1024)
+            os.ftruncate(sparse.fileno(), self.size * 1024)
 
         label_str = ""
         if self.label:
-- 
2.5.0

[PATCH v2 03/10] wic: check that filesystem is specified for a rootfs partition

[Maciej Borzecki]

We explicitly check for --fstype if no source was provided for a
partition. However, this was not the case for rootfs partitions. Make
sure to raise an error if filesystem was left unspecified when preparing
a rootfs partition image.

Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
---
 scripts/lib/wic/partition.py | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index f3835339afc5091604ffd7f0d0acf1d1ad4351cc..ac4c836bdb53300d3a4e4c09926b7b1514b8faf2 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -190,6 +190,10 @@ class Partition():
         if os.path.isfile(rootfs):
             os.remove(rootfs)
 
+        if not self.fstype:
+            msger.error("File system for partition %s not specified in kickstart, " \
+                        "use --fstype option" % (self.mountpoint))
+
         for prefix in ("ext", "btrfs", "vfat", "squashfs"):
             if self.fstype.startswith(prefix):
                 method = getattr(self, "prepare_rootfs_" + prefix)
-- 
2.5.0

[PATCH v2 04/10] wic: fix function comment typos

[Maciej Borzecki]

Fix typos in documentation of Image.add_partition() and
Image.__format_disks().

Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
---
 scripts/lib/wic/utils/partitionedfs.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/scripts/lib/wic/utils/partitionedfs.py b/scripts/lib/wic/utils/partitionedfs.py
index cb03009fc7e3c97305079629ded7d2ff01eba4c4..9e76487844eebfffc7227d053a65dc9fdab3678b 100644
--- a/scripts/lib/wic/utils/partitionedfs.py
+++ b/scripts/lib/wic/utils/partitionedfs.py
@@ -92,7 +92,7 @@ class Image():
     def add_partition(self, size, disk_name, mountpoint, source_file=None, fstype=None,
                       label=None, fsopts=None, boot=False, align=None, no_table=False,
                       part_type=None, uuid=None, system_id=None):
-        """ Add the next partition. Prtitions have to be added in the
+        """ Add the next partition. Partitions have to be added in the
         first-to-last order. """
 
         ks_pnum = len(self.partitions)
@@ -292,7 +292,7 @@ class Image():
             # even number of sectors.
             if part['mountpoint'] == "/boot" and part['fstype'] in ["vfat", "msdos"] \
                and part['size'] % 2:
-                msger.debug("Substracting one sector from '%s' partition to " \
+                msger.debug("Subtracting one sector from '%s' partition to " \
                             "get even number of sectors for the partition" % \
                             part['mountpoint'])
                 part['size'] -= 1
-- 
2.5.0

[PATCH v2 05/10] wic: add --fixed-size wks option

[Maciej Borzecki]

Added new option --fixed-size to wks. The option can be used to indicate
the exact size of a partition. The option cannot be added together with
--size, in which case an error will be raised. Other options that
influence automatic partition size (--extra-space, --overhead-factor),
if specifiec along with --fixed-size, will raise an error.

If it partition data is larger than the amount of space specified with
--fixed-size option wic will raise an error.

Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
---
 scripts/lib/wic/help.py                | 14 ++++--
 scripts/lib/wic/imager/direct.py       |  2 +-
 scripts/lib/wic/ksparser.py            | 41 ++++++++++++++--
 scripts/lib/wic/partition.py           | 88 +++++++++++++++++++++-------------
 scripts/lib/wic/utils/partitionedfs.py |  2 +-
 5 files changed, 105 insertions(+), 42 deletions(-)

diff --git a/scripts/lib/wic/help.py b/scripts/lib/wic/help.py
index e5347ec4b7c900c68fc64351a5293e75de0672b3..daa11bf489c135627ddfe4cef968e48f8e3ad1d8 100644
--- a/scripts/lib/wic/help.py
+++ b/scripts/lib/wic/help.py
@@ -646,6 +646,12 @@ DESCRIPTION
                  not specified, the size is in MB.
                  You do not need this option if you use --source.
 
+         --fixed-size: Exact partition size. Value format is the same
+                       as for --size option. This option cannot be
+                       specified along with --size. If partition data
+                       is larger than --fixed-size and error will be
+                       raised when assembling disk image.
+
          --source: This option is a wic-specific option that names the
                    source of the data that will populate the
                    partition.  The most common value for this option
@@ -719,13 +725,15 @@ DESCRIPTION
                         space after the space filled by the content
                         of the partition. The final size can go
                         beyond the size specified by --size.
-                        By default, 10MB.
+                        By default, 10MB. This option cannot be used
+                        with --fixed-size option.
 
          --overhead-factor: This option is specific to wic. The
                             size of the partition is multiplied by
                             this factor. It has to be greater than or
-                            equal to 1.
-                            The default value is 1.3.
+                            equal to 1. The default value is 1.3.
+                            This option cannot be used with --fixed-size
+                            option.
 
          --part-type: This option is specific to wic. It specifies partition
                       type GUID for GPT partitions.
diff --git a/scripts/lib/wic/imager/direct.py b/scripts/lib/wic/imager/direct.py
index 2bedef08d6450096c786def6f75a9ee53fcd4b3b..11ec15e33f65885618c7adc83e55c6a39fedbe99 100644
--- a/scripts/lib/wic/imager/direct.py
+++ b/scripts/lib/wic/imager/direct.py
@@ -290,7 +290,7 @@ class DirectImageCreator(BaseImageCreator):
                          self.bootimg_dir, self.kernel_dir, self.native_sysroot)
 
 
-            self.__image.add_partition(int(part.size),
+            self.__image.add_partition(part.disk_size,
                                        part.disk,
                                        part.mountpoint,
                                        part.source_file,
diff --git a/scripts/lib/wic/ksparser.py b/scripts/lib/wic/ksparser.py
index 0894e2b199a299fbbed272f2e1c95e9d692e3ab1..62c490274aa92bf82aac304d9323250e3b728d0c 100644
--- a/scripts/lib/wic/ksparser.py
+++ b/scripts/lib/wic/ksparser.py
@@ -113,6 +113,9 @@ def systemidtype(arg):
 class KickStart():
     """"Kickstart parser implementation."""
 
+    DEFAULT_EXTRA_SPACE = 10*1024
+    DEFAULT_OVERHEAD_FACTOR = 1.3
+
     def __init__(self, confpath):
 
         self.partitions = []
@@ -127,16 +130,24 @@ class KickStart():
         part.add_argument('mountpoint', nargs='?')
         part.add_argument('--active', action='store_true')
         part.add_argument('--align', type=int)
-        part.add_argument("--extra-space", type=sizetype, default=10*1024)
+        part.add_argument("--extra-space", type=sizetype)
         part.add_argument('--fsoptions', dest='fsopts')
         part.add_argument('--fstype')
         part.add_argument('--label')
         part.add_argument('--no-table', action='store_true')
         part.add_argument('--ondisk', '--ondrive', dest='disk')
-        part.add_argument("--overhead-factor", type=overheadtype, default=1.3)
+        part.add_argument("--overhead-factor", type=overheadtype)
         part.add_argument('--part-type')
         part.add_argument('--rootfs-dir')
-        part.add_argument('--size', type=sizetype, default=0)
+
+        # --size and --fixed-size cannot be specified together; options
+        # ----extra-space and --overhead-factor should also raise a parser
+        # --error, but since nesting mutually exclusive groups does not work,
+        # ----extra-space/--overhead-factor are handled later
+        sizeexcl = part.add_mutually_exclusive_group()
+        sizeexcl.add_argument('--size', type=sizetype, default=0)
+        sizeexcl.add_argument('--fixed-size', type=sizetype, default=0)
+
         part.add_argument('--source')
         part.add_argument('--sourceparams')
         part.add_argument('--system-id', type=systemidtype)
@@ -170,11 +181,33 @@ class KickStart():
                 lineno += 1
                 if line and line[0] != '#':
                     try:
-                        parsed = parser.parse_args(shlex.split(line))
+                        line_args = shlex.split(line)
+                        parsed = parser.parse_args(line_args)
                     except ArgumentError as err:
                         raise KickStartError('%s:%d: %s' % \
                                              (confpath, lineno, err))
                     if line.startswith('part'):
+                        # using ArgumentParser one cannot easily tell if option
+                        # was passed as argument, if said option has a default
+                        # value; --overhead-factor/--extra-space cannot be used
+                        # with --fixed-size, so at least detect when these were
+                        # passed with non-0 values ...
+                        if parsed.fixed_size:
+                            if parsed.overhead_factor or parsed.extra_space:
+                                err = "%s:%d: arguments --overhead-factor and --extra-space not "\
+                                      "allowed with argument --fixed-size" \
+                                      % (confpath, lineno)
+                                raise KickStartError(err)
+                        else:
+                            # ... and provide defaults if not using
+                            # --fixed-size iff given option was not used
+                            # (again, one cannot tell if option was passed but
+                            # with value equal to 0)
+                            if '--overhead-factor' not in line_args:
+                                parsed.overhead_factor = self.DEFAULT_OVERHEAD_FACTOR
+                            if '--extra-space' not in line_args:
+                                parsed.extra_space = self.DEFAULT_EXTRA_SPACE
+
                         self.partnum += 1
                         self.partitions.append(Partition(parsed, self.partnum))
                     elif line.startswith('include'):
diff --git a/scripts/lib/wic/partition.py b/scripts/lib/wic/partition.py
index ac4c836bdb53300d3a4e4c09926b7b1514b8faf2..8cf966ebc6d07490c44cefc93acbe5868be30ac7 100644
--- a/scripts/lib/wic/partition.py
+++ b/scripts/lib/wic/partition.py
@@ -54,6 +54,7 @@ class Partition():
         self.part_type = args.part_type
         self.rootfs_dir = args.rootfs_dir
         self.size = args.size
+        self.fixed_size = args.fixed_size
         self.source = args.source
         self.sourceparams = args.sourceparams
         self.system_id = args.system_id
@@ -87,6 +88,41 @@ class Partition():
         else:
             return 0
 
+    def get_rootfs_size(self, actual_rootfs_size=0):
+        """
+        Calculate the required size of rootfs taking into consideration
+        --size/--fixed-size flags as well as overhead and extra space, as
+        specified in kickstart file. Raises an error if the
+        `actual_rootfs_size` is larger than fixed-size rootfs.
+
+        """
+        if self.fixed_size:
+            rootfs_size = self.fixed_size
+            if actual_rootfs_size > rootfs_size:
+                msger.error("Actual rootfs size (%d kB) is larger than allowed size %d kB" \
+                            %(actual_rootfs_size, rootfs_size))
+        else:
+            extra_blocks = self.get_extra_block_count(actual_rootfs_size)
+            if extra_blocks < self.extra_space:
+                extra_blocks = self.extra_space
+
+            rootfs_size = actual_rootfs_size + extra_blocks
+            rootfs_size *= self.overhead_factor
+
+            msger.debug("Added %d extra blocks to %s to get to %d total blocks" % \
+                        (extra_blocks, self.mountpoint, rootfs_size))
+
+        return rootfs_size
+
+    @property
+    def disk_size(self):
+        """
+        Obtain on-disk size of partition taking into consideration
+        --size/--fixed-size options.
+
+        """
+        return self.fixed_size if self.fixed_size else self.size
+
     def prepare(self, creator, cr_workdir, oe_builddir, rootfs_dir,
                 bootimg_dir, kernel_dir, native_sysroot):
         """
@@ -97,9 +133,9 @@ class Partition():
             self.sourceparams_dict = parse_sourceparams(self.sourceparams)
 
         if not self.source:
-            if not self.size:
-                msger.error("The %s partition has a size of zero.  Please "
-                            "specify a non-zero --size for that partition." % \
+            if not self.size and not self.fixed_size:
+                msger.error("The %s partition has a size of zero. Please "
+                            "specify a non-zero --size/--fixed-size for that partition." % \
                             self.mountpoint)
             if self.fstype and self.fstype == "swap":
                 self.prepare_swap_partition(cr_workdir, oe_builddir,
@@ -146,6 +182,7 @@ class Partition():
                                                      oe_builddir,
                                                      bootimg_dir, kernel_dir, rootfs_dir,
                                                      native_sysroot)
+
         # further processing required Partition.size to be an integer, make
         # sure that it is one
         if type(self.size) is not int:
@@ -153,6 +190,12 @@ class Partition():
                           "This a bug in source plugin %s and needs to be fixed." \
                           % (self.mountpoint, self.source))
 
+        if self.fixed_size and self.size > self.fixed_size:
+            msger.error("File system image of partition %s is larger (%d kB) than its"\
+                        "allowed size %d kB" % (self.mountpoint,
+                                                self.size, self.fixed_size))
+
+
     def prepare_rootfs_from_fs_image(self, cr_workdir, oe_builddir,
                                      rootfs_dir):
         """
@@ -217,15 +260,7 @@ class Partition():
         out = exec_cmd(du_cmd)
         actual_rootfs_size = int(out.split()[0])
 
-        extra_blocks = self.get_extra_block_count(actual_rootfs_size)
-        if extra_blocks < self.extra_space:
-            extra_blocks = self.extra_space
-
-        rootfs_size = actual_rootfs_size + extra_blocks
-        rootfs_size *= self.overhead_factor
-
-        msger.debug("Added %d extra blocks to %s to get to %d total blocks" % \
-                    (extra_blocks, self.mountpoint, rootfs_size))
+        rootfs_size = self.get_rootfs_size(actual_rootfs_size)
 
         with open(rootfs, 'w') as sparse:
             os.ftruncate(sparse.fileno(), rootfs_size * 1024)
@@ -251,15 +286,7 @@ class Partition():
         out = exec_cmd(du_cmd)
         actual_rootfs_size = int(out.split()[0])
 
-        extra_blocks = self.get_extra_block_count(actual_rootfs_size)
-        if extra_blocks < self.extra_space:
-            extra_blocks = self.extra_space
-
-        rootfs_size = actual_rootfs_size + extra_blocks
-        rootfs_size *= self.overhead_factor
-
-        msger.debug("Added %d extra blocks to %s to get to %d total blocks" % \
-                    (extra_blocks, self.mountpoint, rootfs_size))
+        rootfs_size = self.get_rootfs_size(actual_rootfs_size)
 
         with open(rootfs, 'w') as sparse:
             os.ftruncate(sparse.fileno(), rootfs_size * 1024)
@@ -281,20 +308,13 @@ class Partition():
         out = exec_cmd(du_cmd)
         blocks = int(out.split()[0])
 
-        extra_blocks = self.get_extra_block_count(blocks)
-        if extra_blocks < self.extra_space:
-            extra_blocks = self.extra_space
-
-        blocks += extra_blocks
-
-        msger.debug("Added %d extra blocks to %s to get to %d total blocks" % \
-                    (extra_blocks, self.mountpoint, blocks))
+        rootfs_size = self.get_rootfs_size(blocks)
 
         label_str = "-n boot"
         if self.label:
             label_str = "-n %s" % self.label
 
-        dosfs_cmd = "mkdosfs %s -S 512 -C %s %d" % (label_str, rootfs, blocks)
+        dosfs_cmd = "mkdosfs %s -S 512 -C %s %d" % (label_str, rootfs, rootfs_size)
         exec_native_cmd(dosfs_cmd, native_sysroot)
 
         mcopy_cmd = "mcopy -i %s -s %s/* ::/" % (rootfs, rootfs_dir)
@@ -317,8 +337,9 @@ class Partition():
         """
         Prepare an empty ext2/3/4 partition.
         """
+        size = self.disk_size
         with open(rootfs, 'w') as sparse:
-            os.ftruncate(sparse.fileno(), self.size * 1024)
+            os.ftruncate(sparse.fileno(), size * 1024)
 
         extra_imagecmd = "-i 8192"
 
@@ -335,8 +356,9 @@ class Partition():
         """
         Prepare an empty btrfs partition.
         """
+        size = self.disk_size
         with open(rootfs, 'w') as sparse:
-            os.ftruncate(sparse.fileno(), self.size * 1024)
+            os.ftruncate(sparse.fileno(), size * 1024)
 
         label_str = ""
         if self.label:
@@ -351,7 +373,7 @@ class Partition():
         """
         Prepare an empty vfat partition.
         """
-        blocks = self.size
+        blocks = self.disk_size
 
         label_str = "-n boot"
         if self.label:
diff --git a/scripts/lib/wic/utils/partitionedfs.py b/scripts/lib/wic/utils/partitionedfs.py
index 9e76487844eebfffc7227d053a65dc9fdab3678b..cfa5f5ce09b764c1c2a9b7a3f7bf7d677a6811c4 100644
--- a/scripts/lib/wic/utils/partitionedfs.py
+++ b/scripts/lib/wic/utils/partitionedfs.py
@@ -209,7 +209,7 @@ class Image():
             msger.debug("Assigned %s to %s%d, sectors range %d-%d size %d "
                         "sectors (%d bytes)." \
                             % (part['mountpoint'], part['disk_name'], part['num'],
-                               part['start'], part['start'] + part['size'] - 1,
+                               part['start'], disk['offset'] - 1,
                                part['size'], part['size'] * self.sector_size))
 
         # Once all the partitions have been layed out, we can calculate the
-- 
2.5.0

[PATCH v2 06/10] oe-selftest: fix handling of test cases without ID in --list-tests-by

[Maciej Borzecki]

Running `oe-selftest --list-tests-by module wic` will produce the
following backtrace:

Traceback (most recent call last):
  File "<snip>/poky/scripts/oe-selftest", line 668, in <module>
    ret = main()
  File "<snip>/poky/scripts/oe-selftest", line 486, in main
    list_testsuite_by(criteria, keyword)
  File "<snip>/poky/scripts/oe-selftest", line 340, in list_testsuite_by
    ts = sorted([ (tc.tcid, tc.tctag, tc.tcname, tc.tcclass, tc.tcmodule) for tc in get_testsuite_by(criteria, keyword) ])
TypeError: unorderable types: int() < NoneType()

The root cause is that a test case does not necessarily have an ID
assigned, hence its value is None. Since Python 3 does not allow
comparison of heterogeneous types, TypeError is raised.

Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
---
 scripts/oe-selftest | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/scripts/oe-selftest b/scripts/oe-selftest
index d9ffd40e8c4caa734cd490d77304cc600cc75b73..c3215ea6592e128d17da550d778272985f5bd1a6 100755
--- a/scripts/oe-selftest
+++ b/scripts/oe-selftest
@@ -336,10 +336,15 @@ def list_testsuite_by(criteria, keyword):
     # Get a testsuite based on 'keyword'
     # criteria: name, class, module, id, tag
     # keyword: a list of tests, classes, modules, ids, tags
-
-    ts = sorted([ (tc.tcid, tc.tctag, tc.tcname, tc.tcclass, tc.tcmodule) for tc in get_testsuite_by(criteria, keyword) ])
-
-    print('%-4s\t%-20s\t%-60s\t%-25s\t%-20s' % ('id', 'tag', 'name', 'class', 'module'))
+    def tc_key(t):
+        if t[0] is None:
+            return  (0,) + t[1:]
+        return t
+    # tcid may be None if no ID was assigned, in which case sorted() will throw
+    # a TypeError as Python 3 does not allow comparison (<,<=,>=,>) of
+    # heterogeneous types, handle this by using a custom key generator
+    ts = sorted([ (tc.tcid, tc.tctag, tc.tcname, tc.tcclass, tc.tcmodule) \
+                  for tc in get_testsuite_by(criteria, keyword) ], key=tc_key)
     print('_' * 150)
     for t in ts:
         if isinstance(t[1], (tuple, list)):
-- 
2.5.0

[PATCH v2 07/10] wic: selftest: avoid COMPATIBLE_HOST issues

[Maciej Borzecki]

wic tests will unconditionally attempt to build syslinux and add
configuration options that may not be compatible with current machine.

Resolve this by consulting TARGET_ARCH first and performing
configuration changes or recipe builds for targets that are known to be
compatible.

Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
---
 meta/lib/oeqa/selftest/wic.py | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/meta/lib/oeqa/selftest/wic.py b/meta/lib/oeqa/selftest/wic.py
index faac11e21643e4c32a83b649b6ae986fead498f1..60e31de5825c84fca21d4dbe946e5cc7af8df511 100644
--- a/meta/lib/oeqa/selftest/wic.py
+++ b/meta/lib/oeqa/selftest/wic.py
@@ -41,15 +41,21 @@ class Wic(oeSelfTest):
 
     def setUpLocal(self):
         """This code is executed before each test method."""
-        self.write_config('IMAGE_FSTYPES += " hddimg"\n'
-                          'MACHINE_FEATURES_append = " efi"\n')
+        targetarch = get_bb_var('TARGET_ARCH', 'core-image-minimal')
+        if 'x86' in targetarch:
+            self.write_config('IMAGE_FSTYPES += " hddimg"\n'
+                              'MACHINE_FEATURES_append = " efi"\n')
 
         # Do this here instead of in setUpClass as the base setUp does some
         # clean up which can result in the native tools built earlier in
         # setUpClass being unavailable.
         if not Wic.image_is_ready:
-            bitbake('syslinux syslinux-native parted-native gptfdisk-native '
-                    'dosfstools-native mtools-native bmap-tools-native')
+            tools = 'parted-native gptfdisk-native ' \
+                    'dosfstools-native mtools-native bmap-tools-native'
+            if 'x86' in targetarch:
+                tools += 'syslinux syslinux-native'
+            bitbake(tools)
+
             bitbake('core-image-minimal')
             Wic.image_is_ready = True
 
-- 
2.5.0

[PATCH v2 08/10] wic: selftest: do not repeat core-image-minimal

[Maciej Borzecki]

Replace repeated core-image-minimal with Wic class field.

Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
---
 meta/lib/oeqa/selftest/wic.py | 111 +++++++++++++++++++++++++++---------------
 1 file changed, 73 insertions(+), 38 deletions(-)

diff --git a/meta/lib/oeqa/selftest/wic.py b/meta/lib/oeqa/selftest/wic.py
index 60e31de5825c84fca21d4dbe946e5cc7af8df511..8b86acb917134b389e38c8215a8dff6c7f91c005 100644
--- a/meta/lib/oeqa/selftest/wic.py
+++ b/meta/lib/oeqa/selftest/wic.py
@@ -39,6 +39,8 @@ class Wic(oeSelfTest):
     resultdir = "/var/tmp/wic/build/"
     image_is_ready = False
 
+    OE_IMAGE = "core-image-minimal"
+
     def setUpLocal(self):
         """This code is executed before each test method."""
         targetarch = get_bb_var('TARGET_ARCH', 'core-image-minimal')
@@ -56,7 +58,7 @@ class Wic(oeSelfTest):
                 tools += 'syslinux syslinux-native'
             bitbake(tools)
 
-            bitbake('core-image-minimal')
+            bitbake(self.OE_IMAGE)
             Wic.image_is_ready = True
 
         rmtree(self.resultdir, ignore_errors=True)
@@ -80,13 +82,13 @@ class Wic(oeSelfTest):
     def test_build_image_name(self):
         """Test wic create directdisk --image-name core-image-minimal"""
         self.assertEqual(0, runCmd("wic create directdisk "
-                                   "--image-name core-image-minimal").status)
+                                   "--image-name %s" % self.OE_IMAGE).status)
         self.assertEqual(1, len(glob(self.resultdir + "directdisk-*.direct")))
 
     @testcase(1212)
     def test_build_artifacts(self):
         """Test wic create directdisk providing all artifacts."""
-        bbvars = dict((var.lower(), get_bb_var(var, 'core-image-minimal')) \
+        bbvars = dict((var.lower(), get_bb_var(var, self.OE_IMAGE)) \
                         for var in ('STAGING_DATADIR', 'DEPLOY_DIR_IMAGE',
                                     'STAGING_DIR_NATIVE', 'IMAGE_ROOTFS'))
         status = runCmd("wic create directdisk "
@@ -101,7 +103,7 @@ class Wic(oeSelfTest):
     def test_gpt_image(self):
         """Test creation of core-image-minimal with gpt table and UUID boot"""
         self.assertEqual(0, runCmd("wic create directdisk-gpt "
-                                   "--image-name core-image-minimal").status)
+                                   "--image-name %s" % self.OE_IMAGE).status)
         self.assertEqual(1, len(glob(self.resultdir + "directdisk-*.direct")))
 
     @testcase(1213)
@@ -134,8 +136,8 @@ class Wic(oeSelfTest):
     def test_compress_gzip(self):
         """Test compressing an image with gzip"""
         self.assertEqual(0, runCmd("wic create directdisk "
-                                   "--image-name core-image-minimal "
-                                   "-c gzip").status)
+                                   "--image-name %s "
+                                   "-c gzip" % self.OE_IMAGE).status)
         self.assertEqual(1, len(glob(self.resultdir + \
                                          "directdisk-*.direct.gz")))
 
@@ -143,8 +145,8 @@ class Wic(oeSelfTest):
     def test_compress_bzip2(self):
         """Test compressing an image with bzip2"""
         self.assertEqual(0, runCmd("wic create directdisk "
-                                   "--image-name core-image-minimal "
-                                   "-c bzip2").status)
+                                   "--image-name %s "
+                                   "-c bzip2" % self.OE_IMAGE).status)
         self.assertEqual(1, len(glob(self.resultdir + \
                                          "directdisk-*.direct.bz2")))
 
@@ -152,8 +154,8 @@ class Wic(oeSelfTest):
     def test_compress_xz(self):
         """Test compressing an image with xz"""
         self.assertEqual(0, runCmd("wic create directdisk "
-                                   "--image-name core-image-minimal "
-                                   "-c xz").status)
+                                   "--image-name %s "
+                                   "-c xz" % self.OE_IMAGE).status)
         self.assertEqual(1, len(glob(self.resultdir + \
                                          "directdisk-*.direct.xz")))
 
@@ -161,24 +163,29 @@ class Wic(oeSelfTest):
     def test_wrong_compressor(self):
         """Test how wic breaks if wrong compressor is provided"""
         self.assertEqual(2, runCmd("wic create directdisk "
-                                   "--image-name core-image-minimal "
-                                   "-c wrong", ignore_status=True).status)
+                                   "--image-name %s "
+                                   "-c wrong" % self.OE_IMAGE,
+                                   ignore_status=True).status)
 
     @testcase(1268)
     def test_rootfs_indirect_recipes(self):
         """Test usage of rootfs plugin with rootfs recipes"""
         wks = "directdisk-multi-rootfs"
-        self.assertEqual(0, runCmd("wic create %s "
-                                   "--image-name core-image-minimal "
-                                   "--rootfs rootfs1=core-image-minimal "
-                                   "--rootfs rootfs2=core-image-minimal" \
-                                   % wks).status)
+        wic_cmd_vars = {
+            'wks': wks,
+            'image': self.OE_IMAGE,
+        }
+        self.assertEqual(0, runCmd("wic create %(wks)s "
+                                   "--image-name %(image)s "
+                                   "--rootfs rootfs1=%(image)s "
+                                   "--rootfs rootfs2=%(image)s" \
+                                   % wic_cmd_vars).status)
         self.assertEqual(1, len(glob(self.resultdir + "%s*.direct" % wks)))
 
     @testcase(1269)
     def test_rootfs_artifacts(self):
         """Test usage of rootfs plugin with rootfs paths"""
-        bbvars = dict((var.lower(), get_bb_var(var, 'core-image-minimal')) \
+        bbvars = dict((var.lower(), get_bb_var(var, self.OE_IMAGE)) \
                         for var in ('STAGING_DATADIR', 'DEPLOY_DIR_IMAGE',
                                     'STAGING_DIR_NATIVE', 'IMAGE_ROOTFS'))
         bbvars['wks'] = "directdisk-multi-rootfs"
@@ -197,24 +204,23 @@ class Wic(oeSelfTest):
     def test_iso_image(self):
         """Test creation of hybrid iso image with legacy and EFI boot"""
         self.assertEqual(0, runCmd("wic create mkhybridiso "
-                                   "--image-name core-image-minimal").status)
+                                   "--image-name %s" % self.OE_IMAGE).status)
         self.assertEqual(1, len(glob(self.resultdir + "HYBRID_ISO_IMG-*.direct")))
         self.assertEqual(1, len(glob(self.resultdir + "HYBRID_ISO_IMG-*.iso")))
 
     @testcase(1347)
     def test_image_env(self):
         """Test generation of <image>.env files."""
-        image = 'core-image-minimal'
-        self.assertEqual(0, bitbake('%s -c do_rootfs_wicenv' % image).status)
-        stdir = get_bb_var('STAGING_DIR_TARGET', image)
+        self.assertEqual(0, bitbake('%s -c do_rootfs_wicenv' % self.OE_IMAGE).status)
+        stdir = get_bb_var('STAGING_DIR_TARGET', self.OE_IMAGE)
         imgdatadir = os.path.join(stdir, 'imgdata')
 
-        basename = get_bb_var('IMAGE_BASENAME', image)
-        self.assertEqual(basename, image)
+        basename = get_bb_var('IMAGE_BASENAME', self.OE_IMAGE)
+        self.assertEqual(basename, self.OE_IMAGE)
         path = os.path.join(imgdatadir, basename) + '.env'
         self.assertTrue(os.path.isfile(path))
 
-        wicvars = set(get_bb_var('WICVARS', image).split())
+        wicvars = set(get_bb_var('WICVARS', self.OE_IMAGE).split())
         # filter out optional variables
         wicvars = wicvars.difference(('HDDDIR', 'IMAGE_BOOT_FILES',
                                       'INITRD', 'ISODIR'))
@@ -244,32 +250,48 @@ class Wic(oeSelfTest):
     def test_qemux86_directdisk(self):
         """Test creation of qemux-86-directdisk image"""
         image = "qemux86-directdisk"
-        self.assertEqual(0, runCmd("wic create %s -e core-image-minimal" \
-                                   % image).status)
+        wic_cmd_vars = {
+            'wks': image,
+            'image': self.OE_IMAGE,
+        }
+        self.assertEqual(0, runCmd("wic create %(wks)s -e %(image)s" \
+                                   % wic_cmd_vars).status)
         self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
 
     @testcase(1349)
     def test_mkgummidisk(self):
         """Test creation of mkgummidisk image"""
         image = "mkgummidisk"
-        self.assertEqual(0, runCmd("wic create %s -e core-image-minimal" \
-                                   % image).status)
+        wic_cmd_vars = {
+            'wks': image,
+            'image': self.OE_IMAGE,
+        }
+        self.assertEqual(0, runCmd("wic create %(wks)s -e %(image)s" \
+                                   % wic_cmd_vars).status)
         self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
 
     @testcase(1350)
     def test_mkefidisk(self):
         """Test creation of mkefidisk image"""
         image = "mkefidisk"
-        self.assertEqual(0, runCmd("wic create %s -e core-image-minimal" \
-                                   % image).status)
+        wic_cmd_vars = {
+            'wks': image,
+            'image': self.OE_IMAGE,
+        }
+        self.assertEqual(0, runCmd("wic create %(wks)s -e %(image)s" \
+                                   % wic_cmd_vars).status)
         self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
 
     @testcase(1385)
     def test_directdisk_bootloader_config(self):
         """Test creation of directdisk-bootloader-config image"""
         image = "directdisk-bootloader-config"
-        self.assertEqual(0, runCmd("wic create %s -e core-image-minimal" \
-                                   % image).status)
+        wic_cmd_vars = {
+            'wks': image,
+            'image': self.OE_IMAGE,
+        }
+        self.assertEqual(0, runCmd("wic create %(wks)s -e %(image)s" \
+                                   % wic_cmd_vars).status)
         self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
 
     @testcase(1422)
@@ -286,7 +308,12 @@ class Wic(oeSelfTest):
     def test_bmap(self):
         """Test generation of .bmap file"""
         image = "directdisk"
-        status = runCmd("wic create %s -e core-image-minimal --bmap" % image).status
+        wic_cmd_vars = {
+            'wks': image,
+            'image': self.OE_IMAGE,
+        }
+        status = runCmd("wic create %(wks)s -e %(image)s --bmap" \
+                        % wic_cmd_vars).status
         self.assertEqual(0, status)
         self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
         self.assertEqual(1, len(glob(self.resultdir + "%s-*direct.bmap" % image)))
@@ -294,14 +321,22 @@ class Wic(oeSelfTest):
     def test_systemd_bootdisk(self):
         """Test creation of systemd-bootdisk image"""
         image = "systemd-bootdisk"
-        self.assertEqual(0, runCmd("wic create %s -e core-image-minimal" \
-                                   % image).status)
+        wic_cmd_vars = {
+            'wks': image,
+            'image': self.OE_IMAGE,
+        }
+        self.assertEqual(0, runCmd("wic create %(wic)s -e %(image)s" \
+                                   % wic_cmd_vars).status)
         self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
 
     def test_sdimage_bootpart(self):
         """Test creation of sdimage-bootpart image"""
         image = "sdimage-bootpart"
         self.write_config('IMAGE_BOOT_FILES = "bzImage"\n')
-        self.assertEqual(0, runCmd("wic create %s -e core-image-minimal" \
-                                   % image).status)
+        wic_cmd_vars = {
+            'wks': image,
+            'image': self.OE_IMAGE,
+        }
+        self.assertEqual(0, runCmd("wic create %(wks)s -e %(image)s" \
+                                   % wic_cmd_vars).status)
         self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
-- 
2.5.0

[PATCH v2 09/10] wic: selftest: do not assume bzImage kernel image

[Maciej Borzecki]

Instead of assuming that bzImage is available, query bitbake enviroment
for KERNEL_IMAGETYPE.

Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
---
 meta/lib/oeqa/selftest/wic.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/meta/lib/oeqa/selftest/wic.py b/meta/lib/oeqa/selftest/wic.py
index 8b86acb917134b389e38c8215a8dff6c7f91c005..799886c164a734bcc3226baf97f20d789ae55b56 100644
--- a/meta/lib/oeqa/selftest/wic.py
+++ b/meta/lib/oeqa/selftest/wic.py
@@ -332,7 +332,8 @@ class Wic(oeSelfTest):
     def test_sdimage_bootpart(self):
         """Test creation of sdimage-bootpart image"""
         image = "sdimage-bootpart"
-        self.write_config('IMAGE_BOOT_FILES = "bzImage"\n')
+        kimgtype = get_bb_var('KERNEL_IMAGETYPE', self.OE_IMAGE)
+        self.write_config('IMAGE_BOOT_FILES = "%s"\n' % kimgtype)
         wic_cmd_vars = {
             'wks': image,
             'image': self.OE_IMAGE,
-- 
2.5.0

[PATCH v2 10/10] wic: selftest: add tests for --fixed-size partition flags

[Maciej Borzecki]

wic has a new flag for setting a fixed parition size --fixed-size. Add
tests that verify if partition is indeed sized properly and that errors
are signaled when there is not enough space to fit partition data.

Signed-off-by: Maciej Borzecki <maciej.borzecki@rndity.com>
---
 meta/lib/oeqa/selftest/wic.py | 65 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/meta/lib/oeqa/selftest/wic.py b/meta/lib/oeqa/selftest/wic.py
index 799886c164a734bcc3226baf97f20d789ae55b56..954f8911eff6da6880d25b4fcd2d82fe0a26c4ba 100644
--- a/meta/lib/oeqa/selftest/wic.py
+++ b/meta/lib/oeqa/selftest/wic.py
@@ -27,6 +27,7 @@ import os
 
 from glob import glob
 from shutil import rmtree
+from tempfile import NamedTemporaryFile
 
 from oeqa.selftest.base import oeSelfTest
 from oeqa.utils.commands import runCmd, bitbake, get_bb_var, runqemu
@@ -341,3 +342,67 @@ class Wic(oeSelfTest):
         self.assertEqual(0, runCmd("wic create %(wks)s -e %(image)s" \
                                    % wic_cmd_vars).status)
         self.assertEqual(1, len(glob(self.resultdir + "%s-*direct" % image)))
+
+    def _make_fixed_size_wks(self, size):
+        """
+        Create a wks of an image with a single partition. Size of the partition is set
+        using --fixed-size flag. Returns a tuple: (path to wks file, wks image name)
+        """
+        with NamedTemporaryFile("w", suffix=".wks", delete=False) as tf:
+            wkspath = tf.name
+            tf.write("part " \
+                     "--source rootfs --ondisk hda --align 4 --fixed-size %d "
+                     "--fstype=ext4\n" % size)
+        wksname = os.path.splitext(os.path.basename(wkspath))[0]
+
+        return (wkspath, wksname)
+
+    def test_fixed_size(self):
+        """
+        Test creation of a simple image with partition size controlled through
+        --fixed-size flag
+        """
+        wkspath, wksname = self._make_fixed_size_wks(200)
+
+        wic_cmd_vars = {
+            'wks': wkspath,
+            'image': self.OE_IMAGE,
+        }
+        self.assertEqual(0, runCmd("wic create %(wks)s -e %(image)s" \
+                                   % wic_cmd_vars).status)
+        os.remove(wkspath)
+        wicout = glob(self.resultdir + "%s-*direct" % wksname)
+        self.assertEqual(1, len(wicout))
+
+        wicimg = wicout[0]
+
+        # verify partition size with wic
+        res = runCmd("parted -m %s unit mib p 2>/dev/null" % wicimg, ignore_status=True)
+        self.assertEqual(0, res.status)
+
+        # parse parted output which looks like this:
+        # BYT;\n
+        # /var/tmp/wic/build/tmpfwvjjkf_-201611101222-hda.direct:200MiB:file:512:512:msdos::;\n
+        # 1:0,00MiB:200MiB:200MiB:ext4::;\n
+        partlns = res.output.splitlines()[2:]
+
+        self.assertEqual(1, len(partlns))
+        self.assertEqual("1:0,00MiB:200MiB:200MiB:ext4::;", partlns[0])
+
+    def test_fixed_size_error(self):
+        """
+        Test creation of a simple image with partition size controlled through
+        --fixed-size flag. The size of partition is intentionally set to 1MiB
+        in order to trigger an error in wic.
+        """
+        wkspath, wksname = self._make_fixed_size_wks(1)
+
+        wic_cmd_vars = {
+            'wks': wkspath,
+            'image': self.OE_IMAGE,
+        }
+        self.assertEqual(1, runCmd("wic create %(wks)s -e %(image)s" \
+                                   % wic_cmd_vars, ignore_status=True).status)
+        os.remove(wkspath)
+        wicout = glob(self.resultdir + "%s-*direct" % wksname)
+        self.assertEqual(0, len(wicout))
-- 
2.5.0

Re: [PATCH v2 06/10] oe-selftest: fix handling of test cases without ID in --list-tests-by

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 1167 bytes --]

On 10 November 2016 at 12:18, Maciej Borzecki <maciej.borzecki@rndity.com>
wrote:

> -    ts = sorted([ (tc.tcid, tc.tctag, tc.tcname, tc.tcclass, tc.tcmodule)
> for tc in get_testsuite_by(criteria, keyword) ])
> -
> -    print('%-4s\t%-20s\t%-60s\t%-25s\t%-20s' % ('id', 'tag', 'name',
> 'class', 'module'))
> +    def tc_key(t):
> +        if t[0] is None:
> +            return  (0,) + t[1:]
> +        return t
> +    # tcid may be None if no ID was assigned, in which case sorted() will
> throw
> +    # a TypeError as Python 3 does not allow comparison (<,<=,>=,>) of
> +    # heterogeneous types, handle this by using a custom key generator
> +    ts = sorted([ (tc.tcid, tc.tctag, tc.tcname, tc.tcclass, tc.tcmodule)
> \
> +                  for tc in get_testsuite_by(criteria, keyword) ],
> key=tc_key)
>

Wouldn't a shorter (but untested!) form alternative be just?

-    ts = sorted([ (tc.tcid, tc.tctag, tc.tcname, tc.tcclass, tc.tcmodule)
for tc in get_testsuite_by(criteria, keyword) ])
+    ts = sorted([ (tc.tcid or 0, tc.tctag, tc.tcname, tc.tcclass,
tc.tcmodule) for tc in get_testsuite_by(criteria, keyword) ])

Ross

[-- Attachment #2: Type: text/html, Size: 2161 bytes --]

Re: [PATCH v2 06/10] oe-selftest: fix handling of test cases without ID in --list-tests-by

[Maciej Borzęcki]

On Thu, Nov 10, 2016 at 9:05 PM, Burton, Ross <ross.burton@intel.com> wrote:
>
> On 10 November 2016 at 12:18, Maciej Borzecki <maciej.borzecki@rndity.com>
> wrote:
>>
>> -    ts = sorted([ (tc.tcid, tc.tctag, tc.tcname, tc.tcclass, tc.tcmodule)
>> for tc in get_testsuite_by(criteria, keyword) ])
>> -
>> -    print('%-4s\t%-20s\t%-60s\t%-25s\t%-20s' % ('id', 'tag', 'name',
>> 'class', 'module'))
>> +    def tc_key(t):
>> +        if t[0] is None:
>> +            return  (0,) + t[1:]
>> +        return t
>> +    # tcid may be None if no ID was assigned, in which case sorted() will
>> throw
>> +    # a TypeError as Python 3 does not allow comparison (<,<=,>=,>) of
>> +    # heterogeneous types, handle this by using a custom key generator
>> +    ts = sorted([ (tc.tcid, tc.tctag, tc.tcname, tc.tcclass, tc.tcmodule)
>> \
>> +                  for tc in get_testsuite_by(criteria, keyword) ],
>> key=tc_key)
>
>
> Wouldn't a shorter (but untested!) form alternative be just?
>
> -    ts = sorted([ (tc.tcid, tc.tctag, tc.tcname, tc.tcclass, tc.tcmodule)
> for tc in get_testsuite_by(criteria, keyword) ])
> +    ts = sorted([ (tc.tcid or 0, tc.tctag, tc.tcname, tc.tcclass,
> tc.tcmodule) for tc in get_testsuite_by(criteria, keyword) ])
>

Well, yes, that would work too. A downside though, is that the output of
--list-tests and --list-tests would be different. For --list-test, TCs without
ID would have None in the first column, while the same TCs in --lists-tests-by
would end up having 0.

I've tried to keep the output of both tools consistent. Also, 'None' seems to be
more a better fit in this case, as there's no ID.

Cheers,
-- 
Maciej Borzecki
RnDity

Re: [PATCH v2 08/10] wic: selftest: do not repeat core-image-minimal

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 964 bytes --]

On 10 November 2016 at 12:18, Maciej Borzecki <maciej.borzecki@rndity.com>
wrote:

>      def test_qemux86_directdisk(self):
>          """Test creation of qemux-86-directdisk image"""
>          image = "qemux86-directdisk"
> -        self.assertEqual(0, runCmd("wic create %s -e core-image-minimal" \
> -                                   % image).status)
> +        wic_cmd_vars = {
> +            'wks': image,
> +            'image': self.OE_IMAGE,
> +        }
> +        self.assertEqual(0, runCmd("wic create %(wks)s -e %(image)s" \
> +                                   % wic_cmd_vars).status)
>

Is it just me who thinks that this is more understandable then?

def test_qemux86_directdisk(self):
  self.assertEqual(0, runCmd("wic create qemux86-directdisk -e
core-image-minimal").status)
  self.assertEqual(1, len(glob(os.path.join(self.resultdir,
"core-image-minimal-*direct"))))

Not my test case so just an opinion :)

Ross

[-- Attachment #2: Type: text/html, Size: 1836 bytes --]

Re: [PATCH v2 08/10] wic: selftest: do not repeat core-image-minimal

[Maciej Borzęcki]

On Thu, Nov 10, 2016 at 9:15 PM, Burton, Ross <ross.burton@intel.com> wrote:
>
> On 10 November 2016 at 12:18, Maciej Borzecki <maciej.borzecki@rndity.com>
> wrote:
>>
>>      def test_qemux86_directdisk(self):
>>          """Test creation of qemux-86-directdisk image"""
>>          image = "qemux86-directdisk"
>> -        self.assertEqual(0, runCmd("wic create %s -e core-image-minimal"
>> \
>> -                                   % image).status)
>> +        wic_cmd_vars = {
>> +            'wks': image,
>> +            'image': self.OE_IMAGE,
>> +        }
>> +        self.assertEqual(0, runCmd("wic create %(wks)s -e %(image)s" \
>> +                                   % wic_cmd_vars).status)
>
>
> Is it just me who thinks that this is more understandable then?
>
> def test_qemux86_directdisk(self):
>   self.assertEqual(0, runCmd("wic create qemux86-directdisk -e
> core-image-minimal").status)
>   self.assertEqual(1, len(glob(os.path.join(self.resultdir,
> "core-image-minimal-*direct"))))
>
> Not my test case so just an opinion :)
>

Now that I look at this patch, I probably might have overdone it a little. Let's
wait and see what Ed thinks about this change. I suppose there might be a need
for v3 of this series, in which case I'll just drop this patch and rebase the
remaining patches.

Cheers,
-- 
Maciej Borzecki
RnDity

Re: [PATCH v2 00/10] wic: bugfixes & --fixed-size support, tests, selftest: minor fixes

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 1163 bytes --]

On 10 November 2016 at 12:18, Maciej Borzecki <maciej.borzecki@rndity.com>
wrote:

> v2 of patch series previously posted here [1].
>
> The series introduces anumber of fixes to wic, as well as a new
> --fixed-size
> option applicable to `part` entries in kickstart files. The option makes it
> possible to have a fixed size partition, with additional verification that
> the
> file system image fits into the allocated disk space. This is in cotrast
> with
> --size option, which the minimum size, and partition may in fact be larger
> in
> the final disk image.
>
> The series introduces two tests that verify if wic has created the image
> with
> properly sized partition and that partition size checks work as expected.
>
> Patch `oe-selftest: fix handling of test cases without ID in
> --list-tests-by` is
> a small bugfix for oe-selftes tool which resolves an issue triggered by
> Python
> 3.x being more strict than 2.x.
>

This series appears to be causing selftest to fail on the autobuilders:

https://autobuilder.yoctoproject.org/main/builders/nightly-oe-selftest/builds/732/steps/Running%20oe-selftest/logs/stdio

Ross

[-- Attachment #2: Type: text/html, Size: 1926 bytes --]

Re: [PATCH v2 00/10] wic: bugfixes & --fixed-size support, tests, selftest: minor fixes

[Maciej Borzęcki]

On Mon, Nov 14, 2016 at 10:24 AM, Burton, Ross <ross.burton@intel.com> wrote:
>
> On 10 November 2016 at 12:18, Maciej Borzecki <maciej.borzecki@rndity.com>
> wrote:
>>
>> v2 of patch series previously posted here [1].
>>
>> The series introduces anumber of fixes to wic, as well as a new
>> --fixed-size
>> option applicable to `part` entries in kickstart files. The option makes
>> it
>> possible to have a fixed size partition, with additional verification that
>> the
>> file system image fits into the allocated disk space. This is in cotrast
>> with
>> --size option, which the minimum size, and partition may in fact be larger
>> in
>> the final disk image.
>>
>> The series introduces two tests that verify if wic has created the image
>> with
>> properly sized partition and that partition size checks work as expected.
>>
>> Patch `oe-selftest: fix handling of test cases without ID in
>> --list-tests-by` is
>> a small bugfix for oe-selftes tool which resolves an issue triggered by
>> Python
>> 3.x being more strict than 2.x.
>
>
> This series appears to be causing selftest to fail on the autobuilders:
>
> https://autobuilder.yoctoproject.org/main/builders/nightly-oe-selftest/builds/732/steps/Running%20oe-selftest/logs/stdio

I see that the builds were running with qemux85-64 MACHINE, shouldn't
TARGET_ARCH be set to x86-64 then? There's a check that builds
syslinux(-native) for x86 archs only since it's not buildable for
anything else. Same for HDDIMG.

One more question, the assert raised when comparing parted's output is
caused by different locale. Do you think it would be ok to just enforce
LC_ALL=C when running commands under wic?

-- 
Maciej Borzecki
RnDity

Re: [PATCH v2 00/10] wic: bugfixes & --fixed-size support, tests, selftest: minor fixes

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 970 bytes --]

On 14 November 2016 at 10:02, Maciej Borzęcki <maciej.borzecki@rndity.com>
wrote:

> I see that the builds were running with qemux85-64 MACHINE, shouldn't
> TARGET_ARCH be set to x86-64 then? There's a check that builds
> syslinux(-native) for x86 archs only since it's not buildable for
> anything else. Same for HDDIMG.
>

Not quite:

$ MACHINE=qemux86 bitbake core-image-minimal -e | grep ^TARGET_ARCH=
TARGET_ARCH="i586"

$ MACHINE=qemux86-64 bitbake core-image-minimal -e | grep ^TARGET_ARCH=
TARGET_ARCH="x86_64"

(had to patch bitbake.conf to export TARGET_ARCH)


> One more question, the assert raised when comparing parted's output is
> caused by different locale. Do you think it would be ok to just enforce
> LC_ALL=C when running commands under wic?
>

Yes, enforcing a  locale is essential when comparing output of commands.
Bitbake switches to en_US.UTF8 on startup for the same reason, so feel free
to use that or C.

Ross

[-- Attachment #2: Type: text/html, Size: 1887 bytes --]

Re: [PATCH v2 00/10] wic: bugfixes & --fixed-size support, tests, selftest: minor fixes

[Maciej Borzęcki]

On Mon, Nov 14, 2016 at 11:16 AM, Burton, Ross <ross.burton@intel.com> wrote:
>
> On 14 November 2016 at 10:02, Maciej Borzęcki <maciej.borzecki@rndity.com>
> wrote:
>>
>> I see that the builds were running with qemux85-64 MACHINE, shouldn't
>> TARGET_ARCH be set to x86-64 then? There's a check that builds
>> syslinux(-native) for x86 archs only since it's not buildable for
>> anything else. Same for HDDIMG.
>
>
> Not quite:
>
> $ MACHINE=qemux86 bitbake core-image-minimal -e | grep ^TARGET_ARCH=
> TARGET_ARCH="i586"
>
> $ MACHINE=qemux86-64 bitbake core-image-minimal -e | grep ^TARGET_ARCH=
> TARGET_ARCH="x86_64"
>
> (had to patch bitbake.conf to export TARGET_ARCH)

Do you think it makes sense to replace TARGET_ARCH check with something
like this instead:

        arch = get_bb_var('HOST_ARCH', 'core-image-minimal')

        is_x86 = arch in ['i586', 'i686', 'x86-64']
        if is_x86:
            self.write_config('IMAGE_FSTYPES += " hddimg"\n'
                              'MACHINE_FEATURES_append = " efi"\n')

Basically, I do not want to build syslinux or set config options that
are usable on x86 target when building for other targets.


>
>>
>> One more question, the assert raised when comparing parted's output is
>> caused by different locale. Do you think it would be ok to just enforce
>> LC_ALL=C when running commands under wic?
>
>
> Yes, enforcing a  locale is essential when comparing output of commands.
> Bitbake switches to en_US.UTF8 on startup for the same reason, so feel free
> to use that or C.

I suppose I'll just enforce that in oe-selftest.



-- 
Maciej Borzecki
RnDity

Re: [PATCH v2 08/10] wic: selftest: do not repeat core-image-minimal

[Maciej Borzęcki]

On Thu, Nov 10, 2016 at 9:33 PM, Maciej Borzęcki
<maciej.borzecki@rndity.com> wrote:
> On Thu, Nov 10, 2016 at 9:15 PM, Burton, Ross <ross.burton@intel.com> wrote:
>>
>> On 10 November 2016 at 12:18, Maciej Borzecki <maciej.borzecki@rndity.com>
>> wrote:
>>>
>>>      def test_qemux86_directdisk(self):
>>>          """Test creation of qemux-86-directdisk image"""
>>>          image = "qemux86-directdisk"
>>> -        self.assertEqual(0, runCmd("wic create %s -e core-image-minimal"
>>> \
>>> -                                   % image).status)
>>> +        wic_cmd_vars = {
>>> +            'wks': image,
>>> +            'image': self.OE_IMAGE,
>>> +        }
>>> +        self.assertEqual(0, runCmd("wic create %(wks)s -e %(image)s" \
>>> +                                   % wic_cmd_vars).status)
>>
>>
>> Is it just me who thinks that this is more understandable then?
>>
>> def test_qemux86_directdisk(self):
>>   self.assertEqual(0, runCmd("wic create qemux86-directdisk -e
>> core-image-minimal").status)
>>   self.assertEqual(1, len(glob(os.path.join(self.resultdir,
>> "core-image-minimal-*direct"))))
>>
>> Not my test case so just an opinion :)
>>
>
> Now that I look at this patch, I probably might have overdone it a little. Let's
> wait and see what Ed thinks about this change. I suppose there might be a need
> for v3 of this series, in which case I'll just drop this patch and rebase the
> remaining patches.

Ed, did you get a chance to review this series?

Cheers,
-- 
Maciej Borzecki
RnDity

Re: [PATCH v2 00/10] wic: bugfixes & --fixed-size support, tests, selftest: minor fixes

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 716 bytes --]

On 14 November 2016 at 10:35, Maciej Borzęcki <maciej.borzecki@rndity.com>
wrote:

> Do you think it makes sense to replace TARGET_ARCH check with something
> like this instead:
>
>         arch = get_bb_var('HOST_ARCH', 'core-image-minimal')
>
>         is_x86 = arch in ['i586', 'i686', 'x86-64']
>         if is_x86:
>             self.write_config('IMAGE_FSTYPES += " hddimg"\n'
>                               'MACHINE_FEATURES_append = " efi"\n')
>
> Basically, I do not want to build syslinux or set config options that
> are usable on x86 target when building for other targets.
>

That expression is more likely to work, yeah, though woudn't TARGET_ be
more suitable than HOST_?

Ross

[-- Attachment #2: Type: text/html, Size: 1266 bytes --]

Re: [PATCH v2 00/10] wic: bugfixes & --fixed-size support, tests, selftest: minor fixes

[Maciej Borzęcki]

On Mon, Nov 14, 2016 at 2:34 PM, Burton, Ross <ross.burton@intel.com> wrote:
>
> On 14 November 2016 at 10:35, Maciej Borzęcki <maciej.borzecki@rndity.com>
> wrote:
>>
>> Do you think it makes sense to replace TARGET_ARCH check with something
>> like this instead:
>>
>>         arch = get_bb_var('HOST_ARCH', 'core-image-minimal')
>>
>>         is_x86 = arch in ['i586', 'i686', 'x86-64']
>>         if is_x86:
>>             self.write_config('IMAGE_FSTYPES += " hddimg"\n'
>>                               'MACHINE_FEATURES_append = " efi"\n')
>>
>> Basically, I do not want to build syslinux or set config options that
>> are usable on x86 target when building for other targets.
>
>
> That expression is more likely to work, yeah, though woudn't TARGET_ be more
> suitable than HOST_?

TARGET_ARCH is in fact not exported as you have indicated, there's this
piece in meta/conf/bitbake.conf:

  # Make sure TARGET_ARCH isn't exported
  # (breaks Makefiles using implicit rules, e.g. quilt, as GNU make has this
  # in them, undocumented)
  TARGET_ARCH[unexport] = "1"

HOST_ARCH defaults to TARGET_ARCH, I suppose it should be fine to use:

  HOST_ARCH = "${TARGET_ARCH}"

Cheers,
-- 
Maciej Borzecki
RnDity

Re: [PATCH v2 00/10] wic: bugfixes & --fixed-size support, tests, selftest: minor fixes

[Burton, Ross]

[-- Attachment #1: Type: text/plain, Size: 575 bytes --]

On 14 November 2016 at 14:03, Maciej Borzęcki <maciej.borzecki@rndity.com>
wrote:

> TARGET_ARCH is in fact not exported as you have indicated, there's this
> piece in meta/conf/bitbake.conf:
>

Ah yes there's a bit of bad interaction between unexport variables, the
hack that is -e, and the even greater hack that is get_bb_var.

I actually have a patch to change the -e output, I've just noticed that it
will break selftest as that makes assumptions about -e output, but it does
solve the problem that you can't reliably use get_bb_var() on unexport.

Ross

[-- Attachment #2: Type: text/html, Size: 1064 bytes --]

Re: [PATCH v2 00/10] wic: bugfixes & --fixed-size support, tests, selftest: minor fixes

[Maciej Borzęcki]

On Mon, Nov 14, 2016 at 3:07 PM, Burton, Ross <ross.burton@intel.com> wrote:
>
> On 14 November 2016 at 14:03, Maciej Borzęcki <maciej.borzecki@rndity.com>
> wrote:
>>
>> TARGET_ARCH is in fact not exported as you have indicated, there's this
>> piece in meta/conf/bitbake.conf:
>
>
> Ah yes there's a bit of bad interaction between unexport variables, the hack
> that is -e, and the even greater hack that is get_bb_var.
>
> I actually have a patch to change the -e output, I've just noticed that it
> will break selftest as that makes assumptions about -e output, but it does
> solve the problem that you can't reliably use get_bb_var() on unexport.

I suppose that settles it then. I will stick with HOST_ARCH in v3 of the series.


-- 
Maciej Borzecki
RnDity