Merge lp:~dooferlad/linaro-image-tools/copy-files-new-syntax into lp:linaro-image-tools/11.11

[James Tunnicliffe]

Have addressed Paul's review.

Tests run:
testr run
Built image using 12.06 release Nano image (V2 hwpack). Booted on Pandaboard.
Built V3 hwpack with copy_files entry and two bootloaders defined. Combined with 12.06 Nano image. Booted on Pandaboard.

----
Adds support for copy_files as specified in https://wiki.linaro.org/HardwarePacksV3

Unfortunately had to roll back some previous changes to make this work in a nice way.

[Paul Sokolovsky (pfalcon)]

Just for the record, I don't think such extreme refactoring was required, and it being started, is not complete - instead of moving entire file processing to be package based and happen on the level of linaro-media-create, this still leaves bootloader.file, bootloader.spl_file as it was, just flips copy_files handling - instead of being done consistent with how bootloader.file/bootloader.spl_file works, it is now package based, but that means we have file-extraction code spread around both linaro-hwpack-create and linaro-media-create.

But for the purpose of having it done, it's ok.

(more comments to come.)

[Paul Sokolovsky (pfalcon)]

It seems that EXTRACT_FILES dict is no longer used by this code, then it should be removed, possibly propagating that recursively (i.e. if it used symbol defined elsewhere, and those symbols are not used either, they should be removed too).

review: Needs Fixing

[Paul Sokolovsky (pfalcon)]

It's not really clear what .get_last_used_keys() does, suggested to add more details docstring.

review: Needs Fixing

[Paul Sokolovsky (pfalcon)]

call_for_all_boards_and_bootloaders(self, function) - somehow I got used to such functions be called foreach_board_and_bootloader() , which hints that it implements enumeration loop, though that's of course just a preference.

[Paul Sokolovsky (pfalcon)]

[package for package in copy_files] - that looks a bit strange. Was "copy_files[:]" meant instead (make a copy of list)?

[Paul Sokolovsky (pfalcon)]

> [package for package in copy_files] - that looks a bit strange. Was "copy_files[:]" meant instead (make a copy of list)?

Ah, I see, that's unobvious way to do copy_files.keys(). Btw, I missed moment when they allowed iteration on dict directly, and that started to mean iteration over its keys. I still think that doing it explicitly ("for i in a.iterkeys()") is more clear.

[Paul Sokolovsky (pfalcon)]

> is more clear

And here's example why:

http://stackoverflow.com/questions/3294889/iterating-over-dictionaries-for-loops-in-python

[Paul Sokolovsky (pfalcon)]

> if not dest_path.startswith("/boot"):

Great, thanks for implementing this handling.

[Paul Sokolovsky (pfalcon)]

342 - for expected_file, contents in expected_files:
343 - self.assertThat(
344 - tf, TarfileHasFile(expected_file, content=contents))
345 + stored_package_names = [p.name for p in builder.packages]
346 + for package_name in package_names:
347 + self.assertIn(package_name, stored_package_names)

I'm again not sure about this change - old version did check that file comes from the expected package and not some else, I don't the new does.

review: Needs Fixing

[James Tunnicliffe (dooferlad)]

On 11 September 2012 10:37, Paul Sokolovsky <email address hidden> wrote:
01234567890123456789012345678901234567890123456789012345678901234567890123456789
> Just for the record, I don't think such extreme refactoring was required,
> and it being started, is not complete - instead of moving entire file
> processing to be package based and happen on the level of
> linaro-media-create, this still leaves bootloader.file, bootloader.spl_file
> as it was, just flips copy_files handling - instead of being done consistent
> with how bootloader.file/bootloader.spl_file works, it is now package based,
> but that means we have file-extraction code spread around both
> linaro-hwpack-create and linaro-media-create.
>
> But for the purpose of having it done, it's ok.

I agree that we could move all file extraction to the hardware pack
install stage, and it is something I considered. I think it is worth
doing as part of a separate update because it would change existing
functionality (well, change it more than has already been done as part
of supporting multiple boards and bootloaders). I don't know if anyone
is replacing bootloader binaries in a hardware pack after they have
created it with linaro-hwpack-create. If they are, we need to discuss
changes to the hardware pack format with them.

I didn't make the decision to start refactoring lightly. I only did
when it became clear that I was going to have to write some rather
ugly code to support the new copy_files format as well as
re-implementing storing files that exist in a package - it seemed that
just using packages would be a lot cleaner!

I expect that the reason for U-Boot and the second stage bootloader
binaries being in a separate folder in the hardware pack it is
historical - I could not see any mention of the reasons in the code!
It adds unnecessary complication to the hardware pack create and
install stages and now that we support multiple bootloaders and boards
in a single hardware pack, we have already missed some bugs around it,
which you fixed (thanks!)

Thanks for the other comments. I will address them now and get an
updated branch ready.

--
James Tunnicliffe

[Paul Sokolovsky (pfalcon)]

Great work, thanks for updating and testing!

review: Approve

[Fathi Boudra (fboudra)]

See https://bugs.launchpad.net/linaro-image-tools/+bug/1049529

It breaks precise-armhf-pre-built-images-panda job:
https://ci.linaro.org/jenkins/job/precise-armhf-pre-built-images-panda/64/

=== modified file 'linaro_image_tools/hwpack/builder.py'
--- linaro_image_tools/hwpack/builder.py	2012-09-03 17:12:23 +0000
+++ linaro_image_tools/hwpack/builder.py	2012-09-11 14:33:43 +0000
@@ -37,29 +37,13 @@
     )
 
 from linaro_image_tools.hwpack.hwpack_fields import (
-    FILE_FIELD,
     PACKAGE_FIELD,
-    SPL_FILE_FIELD,
     SPL_PACKAGE_FIELD,
-    COPY_FILES_FIELD,
 )
 
 # The fields that hold packages to be installed.
 PACKAGE_FIELDS = [PACKAGE_FIELD, SPL_PACKAGE_FIELD]
-# Specification of files (boot related) to extract:
-# <field_containing_filepaths>: (<take_files_from_package>,
-#                                <put_into_this_hwpack_subdir>)
-# if <put_into_this_hwpack_subdir> is None, it will be <bootloader_name> for
-# global bootloader, or <board>-<bootloader_name> for board-specific
-# bootloader
-EXTRACT_FILES = {FILE_FIELD: (PACKAGE_FIELD, None),
-                 SPL_FILE_FIELD: (SPL_PACKAGE_FIELD, None),
-                 COPY_FILES_FIELD: (PACKAGE_FIELD, None)}
-
-
 logger = logging.getLogger(__name__)
-
-
 LOCAL_ARCHIVE_LABEL = 'hwpack-local'
 
 
@@ -166,57 +150,72 @@
         # Eliminate duplicates.
         return list(set(boot_packages))
 
-    def extract_bootloader_files(self, board, bootloader_name,
-                                 bootloader_conf):
-        for key, value in bootloader_conf.iteritems():
-            if key in EXTRACT_FILES:
-                package_field, dest_path = EXTRACT_FILES[key]
-                if not dest_path:
-                    dest_path = bootloader_name
-                    if board:
-                        dest_path += "-" + board
-                # Dereference package field to get actual package name
-                package = bootloader_conf.get(package_field)
-                src_files = value
-
-                # Process scalar and list fields consistently below
-                field_value_scalar = False
-                if type(src_files) != type([]):
-                    src_files = [src_files]
-                    field_value_scalar = True
-
-                package_ref = self.find_fetched_package(
-                                self.packages, package)
-                added_files = []
-                for f in src_files:
-                    added_files.append(self.add_file_to_hwpack(
-                                        package_ref, f, dest_path))
-                # Store within-hwpack file paths with the same
-                # scalar/list type as original field.
-                if field_value_scalar:
-                    assert len(added_files) == 1
-                    added_files = added_files[0]
-                bootloader_conf[key] = added_files
-
-    def extract_files(self, config_dictionary, is_bootloader_config,
-                      board=None):
-        """Extract (boot) files based on EXTRACT_FILES spec and put
-        them into hwpack."""
-        self.remove_packages = []
-        if is_bootloader_config:
-            for bootl_name, bootl_conf in config_dictionary.iteritems():
-                self.extract_bootloader_files(board, bootl_name, bootl_conf)
-        else:
-            # This is board config
-            for board, board_conf in config_dictionary.iteritems():
-                bootloaders = board_conf['bootloaders']
-                self.extract_files(bootloaders, True, board)
-
-        # Clean up no longer needed packages.
-        for package in self.remove_packages:
-            if package in self.packages:
-                self.packages.remove(package)
-        self.remove_packages = []
+    def do_extract_file(self, package, source_path, dest_path):
+        """Extract specified file from package to dest_path."""
+        package_ref = self.find_fetched_package(self.packages, package)
+        return self.add_file_to_hwpack(package_ref, source_path, dest_path)
+
+    def do_extract_files(self):
+        """Go through a bootloader config, search for files to extract."""
+        base_dest_path = ""
+        if self.config.board:
+            base_dest_path = self.config.board
+        base_dest_path = os.path.join(base_dest_path, self.config.bootloader)
+        # Extract bootloader file
+        if self.config.bootloader_package and self.config.bootloader_file:
+            dest_path = os.path.join(base_dest_path,
+                            os.path.dirname(self.config.bootloader_file))
+            self.do_extract_file(self.config.bootloader_package,
+                                 self.config.bootloader_file,
+                                 dest_path)
+
+        # Extract SPL file
+        if self.config.spl_package and self.config.spl_file:
+            dest_path = os.path.join(base_dest_path,
+                            os.path.dirname(self.config.spl_file))
+            self.do_extract_file(self.config.spl_package,
+                                 self.config.spl_file,
+                                 dest_path)
+
+    def foreach_boards_and_bootloaders(self, function):
+        """Call function for each board + bootloader combination in metadata"""
+        if self.config.bootloaders is not None:
+            for bootloader in self.config.bootloaders:
+                self.config.set_board(None)
+                self.config.set_bootloader(bootloader)
+                function()
+
+        if self.config.boards is not None:
+            for board in self.config.boards:
+                if self.config.bootloaders is not None:
+                    for bootloader in self.config.bootloaders:
+                        self.config.set_board(board)
+                        self.config.set_bootloader(bootloader)
+                        function()
+
+    def extract_files(self):
+        """Find bootloaders in config that may contain files to extract."""
+        if float(self.config.format.format_as_string) < 3.0:
+            # extract files was introduced in version 3 configurations and is
+            # a null operation for earlier configuration files
+            return
+
+        self.foreach_boards_and_bootloaders(self.do_extract_files)
+
+    def do_find_copy_files_packages(self):
+        """Find packages referenced by copy_files (single board, bootloader)"""
+        copy_files = self.config.bootloader_copy_files
+        if copy_files:
+            self.copy_files_packages.extend(copy_files.keys())
+
+    def find_copy_files_packages(self):
+        """Find all packages referenced by copy_files sections in metadata."""
+        self.copy_files_packages = []
+        self.foreach_boards_and_bootloaders(
+            self.do_find_copy_files_packages)
+        packages = self.copy_files_packages
+        del(self.copy_files_packages)
+        return packages
 
     def build(self):
         for architecture in self.config.architectures:
@@ -242,6 +241,8 @@
                     if self.config.boards is not None:
                         self.packages.extend(self.find_bootloader_packages(
                                                 self.config.boards))
+
+                    self.packages.extend(self.find_copy_files_packages())
                 else:
                     if self.config.bootloader_package is not None:
                         self.packages.append(self.config.bootloader_package)
@@ -268,14 +269,9 @@
                         # On a v3 hwpack, all the values we need to check are
                         # in the bootloaders and boards section, so we loop
                         # through both of them changing what is necessary.
+
                         if self.config.format.format_as_string == '3.0':
-                            if self.config.bootloaders is not None:
-                                self.extract_files(self.config.bootloaders,
-                                                   True)
-                                metadata.bootloaders = self.config.bootloaders
-                            if self.config.boards is not None:
-                                self.extract_files(self.config.boards, False)
-                                metadata.boards = self.config.boards
+                            self.extract_files()
                         else:
                             bootloader_package = None
                             if self.config.bootloader_file is not None:

=== modified file 'linaro_image_tools/hwpack/config.py'
--- linaro_image_tools/hwpack/config.py	2012-08-30 10:39:02 +0000
+++ linaro_image_tools/hwpack/config.py	2012-09-11 14:33:43 +0000
@@ -21,6 +21,7 @@
 
 import ConfigParser
 from operator import attrgetter
+import os
 import re
 import string
 import yaml
@@ -116,6 +117,8 @@
     translate_v2_to_v3[BOOTLOADER_IN_BOOT_PART_KEY] = IN_BOOT_PART_FIELD
     BOOTLOADER_DD_KEY = 'u_boot_dd'
     translate_v2_to_v3[BOOTLOADER_DD_KEY] = DD_FIELD
+    last_used_keys = []
+    board = None
 
     def __init__(self, fp, bootloader=None, board=None,
                  allow_unset_bootloader=False):
@@ -354,9 +357,56 @@
         return self._get_bootloader_option(SPL_IN_BOOT_PART_FIELD)
 
     @property
-    def boot_copy_files(self):
-        """Extra files to copy to boot partition."""
-        return self._get_bootloader_option(COPY_FILES_FIELD)
+    def bootloader_copy_files(self):
+        """Extra files to copy to boot partition.
+
+        This can be stored in several formats. We always present in a common
+        one: {source_package: [{source_file_path: dest_file_path}].
+        dest_file_path (in the above example) is always absolute.
+        """
+        #copy_files:
+        #  source_package:
+        #   - source_file_path : dest_file_path
+        #   - source_file_without_explicit_destination
+        #copy_files:
+        # - file1
+        # - file2: dest_path
+        #
+        # Note that the list of files is always that - a list.
+
+        copy_files = self._get_bootloader_option(COPY_FILES_FIELD)
+
+        if copy_files is None:
+            return None
+
+        if not isinstance(copy_files, dict):
+            copy_files = {self.bootloader_package: copy_files}
+
+        for package in copy_files:
+            new_list = []
+            for value in copy_files[package]:
+                if not isinstance(value, dict):
+                    dest_path = "/boot"
+                    source_path = value
+                else:
+                    if len(value.keys()) > 1:
+                        raise HwpackConfigError("copy_files entry found with"
+                            "more than one destination")
+                    source_path = value.keys()[0]
+                    dest_path = value[source_path]
+
+                if not dest_path.startswith("/boot"):
+                    # Target path should be relative, or start with /boot - we
+                    # don't support to copying to anywhere other than /boot.
+                    if dest_path[0] == "/":
+                        raise HwpackConfigError("copy_files destinations must"
+                            "be relative to /boot or start with /boot.")
+                    dest_path = os.path.join("/boot", dest_path)
+
+                new_list.append({source_path: dest_path})
+            copy_files[package] = new_list
+
+        return copy_files
 
     @property
     def spl_dd(self):
@@ -426,8 +476,22 @@
             key = self._v2_key_to_v3(key)
             if result is not None:
                 result = result.get(key, None)
+        self.last_used_keys = keys
         return result
 
+    def get_last_used_keys(self):
+        """Used so you can work out which boards + boot loader was used.
+
+        Configuration data is stored in a dictionary. This returns a list of
+        keys used to traverse into the dictionary the last time an item was
+        looked up.
+
+        This can be used to see where a bit of information came from - we
+        store data that may be indexed differently depending on which board
+        and bootloader are set.
+        """
+        return self.last_used_keys
+
     def get_option(self, name):
         """Return the value of an attribute by name.
 

=== modified file 'linaro_image_tools/hwpack/tests/test_builder.py'
--- linaro_image_tools/hwpack/tests/test_builder.py	2012-09-04 10:42:27 +0000
+++ linaro_image_tools/hwpack/tests/test_builder.py	2012-09-11 14:33:43 +0000
@@ -31,7 +31,7 @@
     HardwarePackBuilder,
     logger as builder_logger,
     )
-from linaro_image_tools.hwpack.config import HwpackConfigError, Config
+from linaro_image_tools.hwpack.config import HwpackConfigError
 from linaro_image_tools.hwpack.hardwarepack import Metadata
 from linaro_image_tools.hwpack.packages import (
     FetchedPackage,
@@ -55,7 +55,6 @@
     MockSomethingFixture,
     MockCmdRunnerPopenFixture,
     )
-from StringIO import StringIO
 
 
 class ConfigFileMissingTests(TestCase):
@@ -431,11 +430,17 @@
             Equals("Local package 'bar' not included"))
 
     def test_global_and_board_bootloader(self):
-        package_names = ['package0', 'package1']
-        files = {package_names[0]:
-                     ["usr/lib/u-boot/omap4_panda/u-boot.img",
-                      "usr/share/doc/u-boot-linaro-omap4-panda/copyright"],
-                 package_names[1]: ["usr/lib/u-boot/omap4_panda/u-boot.img"]}
+        package_names = ['package0', 'package1', 'package2', 'package3']
+        files = {
+            package_names[0]:
+                ["usr/lib/u-boot/omap4_panda/u-boot.img",
+                 "usr/share/doc/u-boot-linaro-omap4-panda/copyright"],
+            package_names[1]:
+                ["usr/lib/u-boot/omap4_panda/u-boot.img",
+                 "some/path/file"],
+            package_names[2]: [],
+            package_names[3]: [],
+        }
 
         config_v3 = self.config_v3 + "\n".join([
             "bootloaders:",
@@ -443,13 +448,17 @@
             self.bootloader_config,
             "  file: " + files[package_names[0]][0],
             "  copy_files:",
-            "   - " + files[package_names[0]][1],
+            "    " + package_names[2] + ":",
+            "      - some_file",
             "boards:",
             " board1:",
             "  bootloaders:",
             "   u_boot:",
             "    package: %s",
             "    file: " + files[package_names[1]][0],
+            "    copy_files:",
+            "      " + package_names[3] + ":",
+            "       - some_file",
             "    in_boot_part: true",
             "sources:",
             " ubuntu: %s"])
@@ -480,10 +489,6 @@
 
         config_file_fixture = self.useFixture(ConfigFileFixture(config_v3))
 
-        # Parse the config
-        config = Config(StringIO(config_v3))
-        config.set_bootloader("u_boot")
-
         # Build a hardware pack
         builder = HardwarePackBuilder(
             config_file_fixture.filename, "1.0",
@@ -491,6 +496,9 @@
              for package in available_packages])
 
         builder.build()
+        stored_package_names = [p.name for p in builder.packages]
+        for package_name in package_names:
+            self.assertIn(package_name, stored_package_names)
 
         # Read the contents of the hardware pack, making sure it is as expected
         tf = tarfile.open("hwpack_ahwpack_1.0_armel.tar.gz", mode="r:gz")
@@ -499,11 +507,9 @@
         # files this is "<package_name> <originating path>" so they can be
         # uniquely identified.
         expected_files = [
-            ("u_boot/u-boot.img",
+            ("u_boot/" + files[package_names[0]][0],
              package_names[0] + " " + files[package_names[0]][0]),
-            ("u_boot/copyright",
-             package_names[0] + " " + files[package_names[0]][1]),
-            ("u_boot-board1/u-boot.img",
+            ("board1/u_boot/" + files[package_names[1]][0],
              package_names[1] + " " + files[package_names[1]][0])]
 
         for expected_file, contents in expected_files:

=== modified file 'linaro_image_tools/media_create/boards.py'
--- linaro_image_tools/media_create/boards.py	2012-09-05 09:29:14 +0000
+++ linaro_image_tools/media_create/boards.py	2012-09-11 14:33:43 +0000
@@ -37,6 +37,7 @@
 import string
 import logging
 from linaro_image_tools.hwpack.config import Config
+from linaro_image_tools.hwpack.builder import PackageUnpacker
 
 from parted import Device
 
@@ -128,6 +129,7 @@
         self.hwpack_tarfiles = []
         self.bootloader = bootloader
         self.board = board
+        self.tempdirs = {}
 
     class FakeSecHead(object):
         """ Add a fake section header to the metadata file.
@@ -162,9 +164,15 @@
         if self.tempdir is not None and os.path.exists(self.tempdir):
             shutil.rmtree(self.tempdir)
 
-    def get_field(self, field):
+        for name in self.tempdirs:
+            tempdir = self.tempdirs[name]
+            if tempdir is not None and os.path.exists(tempdir):
+                shutil.rmtree(tempdir)
+
+    def get_field(self, field, return_keys=False):
         data = None
         hwpack_with_data = None
+        keys = None
         for hwpack_tarfile in self.hwpack_tarfiles:
             metadata = hwpack_tarfile.extractfile(self.metadata_filename)
             lines = metadata.readlines()
@@ -181,9 +189,13 @@
                                                               new_data)
                     data = new_data
                     hwpack_with_data = hwpack_tarfile
+                    if return_keys:
+                        keys = parser.get_last_used_keys()
             except ConfigParser.NoOptionError:
                 continue
 
+        if return_keys:
+            return data, hwpack_with_data, keys
         return data, hwpack_with_data
 
     def get_format(self):
@@ -207,7 +219,8 @@
                            file reference(s)
         :return: path to a file or list of paths to files
         """
-        file_names, hwpack_tarfile = self.get_field(file_alias)
+        file_names, hwpack_tarfile, keys = self.get_field(file_alias,
+                                                          return_keys=True)
         if not file_names:
             return file_names
         single = False
@@ -215,7 +228,24 @@
             single = True
             file_names = [file_names]
         out_files = []
+
+        # Depending on if board and/or bootloader were used to look up the
+        # file we are getting, we need to prepend those names to the path
+        # to get the correct extracted file from the hardware pack.
+        config_names = [("board", "boards"), ("bootloader", "bootloaders")]
+        base_path = ""
+        if keys:
+            # If keys is non-empty, we have a V3 config option that was
+            # modified by the bootloader and/or boot option...
+            for name, key in config_names:
+                if self.get_field(name):
+                    value = self.get_field(name)[0]
+                    if keys[0] == key:
+                        base_path = os.path.join(base_path, value)
+                        keys = keys[1:]
+
         for f in file_names:
+            f = os.path.join(base_path, f)
             hwpack_tarfile.extract(f, self.tempdir)
             f = os.path.join(self.tempdir, f)
             out_files.append(f)
@@ -223,6 +253,104 @@
             return out_files[0]
         return out_files
 
+    def list_packages(self):
+        """Return list of (package names, TarFile object containing them)"""
+        packages = []
+        for tf in self.hwpack_tarfiles:
+            for name in tf.getnames():
+                if name.startswith("pkgs/") and name.endswith(".deb"):
+                    packages.append((tf, name))
+        return packages
+
+    def find_package_for(self, name, version=None, revision=None,
+                         architecture=None):
+        """Find a package that matches the name, version, rev and arch given.
+
+        Packages are named according to the debian specification:
+        http://www.debian.org/doc/manuals/debian-faq/ch-pkg_basics.en.html
+        <name>_<Version>-<DebianRevisionNumber>_<DebianArchitecture>.deb
+        DebianRevisionNumber seems to be optional.
+        Use this spec to return a package matching the requirements given.
+        """
+        for tar_file, package in self.list_packages():
+            file_name = os.path.basename(package)
+            dpkg_chunks = re.search("^(.+)_(.+)_(.+)\.deb$",
+                                    file_name)
+            assert dpkg_chunks, "Could not split package file name into"\
+                "<name>_<Version>_<DebianArchitecture>.deb"
+
+            pkg_name = dpkg_chunks.group(1)
+            pkg_version = dpkg_chunks.group(2)
+            pkg_architecture = dpkg_chunks.group(3)
+
+            ver_chunks = re.search("^(.+)-(.+)$", pkg_version)
+            if ver_chunks:
+                pkg_version = ver_chunks.group(1)
+                pkg_revision = ver_chunks.group(2)
+            else:
+                pkg_revision = None
+
+            if name != pkg_name:
+                continue
+            if version != None and str(version) != pkg_version:
+                continue
+            if revision != None and str(revision) != pkg_revision:
+                continue
+            if (architecture != None and
+                str(architecture) != pkg_architecture):
+                continue
+
+            # Got a matching package - return its path inside the tarball
+            return tar_file, package
+
+        # Failed to find a matching package - return None
+        return None
+
+    def get_file_from_package(self, file_path, package_name,
+                              package_version=None, package_revision=None,
+                              package_architecture=None):
+        """Extract named file from package specified by name, ver, rev, arch.
+
+        File is extracted from the package matching the given specification
+        to a temporary directory. The absolute path to the extracted file is
+        returned.
+        """
+
+        package_info = self.find_package_for(package_name,
+                                             package_version,
+                                             package_revision,
+                                             package_architecture)
+        if package_info is None:
+            return None
+        tar_file, package = package_info
+
+        # Avoid unpacking hardware pack more than once by assigning each one
+        # its own tempdir to unpack into.
+        # TODO: update logic that uses self.tempdir so we can get rid of this
+        # by sharing nicely.
+        if not package in self.tempdirs:
+            self.tempdirs[package] = tempfile.mkdtemp()
+        tempdir = self.tempdirs[package]
+
+        # We extract everything in the hardware pack so we don't have to worry
+        # about chasing links (extract a link, find where it points to, extract
+        # that...). This is slower, but more reliable.
+        tar_file.extractall(tempdir)
+        package_path = os.path.join(tempdir, package)
+
+        with PackageUnpacker() as self.package_unpacker:
+            extracted_file = self.package_unpacker.get_file(package_path,
+                                                            file_path)
+            after_tmp = re.sub(self.package_unpacker.tempdir, "",
+                               extracted_file).lstrip("/\\")
+            extract_dir = os.path.join(tempdir, "extracted",
+                                       os.path.dirname(after_tmp))
+            os.makedirs(extract_dir)
+            shutil.move(extracted_file, extract_dir)
+            extracted_file = os.path.join(extract_dir,
+                                          os.path.basename(extracted_file))
+        return extracted_file
+
 
 class BoardConfig(object):
     board = None
@@ -465,6 +593,13 @@
                 cls.SAMSUNG_V310_BL2_START = (cls.SAMSUNG_V310_ENV_START +
                                               cls.SAMSUNG_V310_ENV_LEN)
 
+            cls.bootloader_copy_files = cls.hardwarepack_handler.get_field(
+                "bootloader_copy_files")[0]
+
+            cls.bootloader = cls.hardwarepack_handler.get_field(
+                                "bootloader")
+            cls.board = board
+
     @classmethod
     def get_file(cls, file_alias, default=None):
         # XXX remove the 'default' parameter when V1 support is removed!
@@ -793,7 +928,6 @@
         bootloader_parts_dir = os.path.join(chroot_dir, parts_dir)
         cmd_runner.run(['mkdir', '-p', boot_disk]).wait()
         with partition_mounted(boot_partition, boot_disk):
-            boot_files = []
             with cls.hardwarepack_handler:
                 if cls.bootloader_file_in_boot_part:
                     # <legacy v1 support>
@@ -813,22 +947,48 @@
                     assert bootloader_bin is not None, (
                         "bootloader binary could not be found")
 
-                    boot_files.append(bootloader_bin)
-
-                copy_files = cls.get_file('boot_copy_files')
-                if copy_files:
-                    boot_files.extend(copy_files)
-
-                for f in boot_files:
                     proc = cmd_runner.run(
-                        ['cp', '-v', f, boot_disk], as_root=True)
+                        ['cp', '-v', bootloader_bin, boot_disk], as_root=True)
                     proc.wait()
 
+                # Handle copy_files field.
+                cls.copy_files(boot_disk)
+
             cls.make_boot_files(
                 bootloader_parts_dir, is_live, is_lowmem, consoles, chroot_dir,
                 rootfs_id, boot_disk, boot_device_or_file)
 
     @classmethod
+    def copy_files(cls, boot_disk):
+        """Handle the copy_files metadata field."""
+
+        # Extract anything specified by copy_files sections
+        # self.bootloader_copy_files is always of the form:
+        # {'source_package':
+        #  [
+        #   {'source_path': 'dest_path'}
+        #  ]
+        # }
+        if cls.bootloader_copy_files is None:
+            return
+
+        for source_package, file_list in cls.bootloader_copy_files.iteritems():
+            for file_info in file_list:
+                for source_path, dest_path in file_info.iteritems():
+                    source = cls.hardwarepack_handler.get_file_from_package(
+                        source_path, source_package)
+                    dest_path = dest_path.lstrip("/\\")
+                    dirname = os.path.dirname(dest_path)
+                    dirname = os.path.join(boot_disk, dirname)
+                    if not os.path.exists(dirname):
+                        cmd_runner.run(["mkdir", "-p", dirname],
+                                       as_root=True).wait()
+                    proc = cmd_runner.run(
+                        ['cp', '-v', source,
+                         os.path.join(boot_disk, dest_path)], as_root=True)
+                    proc.wait()
+
+    @classmethod
     def _get_kflavor_files(cls, path):
         """Search for kernel, initrd and optional dtb in path."""
         if cls.kernel_flavors is None:

=== modified file 'linaro_image_tools/media_create/tests/test_media_create.py'
--- linaro_image_tools/media_create/tests/test_media_create.py	2012-09-04 13:53:43 +0000
+++ linaro_image_tools/media_create/tests/test_media_create.py	2012-09-11 14:33:43 +0000
@@ -36,6 +36,7 @@
 from testtools import TestCase
 
 from linaro_image_tools import cmd_runner
+from linaro_image_tools.hwpack.packages import PackageMaker
 import linaro_image_tools.media_create
 from linaro_image_tools.media_create import (
     android_boards,
@@ -124,6 +125,7 @@
     )
 from linaro_image_tools.utils import find_command, preferred_tools_dir
 
+from linaro_image_tools.hwpack.testing import ContextManagerFixture
 
 chroot_args = " ".join(cmd_runner.CHROOT_ARGS)
 sudo_args = " ".join(cmd_runner.SUDO_ARGS)
@@ -309,36 +311,95 @@
             test_file = hp.get_file('bootloader_file')
             self.assertEquals(data, open(test_file, 'r').read())
 
-    def test_get_file_v3(self):
-        # Test that get_file() works as expected with hwpackv3 and
-        # supports its new file fields.
-        metadata = textwrap.dedent("""\
-        format: 3.0
-        name: ahwpack
-        version: 4
-        architecture: armel
-        origin: linaro
-        bootloaders:
-         u_boot:
-          file: a_file
-          copy_files:
-           - file1
-           - file2
-         uefi:
-          file: b_file
-        """)
-        files = {'FORMAT': '3.0\n', 'metadata': metadata,
-                 'a_file': 'a_file content', 'file1': 'file1 content',
-                 'file2': 'file2 content'}
-        tarball = self.add_to_tarball(files.items())
-        hp = HardwarepackHandler([tarball], bootloader='u_boot')
-        with hp:
-            test_file = hp.get_file('bootloader_file')
-            self.assertEquals(files['a_file'], open(test_file, 'r').read())
-            test_files = hp.get_file('boot_copy_files')
-            self.assertEquals(len(test_files), 2)
-            self.assertEquals(files['file1'], open(test_files[0], 'r').read())
-            self.assertEquals(files['file2'], open(test_files[1], 'r').read())
+    def test_list_packages(self):
+        metadata = ("format: 3.0\nname: ahwpack\nversion: 4\narchitecture: "
+                    "armel\norigin: linaro\n")
+        format = "3.0\n"
+        tarball = self.add_to_tarball([
+            ("FORMAT", format),
+            ("metadata", metadata),
+            ("pkgs/foo_1-1_all.deb", ''),
+            ("pkgs/bar_1-1_all.deb", ''),
+        ])
+
+        hp = HardwarepackHandler([tarball], board='panda', bootloader='uefi')
+        with hp:
+            packages = hp.list_packages()
+            names = [p[1] for p in packages]
+            self.assertIn('pkgs/foo_1-1_all.deb', names)
+            self.assertIn('pkgs/bar_1-1_all.deb', names)
+            self.assertEqual(len(packages), 2)
+
+    def test_find_package_for(self):
+        metadata = ("format: 3.0\nname: ahwpack\nversion: 4\narchitecture: "
+                    "armel\norigin: linaro\n")
+        format = "3.0\n"
+        tarball = self.add_to_tarball([
+            ("FORMAT", format),
+            ("metadata", metadata),
+            ("pkgs/foo_1-3_all.deb", ''),
+            ("pkgs/foo_2-5_arm.deb", ''),
+            ("pkgs/bar_1-3_arm.deb", ''),
+        ])
+
+        hp = HardwarepackHandler([tarball], board='panda', bootloader='uefi')
+        with hp:
+            self.assertEqual(hp.find_package_for("foo")[1],
+                             "pkgs/foo_1-3_all.deb")
+            self.assertEqual(hp.find_package_for("bar")[1],
+                             "pkgs/bar_1-3_arm.deb")
+            self.assertEqual(hp.find_package_for("foo", version=2)[1],
+                             "pkgs/foo_2-5_arm.deb")
+            self.assertEqual(hp.find_package_for("foo", version=2,
+                                                 revision=5)[1],
+                             "pkgs/foo_2-5_arm.deb")
+            self.assertEqual(hp.find_package_for("foo", version=2, revision=5,
+                                                 architecture="arm")[1],
+                             "pkgs/foo_2-5_arm.deb")
+            self.assertEqual(hp.find_package_for("foo", architecture="arm")[1],
+                             "pkgs/foo_2-5_arm.deb")
+            self.assertEqual(hp.find_package_for("foo", architecture="all")[1],
+                             "pkgs/foo_1-3_all.deb")
+
+    def test_get_file_from_package(self):
+        metadata = ("format: 3.0\nname: ahwpack\nversion: 4\narchitecture: "
+                    "armel\norigin: linaro\n")
+        format = "3.0\n"
+
+        names = ['package0', 'package1', 'package2']
+        files = {names[0]:
+                     ["usr/lib/u-boot/omap4_panda/u-boot.img",
+                      "usr/share/doc/u-boot-linaro-omap4-panda/copyright"],
+                 names[1]: ["usr/lib/u-boot/omap4_panda/u-boot2.img",
+                                    "foo/bar",
+                                    "flim/flam"],
+                 names[2]: ["some/path/config"]}
+
+        # Generate some test packages
+        maker = PackageMaker()
+        self.useFixture(ContextManagerFixture(maker))
+
+        tarball_content = [("FORMAT", format), ("metadata", metadata)]
+
+        package_names = []
+        for package_name in names:
+            # The files parameter to make_package is a list of files to create.
+            # These files are text files containing package_name and their
+            # path. Since package_name is different for each package, this
+            # gives each file a unique content.
+            deb_file_path = maker.make_package(package_name, '1.0', {},
+                                               files=files[package_name])
+            name = os.path.basename(deb_file_path)
+            tarball_content.append((os.path.join("pkgs", name),
+                                    open(deb_file_path).read()))
+            package_names.append(name)
+
+        tarball = self.add_to_tarball(tarball_content)
+
+        hp = HardwarepackHandler([tarball], board='panda', bootloader='uefi')
+        with hp:
+            path = hp.get_file_from_package("some/path/config", "package2")
+            self.assertTrue(path.endswith("some/path/config"))
 
 
 class TestSetMetadata(TestCaseWithFixtures):
@@ -574,6 +635,23 @@
         self.assertRaises(
             AssertionError, config.set_metadata, 'ahwpack.tar.gz')
 
+    def test_sets_copy_files(self):
+        self.useFixture(MockSomethingFixture(
+            linaro_image_tools.media_create.boards, 'HardwarepackHandler',
+            self.MockHardwarepackHandler))
+        field_to_test = 'bootloader_copy_files'
+        data_to_set = {'package':
+                           [{"source1": "dest1"},
+                            {"source2": "dest2"}]}
+        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.bootloader_copy_files)
+
 
 class TestGetMLOFile(TestCaseWithFixtures):
 
@@ -2937,6 +3015,16 @@
         self.popen_fixture = self.useFixture(MockCmdRunnerPopenFixture())
         self.useFixture(MockSomethingFixture(
             self.config, 'make_boot_files', self.save_args))
+        self.config.hardwarepack_handler.get_file_from_package = \
+            self.get_file_from_package
+        self.config.bootloader_copy_files = None
+
+    def get_file_from_package(self, source_path, source_package):
+        if source_package in self.config.bootloader_copy_files:
+            for file_info in self.config.bootloader_copy_files[source_package]:
+                if source_path in file_info:
+                    return source_path
+        return None
 
     def prepare_config_v3(self, config):
         class c(config):
@@ -2949,6 +3037,13 @@
         self.config.hardwarepack_handler.get_format = lambda: '3.0'
         self.config.hardwarepack_handler.get_file = \
                             lambda file_alias: ['file1', 'file2']
+        self.config.hardwarepack_handler.get_file_from_package = \
+            self.get_file_from_package
+        self.config.bootloader_copy_files = {
+            "package1":
+                [{"file1": "/boot/"},
+                 {"file2": "/boot/grub/renamed"}]}
+
         self.popen_fixture = self.useFixture(MockCmdRunnerPopenFixture())
         self.useFixture(MockSomethingFixture(
             self.config, 'make_boot_files', self.save_args))
@@ -2984,6 +3079,7 @@
         self.prepare_config(boards.BoardConfig)
         self.config.bootloader_flavor = "bootloader_flavor"
         self.config.bootloader_file_in_boot_part = True
+        self.config.bootloader = "u_boot"
         self.call_populate_boot(self.config)
         expected_calls = self.expected_calls[:]
         expected_calls.insert(2,
@@ -3003,7 +3099,7 @@
             expected_calls, self.popen_fixture.mock.commands_executed)
         self.assertEquals(self.expected_args, self.saved_args)
 
-    def test_populate_boot_copy_files(self):
+    def test_populate_bootloader_copy_files(self):
         self.prepare_config_v3(boards.BoardConfig)
         self.config.bootloader_flavor = "bootloader_flavor"
         # Test that copy_files works per spec (puts stuff in boot partition)
@@ -3011,12 +3107,35 @@
         self.config.bootloader_file_in_boot_part = False
         self.call_populate_boot(self.config)
         expected_calls = self.expected_calls[:]
-        expected_calls.insert(2,
+        expected_calls.insert(2, '%s mkdir -p boot_disk/boot' % sudo_args)
+        expected_calls.insert(3,
             '%s cp -v file1 '
-            'boot_disk' % sudo_args)
-        expected_calls.insert(3,
+            'boot_disk/boot/' % sudo_args)
+        expected_calls.insert(4, '%s mkdir -p boot_disk/boot/grub' % sudo_args)
+        expected_calls.insert(5,
             '%s cp -v file2 '
-            'boot_disk' % sudo_args)
+            'boot_disk/boot/grub/renamed' % sudo_args)
+        self.assertEquals(
+            expected_calls, self.popen_fixture.mock.commands_executed)
+        self.assertEquals(self.expected_args, self.saved_args)
+
+    def test_populate_bootloader_copy_files_bootloader_set(self):
+        self.prepare_config_v3(boards.BoardConfig)
+        self.config.bootloader_flavor = "bootloader_flavor"
+        # Test that copy_files works per spec (puts stuff in boot partition)
+        # even if bootloader not in_boot_part.
+        self.config.bootloader_file_in_boot_part = False
+        self.config.bootloader = "u_boot"
+        self.call_populate_boot(self.config)
+        expected_calls = self.expected_calls[:]
+        expected_calls.insert(2, '%s mkdir -p boot_disk/boot' % sudo_args)
+        expected_calls.insert(3,
+                              '%s cp -v file1 '
+                              'boot_disk/boot/' % sudo_args)
+        expected_calls.insert(4, '%s mkdir -p boot_disk/boot/grub' % sudo_args)
+        expected_calls.insert(5,
+                              '%s cp -v file2 '
+                              'boot_disk/boot/grub/renamed' % sudo_args)
         self.assertEquals(
             expected_calls, self.popen_fixture.mock.commands_executed)
         self.assertEquals(self.expected_args, self.saved_args)