Merge lp:~lool/linaro-image-tools/move-populate-boot into lp:linaro-image-tools/11.11

[Loïc Minier]

This moves populate_boot() from media_create.populate_boot to media_create.boards.BoardConfig; bug #716469.

Guilherme, is this what you had in mind?

It seems fine there, albeit I didn't find any concrete things to do after the move.

[Guilherme Salgado (salgado)]

On Wed, 2011-03-23 at 23:25 +0000, Loïc Minier wrote:
[...]
>
> This moves populate_boot() from media_create.populate_boot to media_create.boards.BoardConfig; bug #716469.
>
> Guilherme, is this what you had in mind?

Yep, that's exactly what I had in mind.

> It seems fine there, albeit I didn't find any concrete things to do after the move.

The BoardConfig class indeed seems like a more natural place for it, but
my motivation to file the bug was because of a branch that was about to
land which added some board-specific code to populate_boot(). Because
of that change we'd have to move populate_boot to the BoardConfig class
so that we could get rid of the board-specific code outside of there.

That change didn't land, though, so now this is just a nice refactoring
which doesn't unblock anything else. I really appreciate you working on
it, though.

 review approve

review: Approve

=== modified file 'linaro-media-create'
--- linaro-media-create	2011-03-23 22:25:10 +0000
+++ linaro-media-create	2011-03-23 23:25:52 +0000
@@ -32,7 +32,6 @@
     setup_partitions,
     get_uuid,
     )
-from linaro_image_tools.media_create.populate_boot import populate_boot
 from linaro_image_tools.media_create.rootfs import populate_rootfs
 from linaro_image_tools.media_create.unpack_binary_tarball import (
     unpack_binary_tarball,
@@ -135,9 +134,9 @@
     rootfs_uuid = get_uuid(root_partition)
 
     if args.should_format_bootfs:
-        populate_boot(
-            board_config, ROOTFS_DIR, rootfs_uuid, boot_partition, BOOT_DISK,
-            args.device, args.is_live, args.is_lowmem, args.consoles)
+        board_config.populate_boot(
+            ROOTFS_DIR, rootfs_uuid, boot_partition, BOOT_DISK, args.device,
+            args.is_live, args.is_lowmem, args.consoles)
 
     if args.should_format_rootfs:
         create_swap = False

=== modified file 'linaro_image_tools/media_create/boards.py'
--- linaro_image_tools/media_create/boards.py	2011-03-23 22:25:10 +0000
+++ linaro_image_tools/media_create/boards.py	2011-03-23 23:25:52 +0000
@@ -249,16 +249,16 @@
 
     @classmethod
     def make_boot_files(cls, uboot_parts_dir, is_live, is_lowmem, consoles,
-                        chroot_dir, rootfs_uuid, boot_dir, boot_script,
+                        chroot_dir, rootfs_uuid, boot_dir, boot_script_path,
                         boot_device_or_file):
         boot_env = cls._get_boot_env(is_live, is_lowmem, consoles, rootfs_uuid)
         cls._make_boot_files(
-            uboot_parts_dir, boot_env, chroot_dir, boot_dir, boot_script,
+            uboot_parts_dir, boot_env, chroot_dir, boot_dir, boot_script_path,
             boot_device_or_file)
 
     @classmethod
     def _make_boot_files(cls, uboot_parts_dir, boot_env, chroot_dir, boot_dir,
-                         boot_script, boot_device_or_file):
+                         boot_script_path, boot_device_or_file):
         """Make the necessary boot files for this board.
 
         This is usually board-specific so ought to be defined in every
@@ -266,6 +266,38 @@
         """
         raise NotImplementedError()
 
+    @classmethod
+    def populate_boot(cls, chroot_dir, rootfs_uuid, boot_partition, boot_disk,
+                      boot_device_or_file, is_live, is_lowmem, consoles):
+        parts_dir = 'boot'
+        if is_live:
+            parts_dir = 'casper'
+        uboot_parts_dir = os.path.join(chroot_dir, parts_dir)
+
+        cmd_runner.run(['mkdir', '-p', boot_disk]).wait()
+        cmd_runner.run(['mount', boot_partition, boot_disk],
+            as_root=True).wait()
+
+        if cls.uboot_in_boot_part:
+            assert cls.uboot_flavor is not None, (
+                "uboot_in_boot_part is set but not uboot_flavor")
+            uboot_bin = os.path.join(chroot_dir, 'usr', 'lib', 'u-boot',
+                cls.uboot_flavor, 'u-boot.bin')
+            cmd_runner.run(
+                ['cp', '-v', uboot_bin, boot_disk], as_root=True).wait()
+
+        boot_script_path = os.path.join(boot_disk, cls.boot_script)
+
+        cls.make_boot_files(
+            uboot_parts_dir, is_live, is_lowmem, consoles, chroot_dir,
+            rootfs_uuid, boot_disk, boot_script_path, boot_device_or_file)
+
+        cmd_runner.run(['sync']).wait()
+        try:
+            cmd_runner.run(['umount', boot_disk], as_root=True).wait()
+        except cmd_runner.SubcommandNonZeroReturnValue:
+            pass
+
 
 class OmapConfig(BoardConfig):
     kernel_suffix = 'linaro-omap'
@@ -313,7 +345,7 @@
 
     @classmethod
     def make_boot_files(cls, uboot_parts_dir, is_live, is_lowmem, consoles,
-                        chroot_dir, rootfs_uuid, boot_dir, boot_script,
+                        chroot_dir, rootfs_uuid, boot_dir, boot_script_path,
                         boot_device_or_file):
         # XXX: This is also part of our temporary hack to fix bug 697824; we
         # need to call set_appropriate_serial_tty() before doing anything that
@@ -321,17 +353,17 @@
         cls.set_appropriate_serial_tty(chroot_dir)
         super(OmapConfig, cls).make_boot_files(
             uboot_parts_dir, is_live, is_lowmem, consoles, chroot_dir,
-            rootfs_uuid, boot_dir, boot_script, boot_device_or_file)
+            rootfs_uuid, boot_dir, boot_script_path, boot_device_or_file)
 
     @classmethod
     def _make_boot_files(cls, uboot_parts_dir, boot_env, chroot_dir, boot_dir,
-                         boot_script, boot_device_or_file):
+                         boot_script_path, boot_device_or_file):
         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_boot_script(boot_env, boot_script)
-        make_boot_ini(boot_script, boot_dir)
+        make_boot_script(boot_env, boot_script_path)
+        make_boot_ini(boot_script_path, boot_dir)
 
 
 class BeagleConfig(OmapConfig):
@@ -381,12 +413,12 @@
 
     @classmethod
     def _make_boot_files(cls, uboot_parts_dir, boot_env, chroot_dir, boot_dir,
-                         boot_script, boot_device_or_file):
+                         boot_script_path, 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_boot_script(boot_env, boot_script)
-        make_boot_ini(boot_script, boot_dir)
+        make_boot_script(boot_env, boot_script_path)
+        make_boot_ini(boot_script_path, boot_dir)
 
 
 class Ux500Config(BoardConfig):
@@ -407,11 +439,11 @@
 
     @classmethod
     def _make_boot_files(cls, uboot_parts_dir, boot_env, chroot_dir, boot_dir,
-                         boot_script, boot_device_or_file):
+                         boot_script_path, 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_boot_script(boot_env, boot_script)
+        make_boot_script(boot_env, boot_script_path)
 
 
 class Mx5Config(BoardConfig):
@@ -452,14 +484,14 @@
 
     @classmethod
     def _make_boot_files(cls, uboot_parts_dir, boot_env, chroot_dir, boot_dir,
-                         boot_script, boot_device_or_file):
+                         boot_script_path, boot_device_or_file):
         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_boot_script(boot_env, boot_script)
+        make_boot_script(boot_env, boot_script_path)
 
 
 class Mx51Config(Mx5Config):
@@ -509,7 +541,7 @@
 
     @classmethod
     def _make_boot_files(cls, uboot_parts_dir, boot_env, chroot_dir, boot_dir,
-                         boot_script, boot_device_or_file):
+                         boot_script_path, 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)
@@ -571,7 +603,7 @@
 
     @classmethod
     def _make_boot_files(cls, uboot_parts_dir, boot_env, chroot_dir, boot_dir,
-                         boot_script, boot_device_or_file):
+                         boot_script_path, boot_device_or_file):
         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)
@@ -588,9 +620,9 @@
             uboot_parts_dir, cls.kernel_suffix, boot_dir)
         install_smdkv310_initrd(uInitrd_file, boot_device_or_file)
 
-        # unused at the moment once FAT support enabled for the 
+        # unused at the moment once FAT support enabled for the
         # Samsung u-boot this can be used bug 727978
-        #make_boot_script(boot_env, boot_script)
+        #make_boot_script(boot_env, boot_script_path)
 
 
 board_configs = {
@@ -675,7 +707,7 @@
     return img
 
 
-def make_boot_script(boot_env, boot_script):
+def make_boot_script(boot_env, boot_script_path):
     boot_script_data = (
         "setenv bootcmd '%(bootcmd)s'\n"
         "setenv bootargs '%(bootargs)s'\n"
@@ -687,12 +719,12 @@
     _, tmpfile = tempfile.mkstemp()
     atexit.register(os.unlink, tmpfile)
     plain_boot_script = os.path.join(
-        os.path.dirname(boot_script), 'boot.txt')
+        os.path.dirname(boot_script_path), 'boot.txt')
     with open(tmpfile, 'w') as fd:
         fd.write(boot_script_data)
     cmd_runner.run(['cp', tmpfile, plain_boot_script], as_root=True).wait()
     return _run_mkimage(
-        'script', '0', '0', 'boot script', plain_boot_script, boot_script)
+        'script', '0', '0', 'boot script', plain_boot_script, boot_script_path)
 
 
 def make_flashable_env(boot_env, env_size):
@@ -761,9 +793,10 @@
     cmd_runner.run(["sync"]).wait()
 
 
-def make_boot_ini(boot_script, boot_disk):
+def make_boot_ini(boot_script_path, boot_disk):
     proc = cmd_runner.run(
-        ["cp", "-v", boot_script, "%s/boot.ini" % boot_disk], as_root=True)
+        ["cp", "-v", boot_script_path, "%s/boot.ini" % boot_disk],
+        as_root=True)
     proc.wait()
 
 

=== removed file 'linaro_image_tools/media_create/populate_boot.py'
--- linaro_image_tools/media_create/populate_boot.py	2011-03-23 22:25:10 +0000
+++ linaro_image_tools/media_create/populate_boot.py	1970-01-01 00:00:00 +0000
@@ -1,58 +0,0 @@
-# Copyright (C) 2010, 2011 Linaro
-#
-# Author: Guilherme Salgado <[e-mail]>
-#
-# This file is part of Linaro Image Tools.
-# 
-# Linaro Image Tools is free software: you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-#
-# Linaro Image Tools is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with Linaro Image Tools.  If not, see <http://www.gnu.org/licenses/>.
-
-import os
-
-from linaro_image_tools.media_create import cmd_runner
-
-
-def populate_boot(board_config, chroot_dir, rootfs_uuid, boot_partition,
-                  boot_disk, boot_device_or_file, is_live, is_lowmem,
-                  consoles):
-
-    parts_dir = 'boot'
-    if is_live:
-        parts_dir = 'casper'
-    uboot_parts_dir = os.path.join(chroot_dir, parts_dir)
-
-    cmd_runner.run(['mkdir', '-p', boot_disk]).wait()
-    cmd_runner.run(['mount', boot_partition, boot_disk], as_root=True).wait()
-
-    if board_config.uboot_in_boot_part:
-        uboot_flavor = board_config.uboot_flavor
-        assert uboot_flavor is not None, (
-            "uboot_in_boot_part is set but not uboot_flavor")
-        uboot_bin = os.path.join(
-            chroot_dir, 'usr', 'lib', 'u-boot', uboot_flavor, 'u-boot.bin')
-        cmd_runner.run(
-            ['cp', '-v', uboot_bin, boot_disk], as_root=True).wait()
-
-    boot_script = "%(boot_disk)s/%(boot_script_name)s" % (
-        dict(boot_disk=boot_disk,
-             boot_script_name=board_config.boot_script))
-
-    board_config.make_boot_files(
-        uboot_parts_dir, is_live, is_lowmem, consoles, chroot_dir, rootfs_uuid,
-        boot_disk, boot_script, boot_device_or_file)
-
-    cmd_runner.run(['sync']).wait()
-    try:
-        cmd_runner.run(['umount', boot_disk], as_root=True).wait()
-    except cmd_runner.SubcommandNonZeroReturnValue:
-        pass

=== modified file 'linaro_image_tools/media_create/tests/test_media_create.py'
--- linaro_image_tools/media_create/tests/test_media_create.py	2011-03-23 22:25:10 +0000
+++ linaro_image_tools/media_create/tests/test_media_create.py	2011-03-23 23:25:52 +0000
@@ -81,7 +81,6 @@
     get_uuid,
     _parse_blkid_output,
     )
-from linaro_image_tools.media_create.populate_boot import populate_boot
 from linaro_image_tools.media_create.rootfs import (
     create_flash_kernel_config,
     has_space_left_for_swap,
@@ -1083,9 +1082,9 @@
             self.config, 'make_boot_files', self.save_args))
 
     def call_populate_boot(self, config, is_live=False):
-        populate_boot(
-            config, 'chroot_dir', 'rootfs_uuid', 'boot_partition',
-            'boot_disk', 'boot_device_or_file', is_live, False, [])
+        config.populate_boot(
+            'chroot_dir', 'rootfs_uuid', 'boot_partition', 'boot_disk',
+            'boot_device_or_file', is_live, False, [])
 
     def test_populate_boot_live(self):
         self.prepare_config(boards.BoardConfig)