diff mbox

[Branch,~linaro-image-tools/linaro-image-tools/trunk] Rev 554: Fix for multiple bootloaders in a hardware pack configuration

Message ID 20120830101411.24053.29080.launchpad@ackee.canonical.com
State Accepted
Headers show

Commit Message

James Tunnicliffe Aug. 30, 2012, 10:14 a.m. UTC
Merge authors:
  James Tunnicliffe (dooferlad)
Related merge proposals:
  https://code.launchpad.net/~dooferlad/linaro-image-tools/fix-multiple-bootloaders/+merge/121880
  proposed by: James Tunnicliffe (dooferlad)
  review: Approve - Milo Casagrande (milo)
  review: Approve - Paul Sokolovsky (pfalcon)
------------------------------------------------------------
revno: 554 [merge]
committer: James Tunnicliffe <james.tunnicliffe@linaro.org>
branch nick: linaro-image-tools
timestamp: Thu 2012-08-30 11:13:21 +0100
message:
  Fix for multiple bootloaders in a hardware pack configuration
modified:
  linaro_image_tools/hwpack/builder.py
  linaro_image_tools/hwpack/config.py
  linaro_image_tools/hwpack/testing.py
  linaro_image_tools/hwpack/tests/test_config.py
  linaro_image_tools/hwpack/tests/test_config_v3.py
  linaro_image_tools/hwpack/tests/test_script.py


--
lp:linaro-image-tools
https://code.launchpad.net/~linaro-image-tools/linaro-image-tools/trunk

You are subscribed to branch lp:linaro-image-tools.
To unsubscribe from this branch go to https://code.launchpad.net/~linaro-image-tools/linaro-image-tools/trunk/+edit-subscription
diff mbox

Patch

=== modified file 'linaro_image_tools/hwpack/builder.py'
--- linaro_image_tools/hwpack/builder.py	2012-07-26 14:46:31 +0000
+++ linaro_image_tools/hwpack/builder.py	2012-08-28 17:34:16 +0000
@@ -97,7 +97,7 @@ 
     def __init__(self, config_path, version, local_debs):
         try:
             with open(config_path) as fp:
-                self.config = Config(fp)
+                self.config = Config(fp, allow_unset_bootloader=True)
         except IOError, e:
             if e.errno == errno.ENOENT:
                 raise ConfigFileMissing(config_path)
@@ -109,6 +109,7 @@ 
         self.package_unpacker = None
         self.hwpack = None
         self.packages = None
+        self.packages_added_to_hwpack = []
 
     def find_fetched_package(self, packages, wanted_package_name):
         wanted_package = None
@@ -122,8 +123,13 @@ 
         return wanted_package
 
     def add_file_to_hwpack(self, package, wanted_file, target_path):
+        if (package.name, target_path) in self.packages_added_to_hwpack:
+            # Don't bother adding the same package more than once.
+            return
+
         tempfile_name = self.package_unpacker.get_file(
             package.filepath, wanted_file)
+        self.packages_added_to_hwpack.append((package.name, target_path))
         return self.hwpack.add_file(target_path, tempfile_name)
 
     def find_bootloader_packages(self, bootloaders_config):
@@ -150,7 +156,6 @@ 
         :param config_dictionary: The dictionary from the Config we need to
                                     look into.
         """
-        remove_packages = []
         for key, value in config_dictionary.iteritems():
             if isinstance(value, dict):
                 self._set_new_values(value)
@@ -171,11 +176,6 @@ 
                         file_to_add = self.add_file_to_hwpack(
                                             package, boot_file, path)
                         config_dictionary[change_field] = file_to_add
-                        remove_packages.append(package)
-        # Clean up duplicates.
-        for package in remove_packages:
-            if package in self.packages:
-                self.packages.remove(package)
 
     def build(self):
         for architecture in self.config.architectures:

=== modified file 'linaro_image_tools/hwpack/config.py'
--- linaro_image_tools/hwpack/config.py	2012-08-28 06:02:43 +0000
+++ linaro_image_tools/hwpack/config.py	2012-08-30 10:13:21 +0000
@@ -117,11 +117,14 @@ 
     BOOTLOADER_DD_KEY = 'u_boot_dd'
     translate_v2_to_v3[BOOTLOADER_DD_KEY] = DD_FIELD
 
-    def __init__(self, fp, bootloader=None, board=None):
+    def __init__(self, fp, bootloader=None, board=None,
+                 allow_unset_bootloader=False):
         """Create a Config.
 
         :param fp: a file-like object containing the configuration.
         """
+        self.allow_unset_bootloader = allow_unset_bootloader
+        self.bootloader = None
         obfuscated_e = None
         obfuscated_yaml_e = ""
         try:
@@ -178,6 +181,23 @@ 
         """Set board used to look up per-board configuration"""
         self.board = board
 
+    def get_bootloader_list(self):
+        if isinstance(self.bootloaders, dict):
+            # We have a list of bootloaders in the expected format
+            return self.bootloaders.keys()
+        return []
+
+    def validate_bootloader_fields(self):
+        self._validate_bootloader_package()
+        self._validate_bootloader_file()
+        self._validate_spl_package()
+        self._validate_spl_file()
+        self._validate_bootloader_file_in_boot_part()
+        self._validate_bootloader_dd()
+        self._validate_spl_in_boot_part()
+        self._validate_spl_dd()
+        self._validate_env_dd()
+
     def validate(self):
         """Check that this configuration follows the schema.
 
@@ -196,8 +216,15 @@ 
         self._validate_assume_installed()
 
         if self.format.has_v2_fields:
-            self._validate_bootloader_package()
-            self._validate_bootloader_file()
+            # Check config for all bootloaders if one isn't specified.
+            if self.bootloader == None and self._is_v3:
+                for bootloader in self.get_bootloader_list():
+                    self.set_bootloader(bootloader)
+                    self.validate_bootloader_fields()
+                self.set_bootloader(None)
+            else:
+                self.validate_bootloader_fields()
+
             self._validate_serial_tty()
             self._validate_kernel_addr()
             self._validate_initrd_addr()
@@ -210,19 +237,12 @@ 
             self._validate_root_min_size()
             self._validate_loader_min_size()
             self._validate_loader_start()
-            self._validate_spl_package()
-            self._validate_spl_file()
             self._validate_vmlinuz()
             self._validate_initrd()
             self._validate_dtb_file()
             self._validate_mmc_id()
             self._validate_extra_boot_options()
             self._validate_boot_script()
-            self._validate_bootloader_file_in_boot_part()
-            self._validate_bootloader_dd()
-            self._validate_spl_in_boot_part()
-            self._validate_spl_dd()
-            self._validate_env_dd()
             self._validate_extra_serial_opts()
             self._validate_snowball_startup_files_config()
             self._validate_samsung_bl1_start()
@@ -342,6 +362,8 @@ 
         """Get an option inside the current bootloader section."""
         if self._is_v3:
             if not self.bootloader:
+                if self.allow_unset_bootloader:
+                    return None
                 raise ValueError("bootloader not set.")
             if not isinstance(key, list):
                 keys = [key]

=== modified file 'linaro_image_tools/hwpack/testing.py'
--- linaro_image_tools/hwpack/testing.py	2012-06-13 14:53:32 +0000
+++ linaro_image_tools/hwpack/testing.py	2012-08-29 15:01:41 +0000
@@ -336,18 +336,18 @@ 
 
     def __init__(self, metadata, packages, sources,
                  packages_without_content=None,
-                 package_spec=None):
+                 package_spec=None, format="1.0"):
         self.metadata = metadata
         self.packages = packages
         self.sources = sources
         self.packages_without_content = packages_without_content or []
         self.package_spec = package_spec
+        self.format = format + "\n"
 
     def match(self, path):
-        tf = tarfile.open(name=path, mode="r:gz")
-        try:
+        with tarfile.open(name=path, mode="r:gz") as tf:
             matchers = []
-            matchers.append(HardwarePackHasFile("FORMAT", content="1.0\n"))
+            matchers.append(HardwarePackHasFile("FORMAT", content=self.format))
             matchers.append(HardwarePackHasFile(
                 "metadata", content=str(self.metadata)))
             manifest_lines = []
@@ -398,8 +398,6 @@ 
             matchers.append(HardwarePackHasFile(
                 "sources.list.d.gpg", type=tarfile.DIRTYPE))
             return MatchesAll(*matchers).match(tf)
-        finally:
-            tf.close()
 
     def __str__(self):
         return "Is a valid hardware pack."

=== modified file 'linaro_image_tools/hwpack/tests/test_config.py'
--- linaro_image_tools/hwpack/tests/test_config.py	2012-07-26 14:16:31 +0000
+++ linaro_image_tools/hwpack/tests/test_config.py	2012-08-28 17:34:16 +0000
@@ -66,8 +66,8 @@ 
         e = self.assertRaises(HwpackConfigError, f, *args, **kwargs)
         self.assertEqual(contents, str(e))
 
-    def assertValidationError(self, contents, config):
-        self.assertConfigError(contents, config.validate)
+    def assertValidationError(self, contents, config, function="validate"):
+        self.assertConfigError(contents, config.get_option(function))
 
     def test_validate_no_hwpack_section(self):
         config = self.get_config("")
@@ -219,7 +219,8 @@ 
                                      "u-boot-file = u-boot.bin\n" \
                                      "partition_layout = bootfs_rootfs\n"\
                                      "kernel_file = ~~\n")
-        self.assertValidationError("Invalid path: ~~", config)
+        self.assertValidationError("Invalid path: ~~", config,
+                                   "_validate_vmlinuz")
 
     def test_validate_empty_kernel_file(self):
         config = self.get_config(self.valid_start_v2 +
@@ -228,7 +229,7 @@ 
                                      "partition_layout = bootfs_rootfs\n"\
                                      "kernel_file = \n")
         self.assertValidationError("No kernel_file in the [hwpack] section",
-                                   config)
+                                   config, "_validate_vmlinuz")
 
     def test_validate_invalid_initrd_file(self):
         config = self.get_config(
@@ -238,7 +239,8 @@ 
                 "partition_layout = bootfs_rootfs\n"\
                 "kernel_file = boot/vmlinuz-3.0.0-1002-linaro-omap\n"\
                 "initrd_file = ~~\n")
-        self.assertValidationError("Invalid path: ~~", config)
+        self.assertValidationError("Invalid path: ~~", config,
+                                   "_validate_initrd")
 
     def test_validate_empty_initrd_file(self):
         config = self.get_config(
@@ -249,7 +251,7 @@ 
                 "kernel_file = boot/vmlinuz-3.0.0-1002-linaro-omap\n"\
                 "initrd_file = \n")
         self.assertValidationError("No initrd_file in the [hwpack] section",
-                                   config)
+                                   config, "_validate_initrd")
 
     def test_validate_invalid_boot_script(self):
         config = self.get_config(
@@ -309,7 +311,8 @@ 
             "Undefined partition layout %s in the [%s] section. "
             "Valid partition layouts are %s."
             % (partition_layout, 'hwpack',
-               ", ".join(DEFINED_PARTITION_LAYOUTS)), config)
+               ", ".join(DEFINED_PARTITION_LAYOUTS)), config,
+            "_validate_partition_layout")
 
     def test_validate_wired_interfaces(self):
         self.assertTrue("XXX What is an invalid interface name?")
@@ -350,13 +353,15 @@ 
             self.valid_start_v2 +
             "u_boot_package = u-boot-linaro-s5pv310\n" \
                 "u_boot_file = u-boot.bin\nserial_tty=tty\n")
-        self.assertValidationError("Invalid serial tty: tty", config)
+        self.assertValidationError("Invalid serial tty: tty", config,
+                                   "_validate_serial_tty")
         config = self.get_config(
             self.valid_start_v2 +
             "u_boot_package = u-boot-linaro-s5pv310\n" \
                 "u_boot_file = u-boot.bin\n" \
                 "serial_tty=ttxSAC1\n")
-        self.assertValidationError("Invalid serial tty: ttxSAC1", config)
+        self.assertValidationError("Invalid serial tty: ttxSAC1", config,
+                                   "_validate_serial_tty")
 
     def test_validate_mmc_id(self):
         config = self.get_config(self.valid_complete_v2 +

=== modified file 'linaro_image_tools/hwpack/tests/test_config_v3.py'
--- linaro_image_tools/hwpack/tests/test_config_v3.py	2012-07-30 16:56:12 +0000
+++ linaro_image_tools/hwpack/tests/test_config_v3.py	2012-08-28 17:34:16 +0000
@@ -315,6 +315,23 @@ 
         config._validate_bootloader_file_in_boot_part()
         self.assertEqual(config.bootloader_file_in_boot_part, "yes")
 
+    def test_multiple_bootloaders(self):
+        config = self.get_config(
+            self.valid_start_v3 +
+            "bootloaders:\n"
+            " u_boot:\n"
+            "  in_boot_part: No\n"
+            " anotherboot:\n"
+            "  in_boot_part: Yes\n")
+
+        config.set_bootloader("u_boot")
+        config._validate_bootloader_file_in_boot_part()
+        self.assertEqual(config.bootloader_file_in_boot_part, "no")
+
+        config.set_bootloader("anotherboot")
+        config._validate_bootloader_file_in_boot_part()
+        self.assertEqual(config.bootloader_file_in_boot_part, "yes")
+
     def test_validate_serial_tty(self):
         config = self.get_config(self.valid_start_v3 + "serial_tty: tty\n")
         self.assertValidationError("Invalid serial tty: tty",

=== modified file 'linaro_image_tools/hwpack/tests/test_script.py'
--- linaro_image_tools/hwpack/tests/test_script.py	2012-06-13 14:26:02 +0000
+++ linaro_image_tools/hwpack/tests/test_script.py	2012-08-29 15:01:41 +0000
@@ -31,8 +31,11 @@ 
     DummyFetchedPackage,
     IsHardwarePack,
     )
+from linaro_image_tools.hwpack.config import Config
 from linaro_image_tools.testing import TestCaseWithFixtures
 from linaro_image_tools.utils import find_command
+import os
+from StringIO import StringIO
 
 
 class ScriptTests(TestCaseWithFixtures):
@@ -40,7 +43,26 @@ 
 
     def setUp(self):
         super(ScriptTests, self).setUp()
-        self.script_path = find_command("linaro-hwpack-create")
+
+        # Work out root of checkout.
+        # We do this here because when running in PyCharm
+        # the assumption in find_command that os.path.isabs(__file__) is
+        # only true when not running from a Bazaar checkout is broken.
+        # Thankfully find_command allows us to work around this by specifying
+        # prefer_dir.
+        dir = os.path.dirname(__file__)
+        while True:
+            path = os.path.join(dir, "linaro-hwpack-create")
+            if dir == "/" or dir == "":
+                # Didn't find linaro-media-create. Continue as if we haven't
+                # tried to work out prefer_dir.
+                dir = None
+                break
+            if os.path.exists(path) and os.access(path, os.X_OK):
+                break
+            dir = os.path.split(dir)[0]
+
+        self.script_path = find_command("linaro-hwpack-create", prefer_dir=dir)
         self.useFixture(ChdirToTempdirFixture())
 
     def run_script(self, args, expected_returncode=0):
@@ -92,6 +114,70 @@ 
                 {"ubuntu": source.sources_entry},
                 package_spec=package_name))
 
+    def test_builds_a_v3_hwpack_from_config_with_2_bootloaders(self):
+        config_v3 = ("format: 3.0\n"
+                     "name: ahwpack\n"
+                     "architectures: armel\n"
+                     "serial_tty: ttySAC1\n"
+                     "partition_layout:\n"
+                     " - bootfs_rootfs\n"
+                     "boot_script: boot.scr\n"
+                     "extra_serial_options:\n"
+                     "  - console=tty0\n"
+                     "  - console=ttyO2,115200n8\n"
+                     "mmc_id: 0:1\n"
+                     "kernel_file: boot/vmlinuz-*-linaro-omap\n"
+                     "initrd_file: boot/initrd.img-*-linaro-omap\n"
+                     "dtb_file: boot/dt-*-linaro-omap/omap4-panda.dtb\n"
+                     "packages:\n"
+                     " - %s\n"
+                     " - %s\n")
+        bootloader_config = ("  package: %s\n"
+                             "  in_boot_part: %s\n"
+                             "  extra_boot_options:\n"
+                             "   - earlyprintk\n"
+                             "   - fixrtc\n"
+                             "   - nocompcache\n"
+                             "   - vram=48M\n"
+                             "   - omapfb.vram=0:24M\n"
+                             "   - mem=456M@0x80000000\n"
+                             "   - mem=512M@0xA0000000\n")
+
+        config_v3 += ("bootloaders:\n"
+                      " u_boot:\n" + bootloader_config +
+                      " u_boot_2:\n" + bootloader_config)
+
+        config_v3 += ("sources:\n"
+                      " ubuntu: %s\n")
+
+        package_names = ['foo', 'bar']
+        available_packages = []
+        for package_name in package_names:
+            available_packages.append(
+                DummyFetchedPackage(package_name, "1.1", architecture="armel"))
+        source = self.useFixture(AptSourceFixture(available_packages))
+
+        config_v3 = config_v3 % (package_names[0], package_names[1],
+                                 package_names[0], "True",
+                                 package_names[1], "False",
+                                 source.sources_entry)
+
+        config_file_fixture = self.useFixture(ConfigFileFixture(config_v3))
+        self.run_script([config_file_fixture.filename, "1.0"])
+
+        # We now need a real config object to test against the configuration
+        # in the hardware pack we have created.
+        config = Config(StringIO(config_v3))
+        config.set_bootloader("u_boot")
+        metadata = Metadata.from_config(config, "1.0", "armel")
+        self.assertThat(
+            "hwpack_ahwpack_1.0_armel.tar.gz",
+            IsHardwarePack(
+                metadata, available_packages,
+                {"ubuntu": source.sources_entry},
+                package_spec=",".join(package_names),
+                format="3.0"))
+
     def test_log_output(self):
         package_name = 'foo'
         architecture = 'armel'