Merge lp:~salgado/linaro-image-tools/port-install_hwpacks into lp:linaro-image-tools/11.11

[Guilherme Salgado]

Port install_hwpacks to python

[James Westby (james-w)]

Looks good, thanks.

I'm interested in why you chose to use atexit rather than finally
blocks etc?

Thanks,

James

review: Approve

[Guilherme Salgado (salgado)]

Hi James,

Thanks for the review!

The reason why I went with atexit is that most of the functions here are
just setting things up in the chroot so that install_hwpack[s] can run,
and we want these things to be undone only after install_hwpack[s] has
finished. To achieve that using try/finally blocks I'd need to have
most of this code in a single function, which is not nice.

=== modified file 'linaro-media-create'
--- linaro-media-create	2011-01-05 15:52:33 +0000
+++ linaro-media-create	2011-01-05 16:55:11 +0000
@@ -269,66 +269,8 @@
 RFS_UUID=`uuidgen -r`
 
 install_hwpacks () {
-  chroot=${DIR}/binary
-  # Make sure we unmount /proc in the chroot or else it can't be moved to the
-  # rootfs.
-
-  LINARO_HWPACK_INSTALL=$(which linaro-hwpack-install)
-
-  sudo mv -f "${chroot}/etc/resolv.conf" ${TMP_DIR}/resolv.conf.orig
-  sudo cp /etc/resolv.conf "${chroot}/etc/resolv.conf"
-
-  sudo mv -f "${chroot}/etc/hosts" ${TMP_DIR}/hosts.orig
-  sudo cp /etc/hosts "${chroot}/etc/hosts"
-
-  local arch_is_arm=no
-  case `uname -m` in
-    arm*)
-      arch_is_arm=yes
-      ;;
-    *)
-      python -m media_create.ensure_command qemu-arm-static qemu-arm-static
-      python -m media_create.ensure_command qemu-img qemu-kvm
-      sudo cp /usr/bin/qemu-arm-static "${chroot}/usr/bin"
-      ;;
-  esac
-  sudo cp "$LINARO_HWPACK_INSTALL" "${chroot}/usr/bin"
-
-  # Actually install the hwpack.
-  sudo mount proc "${chroot}/proc" -t proc
-  cleanup_chroot() {
-    sudo umount -v "$chroot/proc"
-  }
-
-  for HWPACK_FILE in "${HWPACK_FILES[@]}"; do
-
-    sudo cp "$HWPACK_FILE" "$chroot"
-    echo ""
-    echo "---------------------------------------------------"
-    echo "Installing (apt-get) $HWPACK_FILE in target rootfs."
-    echo ""
-    if [ "$HWPACK_FORCE_YES" = "yes" ]; then
-      sudo LC_ALL=C chroot "$chroot" linaro-hwpack-install --force-yes /"$(basename "$HWPACK_FILE")"
-    else
-      sudo LC_ALL=C chroot "$chroot" linaro-hwpack-install /"$(basename "$HWPACK_FILE")"
-    fi
-    echo "---------------------------------------------------"
-    echo ""
-    sudo rm -f "${chroot}/$(basename "$HWPACK_FILE")"
-
-  done
-
-  # Revert some changes we did to the rootfs as we don't want them in the
-  # image.
-  sudo umount "${chroot}/proc"
-  cleanup_chroot() { :; }
-
-  sudo mv -f ${TMP_DIR}/resolv.conf.orig "${chroot}/etc/resolv.conf"
-  sudo mv -f ${TMP_DIR}/hosts.orig "${chroot}/etc/hosts"
-  if [ "arch_is_arm" = no ]; then
-    sudo rm -f "${chroot}/usr/bin/qemu-arm-static"
-  fi
-  sudo rm -f "${chroot}/usr/bin/linaro-hwpack-install"
+  python -m media_create.hwpack "${DIR}/binary" "$HWPACK_FORCE_YES" \
+    "${HWPACK_FILES[@]}"
 }
 
 unpack_binary_tarball() {
@@ -444,10 +386,6 @@
   exit 2
 }
 
-# Initially, no action is needed to clean up mounts in the chroot.
-# install_hwpack will temporarily define this function when needed.
-cleanup_chroot() { :; }
-
 cleanup_handler() {
   status=$?
 
@@ -463,7 +401,6 @@
   echo "Performing cleanup..."
   echo
 
-  cleanup_chroot
   cleanup_sd
   cleanup_mountpoints
   cleanup_tempfiles

=== modified file 'media_create/cmd_runner.py'
--- media_create/cmd_runner.py	2010-12-17 07:03:34 +0000
+++ media_create/cmd_runner.py	2011-01-05 16:55:11 +0000
@@ -26,10 +26,6 @@
     return Popen(args, stdin=stdin, stdout=stdout, stderr=stderr)
 
 
-def get_extended_env(ext):
-    return os.environ.update(ext)
-
-
 class Popen(subprocess.Popen):
     """A version of Popen which raises an error on non-zero returncode.
 
@@ -37,9 +33,12 @@
     SubcommandNonZeroReturnValue if it's non-zero.
     """
 
-    def __init__(self, args, **kwargs):
+    def __init__(self, args, env=None, **kwargs):
         self._my_args = args
-        super(Popen, self).__init__(args, **kwargs)
+        if env is None:
+            env = os.environ.copy()
+        env['LC_ALL'] = 'C'
+        super(Popen, self).__init__(args, env=env, **kwargs)
 
     def wait(self):
         returncode = super(Popen, self).wait()

=== added file 'media_create/hwpack.py'
--- media_create/hwpack.py	1970-01-01 00:00:00 +0000
+++ media_create/hwpack.py	2011-01-05 16:55:11 +0000
@@ -0,0 +1,120 @@
+import atexit
+import os
+import platform
+import shutil
+import sys
+import tempfile
+
+from media_create import cmd_runner
+from media_create.ensure_command import ensure_command
+
+
+def install_hwpacks(chroot_dir, tmp_dir, hwpack_force_yes, *hwpack_files):
+    """Install the given hwpacks onto the given chroot."""
+
+    chroot_etc = os.path.join(chroot_dir, 'etc')
+    temporarily_overwrite_file_on_dir('/etc/resolv.conf', chroot_etc, tmp_dir)
+    temporarily_overwrite_file_on_dir('/etc/hosts', chroot_etc, tmp_dir)
+
+    if not platform.machine().startswith('arm'):
+        ensure_command('qemu-arm-static', 'qemu-arm-static')
+        ensure_command('qemu-img', 'qemu-kvm')
+        copy_file('/usr/bin/qemu-arm-static',
+                  os.path.join(chroot_dir, 'usr', 'bin'))
+
+    # FIXME: This is an ugly hack to make sure we use the l-h-i script from
+    # the current development tree when possible.
+    here = os.path.dirname(__file__)
+    linaro_hwpack_install_path = os.path.join(
+        here, '..', 'linaro-hwpack-install')
+    if not os.path.exists(linaro_hwpack_install_path):
+        linaro_hwpack_install_path = '/usr/bin/linaro-hwpack-install'
+    copy_file(linaro_hwpack_install_path,
+              os.path.join(chroot_dir, 'usr', 'bin'))
+
+    mount_chroot_proc(chroot_dir)
+
+    for hwpack_file in hwpack_files:
+        install_hwpack(chroot_dir, hwpack_file, hwpack_force_yes)
+
+
+def install_hwpack(chroot_dir, hwpack_file, hwpack_force_yes):
+    """Install an hwpack on the given chroot.
+
+    Copy the hwpack file to the chroot and run linaro-hwpack-install passing
+    that hwpack file to it.  If hwpack_force_yes is True, also pass
+    --force-yes to linaro-hwpack-install.
+    """
+    hwpack_basename = os.path.basename(hwpack_file)
+    copy_file(hwpack_file, chroot_dir)
+    print "-" * 60
+    print "Installing (apt-get) %s in target rootfs." % hwpack_basename
+    args = ['chroot', chroot_dir, 'linaro-hwpack-install']
+    if hwpack_force_yes:
+        args.append('--force-yes')
+    args.append('/%s' % hwpack_basename)
+    cmd_runner.run(args, as_root=True).wait()
+    print "-" * 60
+
+
+def mount_chroot_proc(chroot_dir):
+    """Mount a /proc filesystem on the given chroot.
+
+    Also register an atexit function to unmount that /proc filesystem.
+    """
+    chroot_proc = os.path.join(chroot_dir, 'proc')
+    proc = cmd_runner.run(
+        ['mount', 'proc', chroot_proc, '-t', 'proc'], as_root=True)
+    proc.wait()
+    def umount_chroot_proc():
+        cmd_runner.run(['umount', '-v', chroot_proc], as_root=True).wait()
+    atexit.register(umount_chroot_proc)
+
+
+def copy_file(filepath, directory):
+    """Copy the given file to the given directory.
+
+    The copying of the file is done in a subprocess and run using sudo.
+
+    We also register an atexit function to remove the file from the given
+    directory.
+    """
+    cmd_runner.run(['cp', filepath, directory], as_root=True).wait()
+
+    def undo():
+        new_path = os.path.join(directory, os.path.basename(filepath))
+        cmd_runner.run(['rm', '-f', new_path], as_root=True).wait()
+    atexit.register(undo)
+
+
+def temporarily_overwrite_file_on_dir(filepath, directory, tmp_dir):
+    """Temporarily replace a file on the given directory.
+
+    We'll move the existing file on the given directory to a temp dir, then
+    copy over the given file to that directory and register an atexit function
+    to move the orig file back to the given directory.
+    """
+    basename = os.path.basename(filepath)
+    path_to_orig = os.path.join(tmp_dir, basename)
+    # Move the existing file from the given directory to the temp dir.
+    cmd_runner.run(
+        ['mv', '-f', os.path.join(directory, basename), path_to_orig],
+        as_root=True).wait()
+    # Now copy the given file onto the given directory.
+    cmd_runner.run(['cp', filepath, directory], as_root=True).wait()
+
+    def undo():
+        cmd_runner.run(
+            ['mv', '-f', path_to_orig, directory], as_root=True).wait()
+    atexit.register(undo)
+
+
+if __name__ == '__main__':
+    tmp_dir = tempfile.mkdtemp()
+    atexit.register(shutil.rmtree, tmp_dir)
+    chroot_dir, hwpack_force_yes = sys.argv[1:3]
+    hwpack_force_yes = False
+    if hwpack_force_yes == "yes":
+        hwpack_force_yes = True
+    hwpacks = sys.argv[3:]
+    install_hwpacks(chroot_dir, tmp_dir, hwpack_force_yes, *hwpacks)

=== modified file 'media_create/tests/test_media_create.py'
--- media_create/tests/test_media_create.py	2010-12-20 10:00:07 +0000
+++ media_create/tests/test_media_create.py	2011-01-05 16:55:11 +0000
@@ -1,4 +1,5 @@
 from contextlib import contextmanager
+import atexit
 import os
 import random
 import string
@@ -17,6 +18,13 @@
 from media_create import partitions
 from media_create import rootfs
 from media_create.boot_cmd import create_boot_cmd
+from media_create.hwpack import (
+    copy_file,
+    install_hwpack,
+    install_hwpacks,
+    mount_chroot_proc,
+    temporarily_overwrite_file_on_dir,
+    )
 from media_create.partitions import (
     calculate_partition_size_and_offset,
     convert_size_to_bytes,
@@ -639,3 +647,116 @@
     def test_check_device_not_found(self):
         self._mock_does_device_exist_false()
         self.assertFalse(check_device.check_device(None))
+
+
+class AtExitRegister(object):
+    funcs = None
+    def __call__(self, func, *args, **kwargs):
+        if self.funcs is None:
+            self.funcs = []
+        self.funcs.append((func, args, kwargs))
+
+
+class TestInstallHWPack(TestCaseWithFixtures):
+
+    def test_temporarily_overwrite_file_on_dir(self):
+        fixture = self.useFixture(MockCmdRunnerPopenFixture())
+        temporarily_overwrite_file_on_dir('/path/to/file', '/dir', '/tmp/dir')
+        self.assertEquals(
+            [['sudo', 'mv', '-f', '/dir/file', '/tmp/dir/file'],
+             ['sudo', 'cp', '/path/to/file', '/dir']],
+            fixture.mock.calls)
+
+        fixture.mock.calls = []
+        self._run_registered_atexit_functions()
+        self.assertEquals(
+            [['sudo', 'mv', '-f', '/tmp/dir/file', '/dir']],
+            fixture.mock.calls)
+
+    def test_copy_file(self):
+        fixture = self.useFixture(MockCmdRunnerPopenFixture())
+        copy_file('/path/to/file', '/dir')
+        self.assertEquals(
+            [['sudo', 'cp', '/path/to/file', '/dir']],
+            fixture.mock.calls)
+
+        fixture.mock.calls = []
+        self._run_registered_atexit_functions()
+        self.assertEquals(
+            [['sudo', 'rm', '-f', '/dir/file']], fixture.mock.calls)
+
+    def test_mount_chroot_proc(self):
+        fixture = self.useFixture(MockCmdRunnerPopenFixture())
+        mount_chroot_proc('chroot')
+        self.assertEquals(
+            [['sudo', 'mount', 'proc', 'chroot/proc', '-t', 'proc']],
+            fixture.mock.calls)
+
+        fixture.mock.calls = []
+        self._run_registered_atexit_functions()
+        self.assertEquals(
+            [['sudo', 'umount', '-v', 'chroot/proc']], fixture.mock.calls)
+
+    def test_install_hwpack(self):
+        self.useFixture(MockSomethingFixture(
+            sys, 'stdout', open('/dev/null', 'w')))
+        fixture = self.useFixture(MockCmdRunnerPopenFixture())
+        force_yes = False
+        install_hwpack('chroot', 'hwpack.tgz', force_yes)
+        self.assertEquals(
+            [['sudo', 'cp', 'hwpack.tgz', 'chroot'],
+             ['sudo', 'chroot', 'chroot', 'linaro-hwpack-install',
+              '/hwpack.tgz']],
+            fixture.mock.calls)
+
+        fixture.mock.calls = []
+        self._run_registered_atexit_functions()
+        self.assertEquals(
+            [['sudo', 'rm', '-f', 'chroot/hwpack.tgz']], fixture.mock.calls)
+
+    def test_install_hwpacks(self):
+        self.useFixture(MockSomethingFixture(
+            sys, 'stdout', open('/dev/null', 'w')))
+        fixture = self.useFixture(MockCmdRunnerPopenFixture())
+        force_yes = True
+        install_hwpacks(
+            'chroot', '/tmp/dir', force_yes, 'hwpack1.tgz', 'hwpack2.tgz')
+        self.assertEquals(
+            [['sudo', 'mv', '-f', 'chroot/etc/resolv.conf',
+              '/tmp/dir/resolv.conf'],
+             ['sudo', 'cp', '/etc/resolv.conf', 'chroot/etc'],
+             ['sudo', 'mv', '-f', 'chroot/etc/hosts', '/tmp/dir/hosts'],
+             ['sudo', 'cp', '/etc/hosts', 'chroot/etc'],
+             ['sudo', 'cp', '/usr/bin/qemu-arm-static', 'chroot/usr/bin'],
+             ['sudo', 'cp', 'media_create/../linaro-hwpack-install',
+              'chroot/usr/bin'],
+             ['sudo', 'mount', 'proc', 'chroot/proc', '-t', 'proc'],
+             ['sudo', 'cp', 'hwpack1.tgz', 'chroot'],
+             ['sudo', 'chroot', 'chroot', 'linaro-hwpack-install',
+              '--force-yes', '/hwpack1.tgz'],
+             ['sudo', 'cp', 'hwpack2.tgz', 'chroot'],
+             ['sudo', 'chroot', 'chroot', 'linaro-hwpack-install',
+              '--force-yes', '/hwpack2.tgz']],
+            fixture.mock.calls)
+
+        fixture.mock.calls = []
+        self._run_registered_atexit_functions()
+        self.assertEquals(
+            [['sudo', 'mv', '-f', '/tmp/dir/resolv.conf', 'chroot/etc'],
+             ['sudo', 'mv', '-f', '/tmp/dir/hosts', 'chroot/etc'],
+             ['sudo', 'rm', '-f', 'chroot/usr/bin/qemu-arm-static'],
+             ['sudo', 'rm', '-f', 'chroot/usr/bin/linaro-hwpack-install'],
+             ['sudo', 'umount', '-v', 'chroot/proc'],
+             ['sudo', 'rm', '-f', 'chroot/hwpack1.tgz'],
+             ['sudo', 'rm', '-f', 'chroot/hwpack2.tgz']],
+            fixture.mock.calls)
+
+    def _run_registered_atexit_functions(self):
+        for func, args, kwargs in self.atexit_fixture.mock.funcs:
+            func(*args, **kwargs)
+
+    def setUp(self):
+        super(TestInstallHWPack, self).setUp()
+        self.atexit_fixture = self.useFixture(MockSomethingFixture(
+            atexit, 'register', AtExitRegister()))
+