Merge lp:~mabac/linaro-image-tools/hwpacks-v2-lmc into lp:linaro-image-tools/11.11

[Mattias Backman]

Hi,

This is the second part of hwpacks v2 format support. It adds the new fields present in hwpack-create to linaro-media-create.

Reading the metadata file and other data from the hwpack tarball is handled by HardwarepackHandler which treats the metadata file as a ConfigParser file (by adding a bogus header to it). I'm thinking that when we do one hwpack per SoC we might use a [main] header and [boardX51] like headers to set board specific information.

The metadata fields are used in BoardConfig.set_metadata(hwpack_file) which sets most fields straight to BoardConfig class fields. Special cases are partition_layout and u-boot.

Partition_layout now decides whether to use fat32 or fat16, still the same partition layout but this will support adding a special partition for the Snowball later.

The u-boot file is unpacked from the tarball into a temp folder if the format is something else than V1 and is available for copying to the boot disc or dd:ing as some boards do. The concatenation of paths to find the file is there as a fallback for the V1 format.

Multiple hwpacks can be passed and l-m-c will bail if any field is set in more than one hwpack, with the exception for the standard ones such as name, architecture, supported etc.

It is tested on my Panda by creating a hwpack using lp:~mabac/+junk/hwpack.natty.linaro-panda and then writing that to the Panda with l-m-c. The board boots and looks happy. ;)

I have yet to find out where these fields goes, they are currently set but unused:
 - mmc_id
 - wired_interfaces
 - wireless_interfaces

Thanks,

Mattias

[Mattias Backman (mabac)]

+ board_config.set_metadata(args.hwpacks[0])

This is a bit odd, yes. If we pass multiple hwpacks, I'm not sure which metadata should be used.

[James Westby (james-w)]

Hi,

This is pretty small, nice work.

I wonder if using ConfigParser to read the metadata file will do something
odd to the data in some circumstances? Perhaps we should turn off all its
magic?

As for multiple hwpacks, I think it should set with data from all of them
eventually, only using the data where defined, and erroring on overlaps. For
now I think just finding the hwpack with the metadata that we want is good
enough (only one will include a kernel.)

Thanks,

James

[Mattias Backman (mabac)]

> Hi,
>
> This is pretty small, nice work.
>
> I wonder if using ConfigParser to read the metadata file will do something
> odd to the data in some circumstances? Perhaps we should turn off all its
> magic?

I don't know what magic changes it might do to the data. Is there some type of common conversions that it does? If there's any risk of strange bugs in the future, it might be better to simply tokenize the metadata file at '=' or something.

>
> As for multiple hwpacks, I think it should set with data from all of them
> eventually, only using the data where defined, and erroring on overlaps. For
> now I think just finding the hwpack with the metadata that we want is good
> enough (only one will include a kernel.)

Then I'll go with setting the data from all hwpacks and fail if the same field is set more than once.

[James Westby (james-w)]

56 + "section. Valid partition layouts are bootfs16_rootfs, " \
57 + "bootfs_rootfs.", config)

Could we make that exception text use the defined_partition_layouts so that it
is never out of date?

160 + assert format == format_string, "Mixing hwpack formats is " \
161 + "not supported!"

Is there a particular reason for that?

This looks good to me though.

Thanks,

James

review: Approve

[Mattias Backman (mabac)]

On Tue, Jun 28, 2011 at 6:14 PM, James Westby <email address hidden> wrote:
> Review: Approve
> 56      +                "section. Valid partition layouts are bootfs16_rootfs, " \
> 57      +                "bootfs_rootfs.", config)
>
> Could we make that exception text use the defined_partition_layouts so that it
> is never out of date?

Sure, I'll see to that.

>
>
> 160     +                assert format == format_string, "Mixing hwpack formats is " \
> 161     +                    "not supported!"
>
>
> Is there a particular reason for that?

Actually no, and just removing the assertion would work fine, I think.
I was mostly fishing for any opinion on this.

>
> This looks good to me though.

Good. Adding the support for multiple hardwarepacks made the change
quite a lot bigger and a bit ugly. But I guess there will be some
opportunity to tweak that part a bit when I move along to the
remaining bits.

>
> Thanks,
>
> James
>
> --
> https://code.launchpad.net/~mabac/linaro-image-tools/hwpacks-v2-lmc/+merge/65669
> You are the owner of lp:~mabac/linaro-image-tools/hwpacks-v2-lmc.
>

[James Westby (james-w)]

On Tue, 28 Jun 2011 18:37:23 -0000, Mattias Backman <email address hidden> wrote:
> Actually no, and just removing the assertion would work fine, I think.
> I was mostly fishing for any opinion on this.

I can't see any reason to exclude a 1.0 hwpack being used with a 2.0
hwpack, provided the code doesn't complain about that due to the
defaults.

I've no idea about future formats, but we can exclude those combinations
when we add support for them.

Therefore I don't think there's any need for this assertion.

Thanks,

James

[Mattias Backman (mabac)]

> On Tue, 28 Jun 2011 18:37:23 -0000, Mattias Backman
> <email address hidden> wrote:
> > Actually no, and just removing the assertion would work fine, I think.
> > I was mostly fishing for any opinion on this.
>
> I can't see any reason to exclude a 1.0 hwpack being used with a 2.0
> hwpack, provided the code doesn't complain about that due to the
> defaults.
>
> I've no idea about future formats, but we can exclude those combinations
> when we add support for them.
>
> Therefore I don't think there's any need for this assertion.

Ok. With the current code, which assumes that the defaults are not to be used for 2.0 hwpacks, the 2.0 hwpack would simply take precedence. However if the 2.0 metadata is not there (perhaps allowed for a pure x11 hwpack?) there would be a problem since I just wipe the defaults if I see a 2.0 hwpack.

Do you think I should allow for the 1.0 defaults to trickle in when in mixed 1.0/2.0 mode, or just decide that in mixed mode, the 2.0 metadata has to be there?

[lp:~mabac/linaro-image-tools/hwpacks-v2-lmc]

376. By Mattias Backman on 2011-06-29

Change test to use variable for matching text.

377. By Mattias Backman on 2011-06-29

Allow mixing hwpack formats and consider metadata format to be the same as the highest format version encountered.

[James Westby (james-w)]

On Wed, 29 Jun 2011 07:35:50 -0000, Mattias Backman <email address hidden> wrote:
> Ok. With the current code, which assumes that the defaults are not to
> be used for 2.0 hwpacks, the 2.0 hwpack would simply take
> precedence. However if the 2.0 metadata is not there (perhaps allowed
> for a pure x11 hwpack?) there would be a problem since I just wipe the
> defaults if I see a 2.0 hwpack.

Hmm, that does make it trickier.

Could we have it only wipe the data if the 2.0 hwpack contains a single field?

> Do you think I should allow for the 1.0 defaults to trickle in when in
> mixed 1.0/2.0 mode, or just decide that in mixed mode, the 2.0
> metadata has to be there?

It seems overly restrictive to have that, but I don't think it's
important, so we shouldn't spend a lot of time on this.

2.0 is the future, so let's make that work well, and keep the code
maintainable.

If the easiest way to do that is to forbid a mix of 1.0/2.0 then let's
do that.

Thanks,

James

[Mattias Backman (mabac)]

> On Wed, 29 Jun 2011 07:35:50 -0000, Mattias Backman
> <email address hidden> wrote:
> > Ok. With the current code, which assumes that the defaults are not to
> > be used for 2.0 hwpacks, the 2.0 hwpack would simply take
> > precedence. However if the 2.0 metadata is not there (perhaps allowed
> > for a pure x11 hwpack?) there would be a problem since I just wipe the
> > defaults if I see a 2.0 hwpack.
>
> Hmm, that does make it trickier.
>
> Could we have it only wipe the data if the 2.0 hwpack contains a single field?
>
> > Do you think I should allow for the 1.0 defaults to trickle in when in
> > mixed 1.0/2.0 mode, or just decide that in mixed mode, the 2.0
> > metadata has to be there?
>
> It seems overly restrictive to have that, but I don't think it's
> important, so we shouldn't spend a lot of time on this.
>
> 2.0 is the future, so let's make that work well, and keep the code
> maintainable.
>
> If the easiest way to do that is to forbid a mix of 1.0/2.0 then let's
> do that.

I'll have to sleep on this. I think that we can keep the defaults if there is one or more 1.0 hwpacks supplied and wipe the defaults only there are no 1.0 hwpacks at all. That would mean that eventually when no-one uses 1.0 hwpacks at all anymore, the 2.0 hwpacks need to carry the data.

I think I may have left out making hwpack-create assert that the 2.0 fields should be mandatory in the validate methods.

=== modified file 'linaro-hwpack-install'
--- linaro-hwpack-install	2011-01-29 16:35:06 +0000
+++ linaro-hwpack-install	2011-06-29 14:00:56 +0000
@@ -36,7 +36,7 @@
 FORCE_YES="no"
 SOURCES_LIST_FILE="${TEMP_DIR}/sources.list"
 APT_GET_OPTIONS="Dir::Etc::SourceList=${SOURCES_LIST_FILE}"
-SUPPORTED_FORMATS="1.0"  # A space-separated list of hwpack formats.
+SUPPORTED_FORMATS="1.0 2.0"  # A space-separated list of hwpack formats.
 
 sudo="sudo"
 if [ $(id -u) -eq 0 ]; then

=== modified file 'linaro-media-create'
--- linaro-media-create	2011-06-21 09:16:28 +0000
+++ linaro-media-create	2011-06-29 14:00:56 +0000
@@ -104,6 +104,7 @@
     ROOT_DISK = os.path.join(TMP_DIR, 'root-disc')
 
     board_config = board_configs[args.board]
+    board_config.set_metadata(args.hwpacks)
 
     ensure_required_commands(args)
 

=== modified file 'linaro_image_tools/hwpack/config.py'
--- linaro_image_tools/hwpack/config.py	2011-06-20 13:46:16 +0000
+++ linaro_image_tools/hwpack/config.py	2011-06-29 14:00:56 +0000
@@ -42,7 +42,7 @@
     SOURCES_ENTRY_KEY = "sources-entry"
     PACKAGES_KEY = "packages"
     PACKAGE_REGEX = NAME_REGEX
-    PATH_REGEX = r"[a-z0-9][a-z0-9+\-./]+$"
+    PATH_REGEX = r"[a-z0-9][a-z0-9+\-./_]+$"
     ORIGIN_KEY = "origin"
     MAINTAINER_KEY = "maintainer"
     ARCHITECTURES_KEY = "architectures"
@@ -59,6 +59,13 @@
     MMC_ID_KEY = "mmc_id"
     FORMAT_KEY = "format"
 
+    DEFINED_PARTITION_LAYOUTS = [
+        'bootfs16_rootfs',
+        'bootfs_rootfs',
+        #'reserved_bootfs_rootfs',
+        ]
+
+
     def __init__(self, fp):
         """Create a Config.
 
@@ -375,17 +382,12 @@
         pass
 
     def _validate_partition_layout(self):
-        defined_partition_layouts = [
-            #'bootfs16_rootfs',
-            'bootfs_rootfs',
-            #'reserved_bootfs_rootfs',
-            ]
-        if self.partition_layout not in defined_partition_layouts:
+        if self.partition_layout not in self.DEFINED_PARTITION_LAYOUTS:
             raise HwpackConfigError(
                 "Undefined partition layout %s in the [%s] section. "
                 "Valid partition layouts are %s."
                 % (self.partition_layout, self.MAIN_SECTION,
-                   ", ".join(defined_partition_layouts)))
+                   ", ".join(self.DEFINED_PARTITION_LAYOUTS)))
 
     def _validate_mmc_id(self):
         mmc_id = self.mmc_id

=== modified file 'linaro_image_tools/hwpack/tests/test_config.py'
--- linaro_image_tools/hwpack/tests/test_config.py	2011-06-17 12:28:43 +0000
+++ linaro_image_tools/hwpack/tests/test_config.py	2011-06-29 14:00:56 +0000
@@ -216,13 +216,16 @@
         self.assertValidationError("Invalid path: ~~", config)
 
     def test_validate_partition_layout(self):
-        config = self.get_config(self.valid_start_v2 + 
-                                 "u-boot-package = u-boot-linaro-s5pv310\n" \
-                                     "u-boot-file = u-boot.bin\n" \
-                                     "partition_layout = apafs_bananfs\n")
+        partition_layout = 'apafs_bananfs'
+        config = self.get_config(self.valid_start_v2 + "u-boot-package = " \
+                                     "u-boot-linaro-s5pv310\nu-boot-file = " \
+                                     "u-boot.bin\npartition_layout = %s\n" % \
+                                     partition_layout)
         self.assertValidationError(
-            "Undefined partition layout apafs_bananfs in the [hwpack] " \
-                "section. Valid partition layouts are bootfs_rootfs.", config)
+            "Undefined partition layout %s in the [%s] section. "
+            "Valid partition layouts are %s."
+            % (partition_layout, 'hwpack',
+               ", ".join(config.DEFINED_PARTITION_LAYOUTS)), config)
 
     def test_validate_wired_interfaces(self):
         self.assertTrue("XXX What is an invalid interface name?")

=== modified file 'linaro_image_tools/media_create/boards.py'
--- linaro_image_tools/media_create/boards.py	2011-06-23 15:12:13 +0000
+++ linaro_image_tools/media_create/boards.py	2011-06-29 14:00:56 +0000
@@ -31,11 +31,20 @@
 import tempfile
 import struct
 from binascii import crc32
+import tarfile
+import ConfigParser
+import shutil
 
 from linaro_image_tools import cmd_runner
 
 from linaro_image_tools.media_create.partitions import SECTOR_SIZE
 
+from linaro_image_tools.hwpack.hardwarepack_format import (
+    HardwarePackFormatV1,
+    HardwarePackFormatV2,
+)
+
+
 KERNEL_GLOB = 'vmlinuz-*-%(kernel_flavor)s'
 INITRD_GLOB = 'initrd.img-*-%(kernel_flavor)s'
 DTB_GLOB = 'dt-*-%(kernel_flavor)s/%(dtb_name)s'
@@ -129,6 +138,95 @@
         return self.getter(cls)
 
 
+class HardwarepackHandler(object):
+    metadata_filename = 'metadata'
+    format_filename = 'FORMAT'
+    main_section = 'main'
+    hwpack_tarfiles = []
+    tempdir = None
+
+    def __init__(self, hwpacks):
+        self.hwpacks = hwpacks
+        self.hwpack_tarfiles = []
+    
+    class FakeSecHead(object):
+        """ Add a fake section header to the metadata file.
+
+        This is done so we can use ConfigParser to parse the file.
+        """
+        def __init__(self, fp):
+            self.fp = fp
+            self.sechead = '[%s]\n' % HardwarepackHandler.main_section
+
+        def readline(self):
+            if self.sechead:
+                try:
+                    return self.sechead
+                finally:
+                    self.sechead = None
+            else:
+                return self.fp.readline()
+
+    def __enter__(self):
+        self.tempdir = tempfile.mkdtemp()
+        for hwpack in self.hwpacks:
+            hwpack_tarfile = tarfile.open(hwpack, mode='r:gz')
+            self.hwpack_tarfiles.append(hwpack_tarfile)
+        return self
+
+    def __exit__(self, type, value, traceback):
+        for hwpack_tarfile in self.hwpack_tarfiles:
+            if hwpack_tarfile is not None:
+                hwpack_tarfile.close()
+        self.hwpack_tarfiles = []
+        if self.tempdir is not None and os.path.exists(self.tempdir):
+            shutil.rmtree(self.tempdir)
+
+    def get_field(self, section, field):
+        data = None
+        hwpack_with_data = None
+        for hwpack_tarfile in self.hwpack_tarfiles:
+            metadata = hwpack_tarfile.extractfile(self.metadata_filename)
+            # Use RawConfigParser which does not support the magical interpolation
+            # behavior of ConfigParser so we don't mess up metadata accidentally.
+            parser = ConfigParser.RawConfigParser()
+            parser.readfp(self.FakeSecHead(metadata))
+            try:
+                new_data = parser.get(section, field)
+                if new_data is not None:
+                    assert data is None, "The metadata field '%s' is set to " \
+                        "'%s' and new value '%s' is found" % (field, data, new_data)
+                    data = new_data
+                    hwpack_with_data = hwpack_tarfile
+            except ConfigParser.NoOptionError:
+                continue
+        return data, hwpack_with_data
+
+    def get_format(self):
+        format = None
+        for hwpack_tarfile in self.hwpack_tarfiles:
+            format_file = hwpack_tarfile.extractfile(self.format_filename)
+            format_string = format_file.read().strip()
+            if format_string == '2.0':
+                # Format is taken from the hwpack of highest format version.
+                return HardwarePackFormatV2()
+            else:
+                format = format_string
+        if format is None or format == '1.0':
+            return HardwarePackFormatV1()
+        else:
+            raise AssertionError("Format version '%s' is not supported." % \
+                                     format_string)
+
+    def get_file(self, file_alias):
+        file_name, hwpack_tarfile = self.get_field(self.main_section,
+                                                   file_alias)
+        if file_name is not None:
+            hwpack_tarfile.extract(file_name, self.tempdir)
+            file_name = os.path.join(self.tempdir, file_name)
+        return file_name
+
+
 class BoardConfig(object):
     """The configuration used when building an image for a board."""
     # These attributes may not need to be redefined on some subclasses.
@@ -138,12 +236,14 @@
     mmc_option = '0:1'
     mmc_part_offset = 0
     fat_size = 32
-    extra_serial_opts = ''
-    live_serial_opts = ''
+    _extra_serial_opts = ''
+    _live_serial_opts = ''
     extra_boot_args_options = None
     supports_writing_to_mmc = True
 
-    # These attributes must be defined on all subclasses.
+    # These attributes must be defined on all subclasses for backwards
+    # compatibility with hwpacks v1 format. Hwpacks v2 format allows these to
+    # be specified in the hwpack metadata.
     kernel_addr = None
     initrd_addr = None
     load_addr = None
@@ -152,6 +252,71 @@
     kernel_flavors = None
     boot_script = None
     serial_tty = None
+    wired_interfaces = None
+    wireless_interfaces = None
+    mmc_id = None
+
+    hardwarepack_handler = None
+
+    @classmethod
+    def get_metadata_field(cls, target, field_name):
+        """ Return the metadata value for field_name if it can be found.
+
+        Also assert that target is None to make sure that no defaults have
+        come this far by mistake.
+        """
+        data, _ = cls.hardwarepack_handler.get_field(
+            cls.hardwarepack_handler.main_section, field_name)
+        if data is not None:
+            assert target is None, "Metadata field '%s' is already set to " \
+                "'%s'." % (field_name, target)
+        return data
+
+    @classmethod
+    def set_metadata(cls, hwpacks):
+        cls.hardwarepack_handler = HardwarepackHandler(hwpacks)
+        with cls.hardwarepack_handler:
+            if not cls.hardwarepack_handler.get_format().has_v2_fields:
+                return
+
+            # Clear V1 defaults.
+            cls.kernel_addr = None
+            cls.initrd_addr = None
+            cls.load_addr = None
+            cls.serial_tty = None
+            cls.fat_size = None
+
+            # Set new values from metadata.
+            cls.kernel_addr = cls.get_metadata_field(
+                cls.kernel_addr, 'kernel_addr')
+            cls.initrd_addr = cls.get_metadata_field(
+                cls.initrd_addr, 'initrd_addr')
+            cls.load_addr = cls.get_metadata_field(
+                cls.load_addr, 'load_addr')
+            cls.serial_tty = cls.get_metadata_field(
+                cls.serial_tty, 'serial_tty')
+            cls.wired_interfaces = cls.get_metadata_field(
+                cls.wired_interfaces, 'wired_interfaces')
+            cls.wireless_interfaces = cls.get_metadata_field(
+                cls.wireless_interfaces, 'wireless_interfaces')
+            cls.mmc_id = cls.get_metadata_field(
+                cls.mmc_id, 'mmc_id')
+
+            partition_layout = cls.get_metadata_field(cls.fat_size, 'partition_layout')
+            if partition_layout == 'bootfs_rootfs' or partition_layout is None:
+                cls.fat_size = 32
+            elif partition_layout == 'bootfs16_rootfs':
+                cls.fat_size = 16
+            else:
+                raise AssertionError("Unknown partition layout '%s'." % partition_layout)
+
+    @classmethod
+    def get_file(cls, file_alias, default=None):
+        file_in_hwpack = cls.hardwarepack_handler.get_file(file_alias)
+        if file_in_hwpack is not None:
+            return file_in_hwpack
+        else:
+            return default
 
     @classmethod
     def get_sfdisk_cmd(cls, should_align_boot_part=False):
@@ -299,10 +464,12 @@
         if cls.uboot_in_boot_part:
             assert cls.uboot_flavor is not None, (
                 "uboot_in_boot_part is set but not uboot_flavor")
-            uboot_bin = os.path.join(chroot_dir, 'usr', 'lib', 'u-boot',
-                cls.uboot_flavor, 'u-boot.bin')
-            cmd_runner.run(
-                ['cp', '-v', uboot_bin, boot_disk], as_root=True).wait()
+            with cls.hardwarepack_handler:
+                uboot_bin = cls.get_file('u_boot', default=os.path.join(
+                        chroot_dir, 'usr', 'lib', 'u-boot', cls.uboot_flavor,
+                        'u-boot.bin'))
+                cmd_runner.run(
+                    ['cp', '-v', uboot_bin, boot_disk], as_root=True).wait()
 
         cls.make_boot_files(
             uboot_parts_dir, is_live, is_lowmem, consoles, chroot_dir,
@@ -471,8 +638,8 @@
 
 class Ux500Config(BoardConfig):
     serial_tty = 'ttyAMA2'
-    extra_serial_opts = 'console=tty0 console=%s,115200n8' % serial_tty
-    live_serial_opts = 'serialtty=%s' % serial_tty
+    _extra_serial_opts = 'console=tty0 console=%s,115200n8'
+    _live_serial_opts = 'serialtty=%s'
     kernel_addr = '0x00100000'
     initrd_addr = '0x08000000'
     load_addr = '0x00008000'
@@ -485,6 +652,14 @@
         'hwmem=48M@302M mem=152M@360M')
     mmc_option = '1:1'
 
+    @classproperty
+    def live_serial_opts(cls):
+        return cls._live_serial_opts % cls.serial_tty
+
+    @classproperty
+    def extra_serial_opts(cls):
+        return cls._extra_serial_opts % cls.serial_tty
+
     @classmethod
     def _make_boot_files(cls, boot_env, chroot_dir, boot_dir,
                          boot_device_or_file, k_img_data, i_img_data,
@@ -642,12 +817,20 @@
 
 class Mx5Config(BoardConfig):
     serial_tty = 'ttymxc0'
-    extra_serial_opts = 'console=tty0 console=%s,115200n8' % serial_tty
-    live_serial_opts = 'serialtty=%s' % serial_tty
+    _extra_serial_opts = 'console=tty0 console=%s,115200n8'
+    _live_serial_opts = 'serialtty=%s'
     boot_script = 'boot.scr'
     mmc_part_offset = 1
     mmc_option = '0:2'
 
+    @classproperty
+    def live_serial_opts(cls):
+        return cls._live_serial_opts % cls.serial_tty
+
+    @classproperty
+    def extra_serial_opts(cls):
+        return cls._extra_serial_opts % cls.serial_tty
+
     @classmethod
     def get_sfdisk_cmd(cls, should_align_boot_part=None):
         """Return the sfdisk command to partition the media.
@@ -680,9 +863,11 @@
     def _make_boot_files(cls, boot_env, chroot_dir, boot_dir,
                          boot_device_or_file, k_img_data, i_img_data,
                          d_img_data):
-        uboot_file = os.path.join(
-            chroot_dir, 'usr', 'lib', 'u-boot', cls.uboot_flavor, 'u-boot.imx')
-        install_mx5_boot_loader(uboot_file, boot_device_or_file)
+        with cls.hardwarepack_handler:
+            uboot_file = cls.get_file('u_boot', default=os.path.join(
+                    chroot_dir, 'usr', 'lib', 'u-boot', cls.uboot_flavor,
+                    'u-boot.imx'))
+            install_mx5_boot_loader(uboot_file, boot_device_or_file)
         make_uImage(cls.load_addr, k_img_data, boot_dir)
         make_uInitrd(i_img_data, boot_dir)
         make_dtb(d_img_data, boot_dir)
@@ -730,8 +915,8 @@
     uboot_flavor = 'ca9x4_ct_vxp'
     uboot_in_boot_part = True
     serial_tty = 'ttyAMA0'
-    extra_serial_opts = 'console=tty0 console=%s,38400n8' % serial_tty
-    live_serial_opts = 'serialtty=%s' % serial_tty
+    _extra_serial_opts = 'console=tty0 console=%s,38400n8'
+    _live_serial_opts = 'serialtty=%s'
     kernel_addr = '0x60008000'
     initrd_addr = '0x81000000'
     load_addr = kernel_addr
@@ -741,6 +926,14 @@
     # only allows for FAT16
     fat_size = 16
 
+    @classproperty
+    def live_serial_opts(cls):
+        return cls._live_serial_opts % cls.serial_tty
+
+    @classproperty
+    def extra_serial_opts(cls):
+        return cls._extra_serial_opts % cls.serial_tty
+
     @classmethod
     def _make_boot_files(cls, boot_env, chroot_dir, boot_dir,
                          boot_device_or_file, k_img_data, i_img_data,
@@ -751,7 +944,7 @@
 class SMDKV310Config(BoardConfig):
     uboot_flavor = 'smdkv310'
     serial_tty = 'ttySAC1'
-    extra_serial_opts = 'console=%s,115200n8' % serial_tty
+    _extra_serial_opts = 'console=%s,115200n8'
     kernel_addr = '0x40007000'
     initrd_addr = '0x42000000'
     load_addr = '0x40008000'
@@ -760,6 +953,10 @@
     mmc_part_offset = 1
     mmc_option = '0:2'
 
+    @classproperty
+    def extra_serial_opts(cls):
+        return cls._extra_serial_opts % cls.serial_tty
+
     @classmethod
     def get_sfdisk_cmd(cls, should_align_boot_part=False):
         # bootloaders partition needs to hold BL1, U-Boot environment, and BL2

=== modified file 'linaro_image_tools/media_create/tests/test_media_create.py'
--- linaro_image_tools/media_create/tests/test_media_create.py	2011-06-23 15:12:13 +0000
+++ linaro_image_tools/media_create/tests/test_media_create.py	2011-06-29 14:00:56 +0000
@@ -28,7 +28,9 @@
 import textwrap
 import time
 import types
+import tarfile
 
+from StringIO import StringIO
 from testtools import TestCase
 
 from linaro_image_tools import cmd_runner
@@ -61,6 +63,8 @@
     _get_smdk_spl,
     _get_smdk_uboot,
     _run_mkimage,
+    HardwarepackHandler,
+    BoardConfig,
     )
 from linaro_image_tools.media_create.android_boards import (
     android_board_configs,
@@ -115,12 +119,321 @@
     MockSomethingFixture,
     )
 from linaro_image_tools.utils import find_command, preferred_tools_dir
+from linaro_image_tools.hwpack.hardwarepack_format import (
+    HardwarePackFormatV1,
+    HardwarePackFormatV2,
+)
 
 
 chroot_args = " ".join(cmd_runner.CHROOT_ARGS)
 sudo_args = " ".join(cmd_runner.SUDO_ARGS)
 
 
+class TestHardwarepackHandler(TestCaseWithFixtures):
+    def setUp(self):
+        super(TestHardwarepackHandler, self).setUp()
+        self.tar_dir_fixture = CreateTempDirFixture()
+        self.useFixture(self.tar_dir_fixture)
+
+        self.tarball_fixture = CreateTarballFixture(
+            self.tar_dir_fixture.get_temp_dir())
+        self.useFixture(self.tarball_fixture)
+
+        self.metadata = (
+            "NAME=ahwpack\nVERSION=4\nARCHITECTURE=armel\nORIGIN=linaro\n")
+
+    def add_to_tarball(self, files, tarball=None):
+        if tarball is None:
+            tarball = self.tarball_fixture.get_tarball()
+        tar_file = tarfile.open(tarball, mode='w:gz')
+        for filename, data in files:
+            tarinfo = tarfile.TarInfo(filename)
+            tarinfo.size = len(data)
+            tar_file.addfile(tarinfo, StringIO(data))
+        tar_file.close()
+        return tarball
+
+    def test_get_format_1(self):
+        data = '1.0'
+        format = "%s\n" % data
+        tarball = self.add_to_tarball(
+            [('FORMAT', format), ('metadata', self.metadata)])
+        hp = HardwarepackHandler([tarball])
+        with hp:
+            self.assertIsInstance(hp.get_format(), HardwarePackFormatV1)
+
+    def test_get_format_2(self):
+        data = '2.0'
+        format = "%s\n" % data
+        tarball = self.add_to_tarball(
+            [('FORMAT', format), ('metadata', self.metadata)])
+        hp = HardwarepackHandler([tarball])
+        with hp:
+            self.assertIsInstance(hp.get_format(), HardwarePackFormatV2)
+
+    def test_get_unknown_format_raises(self):
+        data = '9.9'
+        format = "%s\n" % data
+        tarball = self.add_to_tarball(
+            [('FORMAT', format), ('metadata', self.metadata)])
+        hp = HardwarepackHandler([tarball])
+        with hp:
+            self.assertRaises(AssertionError, hp.get_format)
+
+    def test_higher_format_sticks(self):
+        format1 = "%s\n" % '1.0'
+        format2 = "%s\n" % '2.0'
+        tarball1 = self.add_to_tarball(
+            [('FORMAT', format1), ('metadata', self.metadata)],
+            tarball=self.tarball_fixture.get_tarball())
+        tarball_fixture2 = CreateTarballFixture(
+            self.tar_dir_fixture.get_temp_dir(), reldir='tarfile2',
+            filename='secondtarball.tar.gz')
+        self.useFixture(tarball_fixture2)
+        tarball2 = self.add_to_tarball(
+            [('FORMAT', format2), ('metadata', self.metadata)],
+            tarball=tarball_fixture2.get_tarball())
+        hp = HardwarepackHandler([tarball2, tarball1])
+        with hp:
+            self.assertIsInstance(hp.get_format(), HardwarePackFormatV2)
+
+    def test_identical_formats_ok(self):
+        format1 = "%s\n" % '2.0'
+        format2 = "%s\n" % '2.0'
+        tarball1 = self.add_to_tarball(
+            [('FORMAT', format1), ('metadata', self.metadata)],
+            tarball=self.tarball_fixture.get_tarball())
+        tarball_fixture2 = CreateTarballFixture(
+            self.tar_dir_fixture.get_temp_dir(), reldir='tarfile2',
+            filename='secondtarball.tar.gz')
+        self.useFixture(tarball_fixture2)
+        tarball2 = self.add_to_tarball(
+            [('FORMAT', format2), ('metadata', self.metadata)],
+            tarball=tarball_fixture2.get_tarball())
+        hp = HardwarepackHandler([tarball1, tarball2])
+        with hp:
+            self.assertIsInstance(hp.get_format(), HardwarePackFormatV2)
+
+    def test_get_metadata(self):
+        data = 'data to test'
+        metadata = self.metadata + "TEST=%s\n" % data
+        tarball = self.add_to_tarball(
+            [('metadata', metadata)])
+        hp = HardwarepackHandler([tarball])
+        with hp:
+            test_data, _ = hp.get_field(hp.main_section, 'test')
+            self.assertEqual(test_data, data)
+
+    def test_preserves_formatters(self):
+        data = '%s%d'
+        metadata = self.metadata + "TEST=%s\n" % data
+        tarball = self.add_to_tarball(
+            [('metadata', metadata)])
+        hp = HardwarepackHandler([tarball])
+        with hp:
+            test_data, _ = hp.get_field(hp.main_section, 'test')
+            self.assertEqual(test_data, data)
+
+    def test_creates_tempdir(self):
+        tarball = self.add_to_tarball(
+            [('metadata', self.metadata)])
+        hp = HardwarepackHandler([tarball])
+        with hp:
+            self.assertTrue(os.path.exists(hp.tempdir))
+
+    def test_tempfiles_are_removed(self):
+        tempdir = None
+        tarball = self.add_to_tarball(
+            [('metadata', self.metadata)])
+        hp = HardwarepackHandler([tarball])
+        with hp:
+            tempdir = hp.tempdir
+        self.assertFalse(os.path.exists(tempdir))
+
+    def test_get_file(self):
+        data = 'test file contents\n'
+        metadata_file = 'TESTFILE'
+        file_in_archive = 'testfile'
+        metadata = self.metadata + "%s=%s\n" % (metadata_file, file_in_archive)
+        tarball = self.add_to_tarball(
+            [('metadata', metadata),
+             (file_in_archive, data)])
+        hp = HardwarepackHandler([tarball])
+        with hp:
+            test_file = hp.get_file(metadata_file)
+            self.assertEquals(data, open(test_file, 'r').read())
+        
+
+class TestSetMetadata(TestCaseWithFixtures):
+
+    class MockHardwarepackHandler(HardwarepackHandler):
+        metadata_dict = {}
+
+        def __enter__(self):
+            return self
+
+        def get_field(self, section, field):
+            try:
+                return self.metadata_dict[field], None
+            except:
+                return None, None
+
+        def get_format(self):
+            return HardwarePackFormatV2()
+
+        def get_file(self, file_alias):
+            return None
+
+    def test_does_not_set_if_old_format(self):
+        self.useFixture(MockSomethingFixture(
+                linaro_image_tools.media_create.boards, 'HardwarepackHandler',
+                self.MockHardwarepackHandler))
+
+        class config(BoardConfig):
+            pass
+        config.set_metadata('ahwpack.tar.gz')
+        self.assertEquals(None, config.kernel_addr)
+
+    def test_sets_kernel_addr(self):
+        self.useFixture(MockSomethingFixture(
+                linaro_image_tools.media_create.boards, 'HardwarepackHandler',
+                self.MockHardwarepackHandler))
+        field_to_test = 'kernel_addr'
+        data_to_set = '0x8123ABCD'
+        self.MockHardwarepackHandler.metadata_dict = {
+            field_to_test: data_to_set,
+            }
+        class config(BoardConfig):
+            pass
+        config.set_metadata('ahwpack.tar.gz')
+        self.assertEquals(data_to_set, config.kernel_addr)
+
+    def test_sets_initrd_addr(self):
+        self.useFixture(MockSomethingFixture(
+                linaro_image_tools.media_create.boards, 'HardwarepackHandler',
+                self.MockHardwarepackHandler))
+        field_to_test = 'initrd_addr'
+        data_to_set = '0x8123ABCD'
+        self.MockHardwarepackHandler.metadata_dict = {
+            field_to_test: data_to_set,
+            }
+        class config(BoardConfig):
+            pass
+        config.set_metadata('ahwpack.tar.gz')
+        self.assertEquals(data_to_set, config.initrd_addr)
+
+    def test_sets_load_addr(self):
+        self.useFixture(MockSomethingFixture(
+                linaro_image_tools.media_create.boards, 'HardwarepackHandler',
+                self.MockHardwarepackHandler))
+        field_to_test = 'load_addr'
+        data_to_set = '0x8123ABCD'
+        self.MockHardwarepackHandler.metadata_dict = {
+            field_to_test: data_to_set,
+            }
+        class config(BoardConfig):
+            pass
+        config.set_metadata('ahwpack.tar.gz')
+        self.assertEquals(data_to_set, config.load_addr)
+
+    def test_sets_serial_tty(self):
+        self.useFixture(MockSomethingFixture(
+                linaro_image_tools.media_create.boards, 'HardwarepackHandler',
+                self.MockHardwarepackHandler))
+        field_to_test = 'serial_tty'
+        data_to_set = 'ttyAA'
+        self.MockHardwarepackHandler.metadata_dict = {
+            field_to_test: data_to_set,
+            }
+        class config(BoardConfig):
+            pass
+        config.set_metadata('ahwpack.tar.gz')
+        self.assertEquals(data_to_set, config.serial_tty)
+
+    def test_sets_wired_interfaces(self):
+        self.useFixture(MockSomethingFixture(
+                linaro_image_tools.media_create.boards, 'HardwarepackHandler',
+                self.MockHardwarepackHandler))
+        field_to_test = 'wired_interfaces'
+        data_to_set = 'eth0 eth1'
+        self.MockHardwarepackHandler.metadata_dict = {
+            field_to_test: data_to_set,
+            }
+        class config(BoardConfig):
+            pass
+        config.set_metadata('ahwpack.tar.gz')
+        self.assertEquals(data_to_set, config.wired_interfaces)
+
+    def test_sets_wireless_interfaces(self):
+        self.useFixture(MockSomethingFixture(
+                linaro_image_tools.media_create.boards, 'HardwarepackHandler',
+                self.MockHardwarepackHandler))
+        field_to_test = 'wireless_interfaces'
+        data_to_set = 'wlan0 wl1'
+        self.MockHardwarepackHandler.metadata_dict = {
+            field_to_test: data_to_set,
+            }
+        class config(BoardConfig):
+            pass
+        config.set_metadata('ahwpack.tar.gz')
+        self.assertEquals(data_to_set, config.wireless_interfaces)
+
+    def test_sets_mmc_id(self):
+        self.useFixture(MockSomethingFixture(
+                linaro_image_tools.media_create.boards, 'HardwarepackHandler',
+                self.MockHardwarepackHandler))
+        field_to_test = 'mmc_id'
+        data_to_set = '1'
+        self.MockHardwarepackHandler.metadata_dict = {
+            field_to_test: data_to_set,
+            }
+        class config(BoardConfig):
+            pass
+        config.set_metadata('ahwpack.tar.gz')
+        self.assertEquals(data_to_set, config.mmc_id)
+
+    def test_sets_partition_layout_32(self):
+        self.useFixture(MockSomethingFixture(
+                linaro_image_tools.media_create.boards, 'HardwarepackHandler',
+                self.MockHardwarepackHandler))
+        field_to_test = 'partition_layout'
+        data_to_set = 'bootfs_rootfs'
+        self.MockHardwarepackHandler.metadata_dict = {
+            field_to_test: data_to_set,
+            }
+        class config(BoardConfig):
+            pass
+        config.set_metadata('ahwpack.tar.gz')
+        self.assertEquals(32, config.fat_size)
+
+    def test_sets_partition_layout_16(self):
+        self.useFixture(MockSomethingFixture(
+                linaro_image_tools.media_create.boards, 'HardwarepackHandler',
+                self.MockHardwarepackHandler))
+        field_to_test = 'partition_layout'
+        data_to_set = 'bootfs16_rootfs'
+        self.MockHardwarepackHandler.metadata_dict = {
+            field_to_test: data_to_set,
+            }
+        class config(BoardConfig):
+            pass
+        config.set_metadata('ahwpack.tar.gz')
+        self.assertEquals(16, config.fat_size)
+
+    def test_sets_partition_layout_raises(self):
+        self.useFixture(MockSomethingFixture(
+                linaro_image_tools.media_create.boards, 'HardwarepackHandler',
+                self.MockHardwarepackHandler))
+        field_to_test = 'partition_layout'
+        data_to_set = 'bootfs_bogus_rootfs'
+        self.MockHardwarepackHandler.metadata_dict = {
+            field_to_test: data_to_set,
+            }
+        class config(BoardConfig):
+            pass
+        self.assertRaises(AssertionError, config.set_metadata, 'ahwpack.tar.gz')
+
+
 class TestGetMLOFile(TestCaseWithFixtures):
 
     def test_mlo_from_new_xloader(self):
@@ -255,6 +568,10 @@
     def test_mx5_steps(self):
         class SomeMx5Config(boards.Mx5Config):
             uboot_flavor = 'uboot_flavor'
+        SomeMx5Config.hardwarepack_handler = (
+            TestSetMetadata.MockHardwarepackHandler('ahwpack.tar.gz'))
+        SomeMx5Config.hardwarepack_handler.get_format = (
+            lambda: HardwarePackFormatV1())
         self.make_boot_files(SomeMx5Config)
         expected = [
             'install_mx5_boot_loader', 'make_uImage', 'make_uInitrd',
@@ -1252,6 +1569,9 @@
 
         self.config = c
         self.config.boot_script = 'boot_script'
+        self.config.hardwarepack_handler = \
+            TestSetMetadata.MockHardwarepackHandler('ahwpack.tar.gz')
+        self.config.hardwarepack_handler.get_format = lambda: HardwarePackFormatV1()
         self.popen_fixture = self.useFixture(MockCmdRunnerPopenFixture())
         self.useFixture(MockSomethingFixture(
             self.config, 'make_boot_files', self.save_args))