Merge lp:~mabac/linaro-image-tools/bug-744857 into lp:linaro-image-tools/11.11

[Mattias Backman]

Hi,

This branch addresses Bug #744857 by changing the board config item 'kernel_suffix' to become 'kernel_flavors' which accepts a list of strings.

Each string in the list is supposed to be a partial regexp just as the kernel_suffix was.

This change is implemented by making boards._get_file_matching take a list of strings and an optional prefix string. By making the prefix optional, I hope this change is less confusing.

An alternative to the prefix parameter would be to have make_uImage and make_uInitrd prepend their respective prefixes and supply a list of complete regexps to _get_file_matching but I found that uglier.

Thanks,

Mattias

[James Westby (james-w)]

Hi,

253 + prefix = ''.join(
254 + random.choice(string.ascii_lowercase) for x in range(5))
255 + junk_prefix = ''.join(
256 + random.choice(string.ascii_lowercase) for x in range(5))

Please avoid things like this in tests. This particular test will test
the wrong thing occasionally when the strings end up the same. Not so bad in this
case, but something to avoid anyway.

If you want to signal that the actual strings don't matter then you can use
self.getUniqueString() which will never return the same thing twice.

Otherwise this looks good to me, provided that the mx changes are what we want
to do.

Thanks,

James

review: Approve

[Mattias Backman (mabac)]

> Hi,
>
> 253 + prefix = ''.join(
> 254 + random.choice(string.ascii_lowercase) for x in range(5))
> 255 + junk_prefix = ''.join(
> 256 + random.choice(string.ascii_lowercase) for x in range(5))
>
> Please avoid things like this in tests. This particular test will test
> the wrong thing occasionally when the strings end up the same. Not so bad in
> this
> case, but something to avoid anyway.
>
> If you want to signal that the actual strings don't matter then you can use
> self.getUniqueString() which will never return the same thing twice.

Ok, that's fixed.

>
> Otherwise this looks good to me, provided that the mx changes are what we want
> to do.

I think they are.
Eric, does this look ok?

Thanks,

Mattias

[Loïc Minier (lool)]

 Thanks for working on this!

On Wed, Mar 30, 2011, Mattias Backman wrote:
> vmlinuz = _get_file_matching(
> + [os.path.join(chroot_dir, 'boot', 'vmlinuz*')])
[...]
> + kernel_flavors = ['linaro-lt-mx5', 'linaro-mx51']
[...]
> +def _get_file_matching(suffixes, prefix=''):
[...]
> + for suffix in suffixes:
> + regex = prefix + suffix
> + files = glob.glob(regex)
> + if len(files) == 1:
> + return files[0]
> + elif len(files) > 1:
> + # TODO: Could ask the user to choose which file to use instead of
> + # raising an exception.
> + raise ValueError("Too many files matching '%s' found." % regex)
> + raise ValueError(
> + "No files found matching '%s[%s]'; can't continue" % (prefix,
> + '|'.join(suffixes)))

 I wonder whether it would look a bit nicer with:
    _get_file_matching('foo/boot/vmlinuz-*-%(kflavor)s', flavors)
 so that we're not dealing with "prefix" and "suffix".
 (_get_file_matching() would replace kflavor with regex % dict(...))

 Since it's also getting quite specific, perhaps it would make sense to
 move this to a BoardConfig function like:
    _get_kflavor_file('foo/boot/vmlinuz-*-%(kflavor)s')
 and let it use self.kflavors.

 I'm not sold on my own proposals above; the code can also go in as is,
 except for James comment, I don't think we actually need random strings
 and something like "foo" and "bar" would probably be fine.

 Technically, there's also a potential bug if there is:
  vmlinuz-mx5
  vmlinuz-mx51
  initrd-mx51
 you might pick vmlinuz-mx5 + initrd-mx51, but that's a bug in the
 rootfs in the first place, so not too worried about; if there was some
 kind of assert, I wouldn't mind.

   Thanks!
--
Loïc Minier

[Loïc Minier (lool)]

On r311, you drop -mx53 and add -mx5; I think it wouldn't hurt to keep -mx5 working, for instance until 11.05 is released; it's cheap and it allows using older hwpacks.

There is a question of which order we use for kernel flavors; do we prefer the more specific name over the least specific name (mx51 first, then mx5) or do we prefer the latest name over the older names (linaro-lt-mx5 before linaro-lt-mx53). The latter is interesting because it's expected that you have exactly one kernel and we know which Linaro ones are the best ones, but I find the former rules easier to be consistent with. Anyway, we can think of a better mechanism in the future or allow overriding the kernel flavor.

If you're tempted, you could also commit a similar fix for bug #720114 by supporting the "omap4" kernel flavor. I think it was used in 10.11, and is still used in 11.05, so ordering wont help as both omap and omap4 are valid.

[Loïc Minier (lool)]

Would also be nice to set kernel_flavors to u8500, ux500 or something similar for bug #737173

[lp:~mabac/linaro-image-tools/bug-744857]

308. By Loïc Minier on 2011-04-01

Merge lp:~lag/linaro-image-tools/lt-ux500-support but change glob from "u*500"
to "u?500"; fixes support for new name of vmlinuz file in U8500 hwpack;
LP: #737173.

[Mattias Backman (mabac)]

> On r311, you drop -mx53 and add -mx5; I think it wouldn't hurt to keep -mx5
> working, for instance until 11.05 is released; it's cheap and it allows using
> older hwpacks.

Ok, I re-added -mx53.

>
> There is a question of which order we use for kernel flavors; do we prefer the
> more specific name over the least specific name (mx51 first, then mx5) or do
> we prefer the latest name over the older names (linaro-lt-mx5 before linaro-
> lt-mx53). The latter is interesting because it's expected that you have
> exactly one kernel and we know which Linaro ones are the best ones, but I find
> the former rules easier to be consistent with. Anyway, we can think of a
> better mechanism in the future or allow overriding the kernel flavor.

The current fix is as simple as just using the suffixes in order and if one early in the list matches more than one kernel it fails, even if a later suffix would match just one file. Is it ok to keep this behavior until the above is looked into?

>
> If you're tempted, you could also commit a similar fix for bug #720114 by
> supporting the "omap4" kernel flavor. I think it was used in 10.11, and is
> still used in 11.05, so ordering wont help as both omap and omap4 are valid.

So this might not work as expected?
+ kernel_flavors = ['linaro-omap4', 'linaro-omap']
It will use -omap4 if that kernel is found and -omap otherwise.

[Mattias Backman (mabac)]

> Would also be nice to set kernel_flavors to u8500, ux500 or something similar
> for bug #737173

I see this has been worked on in trunk. Should I make this
60 + kernel_flavors = ['u8500', 'ux500', 'u?500']
or will the explicit list ['u8500', 'ux500'] cover it?

[Mattias Backman (mabac)]

> Thanks for working on this!
>
> On Wed, Mar 30, 2011, Mattias Backman wrote:
> > vmlinuz = _get_file_matching(
> > + [os.path.join(chroot_dir, 'boot', 'vmlinuz*')])
> [...]
> > + kernel_flavors = ['linaro-lt-mx5', 'linaro-mx51']
> [...]
> > +def _get_file_matching(suffixes, prefix=''):
> [...]
> > + for suffix in suffixes:
> > + regex = prefix + suffix
> > + files = glob.glob(regex)
> > + if len(files) == 1:
> > + return files[0]
> > + elif len(files) > 1:
> > + # TODO: Could ask the user to choose which file to use instead
> of
> > + # raising an exception.
> > + raise ValueError("Too many files matching '%s' found." % regex)
> > + raise ValueError(
> > + "No files found matching '%s[%s]'; can't continue" % (prefix,
> > +
> '|'.join(suffixes)))
>
> I wonder whether it would look a bit nicer with:
> _get_file_matching('foo/boot/vmlinuz-*-%(kflavor)s', flavors)
> so that we're not dealing with "prefix" and "suffix".
> (_get_file_matching() would replace kflavor with regex % dict(...))
>
> Since it's also getting quite specific, perhaps it would make sense to
> move this to a BoardConfig function like:
> _get_kflavor_file('foo/boot/vmlinuz-*-%(kflavor)s')
> and let it use self.kflavors.

That would actually help with the assert suggestion below.

>
> I'm not sold on my own proposals above; the code can also go in as is,
> except for James comment, I don't think we actually need random strings
> and something like "foo" and "bar" would probably be fine.
>
>
> Technically, there's also a potential bug if there is:
> vmlinuz-mx5
> vmlinuz-mx51
> initrd-mx51
> you might pick vmlinuz-mx5 + initrd-mx51, but that's a bug in the
> rootfs in the first place, so not too worried about; if there was some
> kind of assert, I wouldn't mind.

I can't find a good place to do this since the file names matched by the regexp are not available on the same level. Something like this in BoardConfig would make that assert simpler.
def _make_boot_files(...):
    ...
    kfile = _get_kflavor_file('foo/boot/vmlinuz-*-%(kflavor)s')
    ifile = _get_initrd_file('foo/boot/initrd-img-*-%(kflavor)s')
    assert(files match in some way)
    make_uImage(kfile, boot_dir)
    make_uInitrd(ifile, boot_dir)

Would you like me to make this change?

[Loïc Minier (lool)]

On Mon, Apr 04, 2011, Mattias Backman wrote:
> ...
> kfile = _get_kflavor_file('foo/boot/vmlinuz-*-%(kflavor)s')
> ifile = _get_initrd_file('foo/boot/initrd-img-*-%(kflavor)s')
> assert(files match in some way)
> make_uImage(kfile, boot_dir)
> make_uInitrd(ifile, boot_dir)

 Sounds good!

 There might be another way to deal with this:
 (kfile, flavor) = _get_kflavor_file('foo/boot/vmlinuz-*-%(kflavor)s',
                                     all_flavors)
 (ifile, _) _get_kflavor_file('foo/boot/vmlinuz-*-%(kflavor)s',
                              [flavor])

 i.e. pass the list of flavors to the matching function.

 That way, you don't need an assert but kfile and ifile are of the same
 flavor by construction.

--
Loïc Minier

[Loïc Minier (lool)]

On Mon, Apr 04, 2011, Mattias Backman wrote:
> The current fix is as simple as just using the suffixes in order and
> if one early in the list matches more than one kernel it fails, even
> if a later suffix would match just one file. Is it ok to keep this
> behavior until the above is looked into?

 I think it's fine for now, yes

> > If you're tempted, you could also commit a similar fix for bug #720114 by
> > supporting the "omap4" kernel flavor. I think it was used in 10.11, and is
> > still used in 11.05, so ordering wont help as both omap and omap4 are valid.
>
> So this might not work as expected?
> + kernel_flavors = ['linaro-omap4', 'linaro-omap']
> It will use -omap4 if that kernel is found and -omap otherwise.

 I think that's just fine for linaro-image-tools right now! :-)

--
Loïc Minier

[Loïc Minier (lool)]

On Mon, Apr 04, 2011, Mattias Backman wrote:
> I see this has been worked on in trunk. Should I make this
> 60 + kernel_flavors = ['u8500', 'ux500', 'u?500']
> or will the explicit list ['u8500', 'ux500'] cover it?

 I committed to trunk so that people could continue using bzr
 linaro-image-tools while you were implementing support for multiple
 flavors. I prefer the explicit list rather than regexps in the
 flavors.

   Thanks!
--
Loïc Minier

[lp:~mabac/linaro-image-tools/bug-744857]

309. By Mattias Backman on 2011-04-04

Change kernel_suffix to kernel_flavors.

310. By Mattias Backman on 2011-04-04

Add get_kflavor_file function.

311. By Mattias Backman on 2011-04-05

Make the changes to _make_boot_files.

[Mattias Backman (mabac)]

> On Mon, Apr 04, 2011, Mattias Backman wrote:
> > ...
> > kfile = _get_kflavor_file('foo/boot/vmlinuz-*-%(kflavor)s')
> > ifile = _get_initrd_file('foo/boot/initrd-img-*-%(kflavor)s')
> > assert(files match in some way)
> > make_uImage(kfile, boot_dir)
> > make_uInitrd(ifile, boot_dir)
>
> Sounds good!
>
> There might be another way to deal with this:
> (kfile, flavor) = _get_kflavor_file('foo/boot/vmlinuz-*-%(kflavor)s',
> all_flavors)
> (ifile, _) _get_kflavor_file('foo/boot/vmlinuz-*-%(kflavor)s',
> [flavor])

I tried something like this in bug-744857-kflavor-file but I'm not sure that anything really became more beautiful. ;) Can you have a look and see if this is getting close to what you're after?

I'm aware that some of the tests fail, since my class function get None from mocked module functions. I've spent a little too much time on finding a way to fix that without mocking the crap out of existing tests. Will keep working on that if this is a change we want to keep. Any help with the tests will be very appreciated. (the mock of _get_file_matching returns None, which to BoardConfig._get_kflavor_file means that nothing was found.)

Thanks,

Mattias

[Mattias Backman (mabac)]

Here's the new branch https://code.launchpad.net/~mabac/linaro-image-tools/bug-744857-kflavor-file

[Loïc Minier (lool)]

I've sent some fixes for your branch at https://code.launchpad.net/~lool/linaro-image-tools/bug-744857-kflavor-file/+merge/56341

Looking at making things more elegant, I opted to make _get_kflavor_files a function taking just a directory and returning both kernel and initrd; let me know if that looks good.

I've fixed some typos and your tests to pass, but there are regressions in other tests.

Guilherme, we could lookup kernel and initrd in make_boot_files() and pass that down to _make_boot_files(), but that's two more args; are you ok with this?

[Guilherme Salgado (salgado)]

If it's safe to assume that all hwpacks will include kernel/initrd images matching the current glob, then that should be fine -- I don't think adding a couple extra arguments to _make_boot_files() is a big deal.

Oh, you might want to merge from trunk before doing any more changes here as you're touching some code that was changed yesterday.

[lp:~mabac/linaro-image-tools/bug-744857]

312. By Mattias Backman on 2011-04-05

Merge lool's changes for _get_kflavor_files.

313. By Mattias Backman on 2011-04-05

Fix tests by mocking _get_kflavor_files.

314. By Mattias Backman on 2011-04-05

Merge from trunk.

[Mattias Backman (mabac)]

> I've sent some fixes for your branch at https://code.launchpad.net/~lool
> /linaro-image-tools/bug-744857-kflavor-file/+merge/56341
>
> Looking at making things more elegant, I opted to make _get_kflavor_files a
> function taking just a directory and returning both kernel and initrd; let me
> know if that looks good.

Looks great, I have merged your changes. Thanks!

>
> I've fixed some typos and your tests to pass, but there are regressions in
> other tests.

Yeah, those where there failing tests I mentioned. Guilherme has helped me fix those now.

>
> Guilherme, we could lookup kernel and initrd in make_boot_files() and pass
> that down to _make_boot_files(), but that's two more args; are you ok with
> this?

That would take away the last smell I think, no need for all configs to look them up. I'll change that too.

[lp:~mabac/linaro-image-tools/bug-744857]

315. By Mattias Backman on 2011-04-05

Move looking up kernel and initrd files to make_boot_files().

[Mattias Backman (mabac)]

Left to do is some digging for kernel flavors used by old hwpacks, which I haven't done yet.

[Mattias Backman (mabac)]

I checked the hwpacks at http://releases.linaro.org/platform/linaro-m/hwpacks/final/ and the only kernel I found not matching the current kernel_flavors is this one.
bsp-omap4 which contains kernel 2.6.35-903-omap4

[lp:~mabac/linaro-image-tools/bug-744857]

316. By Mattias Backman on 2011-04-06

Add kernel_flavor 'omap4' for linaro-m hwpack bsp-omap4.

317. By Mattias Backman on 2011-04-06

Merge from trunk.

318. By Mattias Backman on 2011-04-06

Fix deprecation warning. Unrelated but maybe I can sneak it in here.

[Loïc Minier (lool)]

Looks good! I'd do some minor changes while merging if you don't mind:
- drop newline added near mock_set_appropriate_serial_tty
- add a test when no kflavor matches
- sort kflavors to have the most specific flavor first (e.g. omap4 before omap)
let me know if you'd disagree with the above changes

review: Approve

[lp:~mabac/linaro-image-tools/bug-744857]

319. By Mattias Backman on 2011-04-06

Remove blank line, sort kernel_flavors and add test for _get_kflavor_files not finding files.

=== modified file 'linaro_image_tools/hwpack/packages.py'
--- linaro_image_tools/hwpack/packages.py	2011-03-24 21:47:55 +0000
+++ linaro_image_tools/hwpack/packages.py	2011-04-06 12:55:54 +0000
@@ -617,7 +617,7 @@
             base = os.path.basename(candidate.filename)
             installed.append(FetchedPackage.from_apt(candidate, base))
         for package in self.cache.cache:
-            if not package.isInstalled:
+            if not package.is_installed:
                 continue
             candidate = package.installed
             base = os.path.basename(candidate.filename)

=== modified file 'linaro_image_tools/media_create/boards.py'
--- linaro_image_tools/media_create/boards.py	2011-04-05 11:06:49 +0000
+++ linaro_image_tools/media_create/boards.py	2011-04-06 12:55:54 +0000
@@ -36,6 +36,9 @@
 
 from linaro_image_tools.media_create.partitions import SECTOR_SIZE
 
+KERNEL_GLOB = 'vmlinuz-*-%(kernel_flavor)s'
+INITRD_GLOB = 'initrd.img-*-%(kernel_flavor)s'
+
 # Notes:
 # * since we align partitions on 4 MiB by default, geometry is currently 128
 #   heads and 32 sectors (2 MiB) as to have CHS-aligned partition start/end
@@ -150,7 +153,7 @@
     kernel_addr = None
     initrd_addr = None
     load_addr = None
-    kernel_suffix = None
+    kernel_flavors = None
     boot_script = None
     serial_tty = None
 
@@ -304,13 +307,14 @@
     def make_boot_files(cls, uboot_parts_dir, is_live, is_lowmem, consoles,
                         chroot_dir, rootfs_uuid, boot_dir, boot_device_or_file):
         boot_env = cls._get_boot_env(is_live, is_lowmem, consoles, rootfs_uuid)
+        (k_img_data, i_img_data) = cls._get_kflavor_files(uboot_parts_dir)
         cls._make_boot_files(
-            uboot_parts_dir, boot_env, chroot_dir, boot_dir, 
-            boot_device_or_file)
+            boot_env, chroot_dir, boot_dir, 
+            boot_device_or_file, k_img_data, i_img_data)
 
     @classmethod
-    def _make_boot_files(cls, uboot_parts_dir, boot_env, chroot_dir, boot_dir,
-                         boot_device_or_file):
+    def _make_boot_files(cls, boot_env, chroot_dir, boot_dir,
+                         boot_device_or_file, k_img_data, i_img_data):
         """Make the necessary boot files for this board.
 
         This is usually board-specific so ought to be defined in every
@@ -348,9 +352,27 @@
         except cmd_runner.SubcommandNonZeroReturnValue:
             pass
 
+    @classmethod
+    def _get_kflavor_files(cls, path):
+        """Search for kernel and initrd in path."""
+        for flavor in cls.kernel_flavors:
+            kregex = KERNEL_GLOB % {'kernel_flavor' : flavor}
+            iregex = INITRD_GLOB % {'kernel_flavor' : flavor}
+            kernel = _get_file_matching(os.path.join(path, kregex))
+            if kernel is not None:
+                initrd = _get_file_matching(os.path.join(path, iregex))
+                if initrd is not None:
+                    return (kernel, initrd)
+                raise ValueError(
+                    "Found kernel for flavor %s but no initrd matching %s" % (
+                        flavor, iregex))
+        raise ValueError(
+            "No kernel found matching %s for flavors %s" % (
+                KERNEL_GLOB, " ".join(cls.kernel_flavors)))
+
 
 class OmapConfig(BoardConfig):
-    kernel_suffix = 'linaro-omap'
+    kernel_flavors = ['linaro-omap4', 'linaro-omap', 'omap4']
     uboot_in_boot_part = True
 
     # XXX: Here we define these things as dynamic properties because our
@@ -405,12 +427,11 @@
             rootfs_uuid, boot_dir, boot_device_or_file)
 
     @classmethod
-    def _make_boot_files(cls, uboot_parts_dir, boot_env, chroot_dir, boot_dir,
-                         boot_device_or_file):
+    def _make_boot_files(cls, boot_env, chroot_dir, boot_dir,
+                         boot_device_or_file, k_img_data, i_img_data):
         install_omap_boot_loader(chroot_dir, boot_dir)
-        make_uImage(
-            cls.load_addr, uboot_parts_dir, cls.kernel_suffix, boot_dir)
-        make_uInitrd(uboot_parts_dir, cls.kernel_suffix, boot_dir)
+        make_uImage(cls.load_addr, k_img_data, boot_dir)
+        make_uInitrd(i_img_data, boot_dir)
         boot_script_path = os.path.join(boot_dir, cls.boot_script)
         make_boot_script(boot_env, boot_script_path)
         make_boot_ini(boot_script_path, boot_dir)
@@ -462,11 +483,10 @@
     uboot_flavor = None
 
     @classmethod
-    def _make_boot_files(cls, uboot_parts_dir, boot_env, chroot_dir, boot_dir,
-                         boot_device_or_file):
-        make_uImage(
-            cls.load_addr, uboot_parts_dir, cls.kernel_suffix, boot_dir)
-        make_uInitrd(uboot_parts_dir, cls.kernel_suffix, boot_dir)
+    def _make_boot_files(cls, boot_env, chroot_dir, boot_dir,
+                         boot_device_or_file, k_img_data, i_img_data):
+        make_uImage(cls.load_addr, k_img_data, boot_dir)
+        make_uInitrd(i_img_data, boot_dir)
         boot_script_path = os.path.join(boot_dir, cls.boot_script)
         make_boot_script(boot_env, boot_script_path)
         make_boot_ini(boot_script_path, boot_dir)
@@ -479,7 +499,7 @@
     kernel_addr = '0x00100000'
     initrd_addr = '0x08000000'
     load_addr = '0x00008000'
-    kernel_suffix = 'u?500'
+    kernel_flavors = ['u8500', 'ux500']
     boot_script = 'flash.scr'
     extra_boot_args_options = (
         'earlyprintk rootdelay=1 fixrtc nocompcache '
@@ -489,11 +509,10 @@
     mmc_option = '1:1'
 
     @classmethod
-    def _make_boot_files(cls, uboot_parts_dir, boot_env, chroot_dir, boot_dir,
-                         boot_device_or_file):
-        make_uImage(
-            cls.load_addr, uboot_parts_dir, cls.kernel_suffix, boot_dir)
-        make_uInitrd(uboot_parts_dir, cls.kernel_suffix, boot_dir)
+    def _make_boot_files(cls, boot_env, chroot_dir, boot_dir,
+                         boot_device_or_file, k_img_data, i_img_data):
+        make_uImage(cls.load_addr, k_img_data, boot_dir)
+        make_uInitrd(i_img_data, boot_dir)
         boot_script_path = os.path.join(boot_dir, cls.boot_script)
         make_boot_script(boot_env, boot_script_path)
 
@@ -535,14 +554,13 @@
             loader_start, loader_len, boot_start, boot_len, root_start)
 
     @classmethod
-    def _make_boot_files(cls, uboot_parts_dir, boot_env, chroot_dir, boot_dir,
-                         boot_device_or_file):
+    def _make_boot_files(cls, boot_env, chroot_dir, boot_dir,
+                         boot_device_or_file, k_img_data, i_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)
-        make_uImage(
-            cls.load_addr, uboot_parts_dir, cls.kernel_suffix, boot_dir)
-        make_uInitrd(uboot_parts_dir, cls.kernel_suffix, boot_dir)
+        make_uImage(cls.load_addr, k_img_data, boot_dir)
+        make_uInitrd(i_img_data, boot_dir)
         boot_script_path = os.path.join(boot_dir, cls.boot_script)
         make_boot_script(boot_env, boot_script_path)
 
@@ -551,14 +569,14 @@
     kernel_addr = '0x90000000'
     initrd_addr = '0x92000000'
     load_addr = '0x90008000'
-    kernel_suffix = 'linaro-mx51'
+    kernel_flavors = ['linaro-mx51', 'linaro-lt-mx5']
 
 
 class Mx53Config(Mx5Config):
     kernel_addr = '0x70800000'
     initrd_addr = '0x71800000'
     load_addr = '0x70008000'
-    kernel_suffix = 'linaro-lt-mx53'
+    kernel_flavors = ['linaro-lt-mx53', 'linaro-lt-mx5']
 
 
 class EfikamxConfig(Mx51Config):
@@ -586,18 +604,17 @@
     kernel_addr = '0x60008000'
     initrd_addr = '0x81000000'
     load_addr = kernel_addr
-    kernel_suffix = 'linaro-vexpress'
+    kernel_flavors = ['linaro-vexpress']
     boot_script = None
     # ARM Boot Monitor is used to load u-boot, uImage etc. into flash and
     # only allows for FAT16
     fat_size = 16
 
     @classmethod
-    def _make_boot_files(cls, uboot_parts_dir, boot_env, chroot_dir, boot_dir,
-                         boot_device_or_file):
-        make_uImage(
-            cls.load_addr, uboot_parts_dir, cls.kernel_suffix, boot_dir)
-        make_uInitrd(uboot_parts_dir, cls.kernel_suffix, boot_dir)
+    def _make_boot_files(cls, boot_env, chroot_dir, boot_dir,
+                         boot_device_or_file, k_img_data, i_img_data):
+        make_uImage(cls.load_addr, k_img_data, boot_dir)
+        make_uInitrd(i_img_data, boot_dir)
 
 class SMDKV310Config(BoardConfig):
     serial_tty = 'ttySAC1'
@@ -605,7 +622,7 @@
     kernel_addr = '0x40007000'
     initrd_addr = '0x41000000'
     load_addr = '0x40008000'
-    kernel_suffix = 's5pv310'
+    kernel_flavors = ['s5pv310']
     boot_script = 'boot.scr'
     mmc_part_offset = 1
     mmc_option = '0:2'
@@ -655,8 +672,8 @@
         return boot_env
 
     @classmethod
-    def _make_boot_files(cls, uboot_parts_dir, boot_env, chroot_dir, boot_dir,
-                         boot_device_or_file):
+    def _make_boot_files(cls, boot_env, chroot_dir, boot_dir,
+                         boot_device_or_file, k_img_data, i_img_data):
         uboot_file = os.path.join(
             chroot_dir, 'usr', 'lib', 'u-boot', 'smdkv310', 'u-boot.v310')
         install_smdkv310_boot_loader(uboot_file, boot_device_or_file)
@@ -665,12 +682,9 @@
         env_file = make_flashable_env(boot_env, env_size)
         install_smdkv310_boot_env(env_file, boot_device_or_file)
 
-        uImage_file = make_uImage(
-            cls.load_addr, uboot_parts_dir, cls.kernel_suffix, boot_dir)
+        uImage_file = make_uImage(cls.load_addr, k_img_data, boot_dir)
         install_smdkv310_uImage(uImage_file, boot_device_or_file)
-
-        uInitrd_file = make_uInitrd(
-            uboot_parts_dir, cls.kernel_suffix, boot_dir)
+        uInitrd_file = make_uInitrd(i_img_data, boot_dir)
         install_smdkv310_initrd(uInitrd_file, boot_device_or_file)
 
         # unused at the moment once FAT support enabled for the
@@ -741,26 +755,21 @@
     if len(files) == 1:
         return files[0]
     elif len(files) == 0:
-        raise ValueError(
-            "No files found matching '%s'; can't continue" % regex)
+        return None
     else:
         # TODO: Could ask the user to chosse which file to use instead of
         # raising an exception.
         raise ValueError("Too many files matching '%s' found." % regex)
 
 
-def make_uImage(load_addr, uboot_parts_dir, suffix, boot_disk):
-    img_data = _get_file_matching(
-        '%s/vmlinuz-*-%s' % (uboot_parts_dir, suffix))
+def make_uImage(load_addr, img_data, boot_disk):
     img = '%s/uImage' % boot_disk
     _run_mkimage(
         'kernel', load_addr, load_addr, 'Linux', img_data, img)
     return img
 
 
-def make_uInitrd(uboot_parts_dir, suffix, boot_disk):
-    img_data = _get_file_matching(
-        '%s/initrd.img-*-%s' % (uboot_parts_dir, suffix))
+def make_uInitrd(img_data, boot_disk):
     img = '%s/uInitrd' % boot_disk
     _run_mkimage('ramdisk', '0', '0', 'initramfs', img_data, img)
     return img

=== modified file 'linaro_image_tools/media_create/tests/test_media_create.py'
--- linaro_image_tools/media_create/tests/test_media_create.py	2011-04-04 22:28:58 +0000
+++ linaro_image_tools/media_create/tests/test_media_create.py	2011-04-06 12:55:54 +0000
@@ -175,6 +175,13 @@
             classmethod(set_appropriate_serial_tty_mock)))
 
     def make_boot_files(self, config):
+        def _get_kflavor_files_mock(cls, path):
+            return (path, path)
+
+        self.useFixture(MockSomethingFixture(
+            config, '_get_kflavor_files',
+            classmethod(_get_kflavor_files_mock)))
+
         config.make_boot_files('', False, False, [], '', '', '', '')
 
     def test_vexpress_steps(self):
@@ -518,7 +525,7 @@
     def test_make_uImage(self):
         self._mock_get_file_matching()
         fixture = self._mock_Popen()
-        make_uImage('load_addr', 'parts_dir', 'sub_arch', 'boot_disk')
+        make_uImage('load_addr', 'parts_dir/vmlinuz-*-sub_arch', 'boot_disk')
         expected = [
             '%s mkimage -A arm -O linux -T kernel -C none -a load_addr '
             '-e load_addr -n Linux -d parts_dir/vmlinuz-*-sub_arch '
@@ -528,7 +535,7 @@
     def test_make_uInitrd(self):
         self._mock_get_file_matching()
         fixture = self._mock_Popen()
-        make_uInitrd('parts_dir', 'sub_arch', 'boot_disk')
+        make_uInitrd('parts_dir/initrd.img-*-sub_arch', 'boot_disk')
         expected = [
             '%s mkimage -A arm -O linux -T ramdisk -C none -a 0 -e 0 '
             '-n initramfs -d parts_dir/initrd.img-*-sub_arch '
@@ -609,9 +616,41 @@
         self.assertRaises(
             ValueError, _get_file_matching, '%s/%s*' % (directory, prefix))
 
+    def test_get_kflavor_files_more_specific(self):
+        tempdir = self.useFixture(CreateTempDirFixture()).tempdir
+        flavorx = 'flavorX'
+        flavorxy = 'flavorXY'
+        class config(boards.BoardConfig):
+            kernel_flavors = [flavorx, flavorxy]
+        for f in reversed(config.kernel_flavors):
+            kfile = os.path.join(tempdir, 'vmlinuz-1-%s' % f)
+            ifile = os.path.join(tempdir, 'initrd.img-1-%s' % f)
+            open(kfile, "w").close()
+            open(ifile, "w").close()
+        self.assertEqual((kfile, ifile), config._get_kflavor_files(tempdir))
+
+    def test_get_kflavor_files_later_in_flavors(self):
+        tempdir = self.useFixture(CreateTempDirFixture()).tempdir
+        flavor1 = 'flavorXY'
+        flavor2 = 'flavorAA'
+        class config(boards.BoardConfig):
+            kernel_flavors = [flavor1, flavor2]
+        kfile = os.path.join(tempdir, 'vmlinuz-1-%s' % flavor1)
+        ifile = os.path.join(tempdir, 'initrd.img-1-%s' % flavor1)
+        open(kfile, "w").close()
+        open(ifile, "w").close()
+        self.assertEqual((kfile, ifile), config._get_kflavor_files(tempdir))
+
+    def test_get_kflavor_files_raises_when_no_match(self):
+        tempdir = self.useFixture(CreateTempDirFixture()).tempdir
+        flavor1 = 'flavorXY'
+        flavor2 = 'flavorAA'
+        class config(boards.BoardConfig):
+            kernel_flavors = [flavor1, flavor2]
+        self.assertRaises(ValueError, config._get_kflavor_files, tempdir)
+
     def test_get_file_matching_no_files_found(self):
-        self.assertRaises(
-            ValueError, _get_file_matching, '/foo/bar/baz/*non-existent')
+        self.assertEqual(None, _get_file_matching('/foo/bar/baz/*non-existent'))
 
     def test_run_mkimage(self):
         # Create a fake boot script.