From patchwork Thu Aug 30 10:14:11 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: James Tunnicliffe X-Patchwork-Id: 11037 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id 7C57023E52 for ; Thu, 30 Aug 2012 10:14:15 +0000 (UTC) Received: from mail-iy0-f180.google.com (mail-iy0-f180.google.com [209.85.210.180]) by fiordland.canonical.com (Postfix) with ESMTP id EF58CA1884B for ; Thu, 30 Aug 2012 10:13:39 +0000 (UTC) Received: by iafj25 with SMTP id j25so2706592iaf.11 for ; Thu, 30 Aug 2012 03:14:14 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-forwarded-to:x-forwarded-for:delivered-to:received-spf :content-type:mime-version:x-launchpad-project:x-launchpad-branch :x-launchpad-message-rationale:x-launchpad-branch-revision-number :x-launchpad-notification-type:to:from:subject:message-id:date :reply-to:sender:errors-to:precedence:x-generated-by :x-launchpad-hash:x-gm-message-state; bh=kKhDIeWU2hjdQERQbcftp2ATMB0IuKMdbNBU5lX7RFs=; b=o77YAP79JIKxfp5wiW6vpAdFTR0b/q0qYXZl2n/OZdB6VC0I04g7rehvTaOBVcoPqi Bzb41rI66y/fwQEmYTxc6cVc/FBreKTQrhw5wNn9QDtL4H8VoKMA6OwMhElueOgbYVgu LBAv2FxR6wH+t25mzl6Hqy8e67hS3/SY5NLL0YZ9EF74Yap0t0UZKrQ+ZI8t0x2gatCZ bRc9GRTq9uz/FvX2/0rMxuM6QpNp88VPgfcHKADGp72hXx8c3AtlVgFl8fXBAqhq7CpI IhFDZTNC4/0L6TTzp6UxjW36OmpS7ofZW13dGlfRreJ5Q4Nc8s4v03rMbKXRCTX5AcWQ SScw== Received: by 10.50.242.3 with SMTP id wm3mr176070igc.0.1346321654135; Thu, 30 Aug 2012 03:14:14 -0700 (PDT) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.50.184.232 with SMTP id ex8csp3427igc; Thu, 30 Aug 2012 03:14:13 -0700 (PDT) Received: by 10.216.135.148 with SMTP id u20mr2530921wei.137.1346321651962; Thu, 30 Aug 2012 03:14:11 -0700 (PDT) Received: from indium.canonical.com (indium.canonical.com. [91.189.90.7]) by mx.google.com with ESMTPS id v56si1689109web.77.2012.08.30.03.14.11 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 30 Aug 2012 03:14:11 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of bounces@canonical.com designates 91.189.90.7 as permitted sender) client-ip=91.189.90.7; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of bounces@canonical.com designates 91.189.90.7 as permitted sender) smtp.mail=bounces@canonical.com Received: from ackee.canonical.com ([91.189.89.26]) by indium.canonical.com with esmtp (Exim 4.71 #1 (Debian)) id 1T71lL-0006DY-34 for ; Thu, 30 Aug 2012 10:14:11 +0000 Received: from ackee.canonical.com (localhost [127.0.0.1]) by ackee.canonical.com (Postfix) with ESMTP id 0934FE0108 for ; Thu, 30 Aug 2012 10:14:11 +0000 (UTC) MIME-Version: 1.0 X-Launchpad-Project: linaro-image-tools X-Launchpad-Branch: ~linaro-image-tools/linaro-image-tools/trunk X-Launchpad-Message-Rationale: Subscriber X-Launchpad-Branch-Revision-Number: 554 X-Launchpad-Notification-Type: branch-revision To: Linaro Patch Tracker From: noreply@launchpad.net Subject: [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> Date: Thu, 30 Aug 2012 10:14:11 -0000 Reply-To: noreply@launchpad.net Sender: bounces@canonical.com Errors-To: bounces@canonical.com Precedence: bulk X-Generated-By: Launchpad (canonical.com); Revision="15877"; Instance="launchpad-lazr.conf" X-Launchpad-Hash: d4a74c2d03acb8d262fc27f3d0d1fc224b4ea08e X-Gm-Message-State: ALoCoQkpIOvVxYPMLnv3mpKsclg4LOAA2Q5WMQG/ZvfUvwZLXRZkckazn7FAX7FU5FxmnQZKz+ob 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 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 === 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'