[Branch,~linaro-maintainers/linaro-image-tools/trunk] Rev 275: Fix bug 710971 by making sure OmapConfig.serial_tty is never used before set_appropriate_serial_t...

Message ID 20110201145733.7254.65206.launchpad@loganberry.canonical.com
State Accepted
Headers show

Commit Message

Guilherme Salgado Feb. 1, 2011, 2:57 p.m.
Merge authors:
  Guilherme Salgado (salgado)
Related merge proposals:
  https://code.launchpad.net/~salgado/linaro-image-tools/bug-710971/+merge/48154
  proposed by: Guilherme Salgado (salgado)
  review: Approve - James Westby (james-w)
------------------------------------------------------------
revno: 275 [merge]
committer: Guilherme Salgado <salgado@canonical.com>
branch nick: trunk
timestamp: Tue 2011-02-01 12:53:09 -0200
message:
  Fix bug 710971 by making sure OmapConfig.serial_tty is never used before set_appropriate_serial_tty() is called.
modified:
  linaro_media_create/boards.py
  linaro_media_create/tests/test_media_create.py


--
lp:linaro-image-tools
https://code.launchpad.net/~linaro-maintainers/linaro-image-tools/trunk

You are subscribed to branch lp:linaro-image-tools.
To unsubscribe from this branch go to https://code.launchpad.net/~linaro-maintainers/linaro-image-tools/trunk/+edit-subscription

Patch

=== modified file 'linaro_media_create/boards.py'
--- linaro_media_create/boards.py	2011-01-28 19:50:48 +0000
+++ linaro_media_create/boards.py	2011-02-01 13:16:43 +0000
@@ -146,6 +146,15 @@ 
     _serial_tty = None
 
     @classproperty
+    def serial_tty(cls):
+        # This is just to make sure no callsites use .serial_tty before
+        # calling set_appropriate_serial_tty(). If we had this in the first
+        # place we'd have uncovered bug 710971 before releasing.
+        raise AttributeError(
+            "You must not use this attribute before calling "
+            "set_appropriate_serial_tty")
+
+    @classproperty
     def live_serial_opts(cls):
         return cls._live_serial_opts % cls.serial_tty
 
@@ -161,18 +170,29 @@ 
         we use the default value (_serial_tty).
         """
         # XXX: This is also part of our temporary hack to fix bug 697824.
-        cls.serial_tty = cls._serial_tty
+        cls.serial_tty = classproperty(lambda cls: cls._serial_tty)
         vmlinuz = _get_file_matching(
             os.path.join(chroot_dir, 'boot', 'vmlinuz*'))
         basename = os.path.basename(vmlinuz)
         minor_version = re.match('.*2\.6\.([0-9]{2}).*', basename).group(1)
         if int(minor_version) < 36:
-            cls.serial_tty = 'ttyS2'
+            cls.serial_tty = classproperty(lambda cls: 'ttyS2')
+
+    @classmethod
+    def make_boot_files(cls, uboot_parts_dir, is_live, is_lowmem, consoles,
+                        root_dir, rootfs_uuid, boot_dir, boot_script,
+                        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
+        # may use cls.serial_tty.
+        cls.set_appropriate_serial_tty(root_dir)
+        super(OmapConfig, cls).make_boot_files(
+            uboot_parts_dir, is_live, is_lowmem, consoles, root_dir,
+            rootfs_uuid, boot_dir, boot_script, boot_device_or_file)
 
     @classmethod
     def _make_boot_files(cls, uboot_parts_dir, boot_cmd, chroot_dir,
                          boot_dir, boot_script, boot_device_or_file):
-        cls.set_appropriate_serial_tty(chroot_dir)
         install_omap_boot_loader(chroot_dir, boot_dir)
         make_uImage(
             cls.load_addr, uboot_parts_dir, cls.kernel_suffix, boot_dir)

=== modified file 'linaro_media_create/tests/test_media_create.py'
--- linaro_media_create/tests/test_media_create.py	2011-01-28 19:50:48 +0000
+++ linaro_media_create/tests/test_media_create.py	2011-02-01 13:16:43 +0000
@@ -3,7 +3,7 @@ 
 # Author: Guilherme Salgado <guilherme.salgado@linaro.org>
 #
 # 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
@@ -42,6 +42,7 @@ 
     )
 import linaro_media_create
 from linaro_media_create.boards import (
+    BoardConfig,
     board_configs,
     make_boot_script,
     make_uImage,
@@ -248,6 +249,32 @@ 
 
 class TestFixForBug697824(TestCaseWithFixtures):
 
+    def mock_set_appropriate_serial_tty(self, config):
+
+        def set_appropriate_serial_tty_mock(cls, chroot_dir):
+            self.set_appropriate_serial_tty_called = True
+            cls.serial_tty = cls._serial_tty
+
+        self.useFixture(MockSomethingFixture(
+            config, 'set_appropriate_serial_tty',
+            classmethod(set_appropriate_serial_tty_mock)))
+
+    def test_omap_make_boot_files(self):
+        self.set_appropriate_serial_tty_called = False
+        self.mock_set_appropriate_serial_tty(board_configs['beagle'])
+        self.useFixture(MockSomethingFixture(
+            BoardConfig, 'make_boot_files',
+            classmethod(lambda *args: None)))
+        # We don't need to worry about what's passed to make_boot_files()
+        # because we mock the method which does the real work above and here
+        # we're only interested in ensuring that OmapConfig.make_boot_files()
+        # calls set_appropriate_serial_tty().
+        board_configs['beagle'].make_boot_files(
+            None, None, None, None, None, None, None, None, None)
+        self.assertTrue(
+            self.set_appropriate_serial_tty_called,
+            "make_boot_files didn't call set_appropriate_serial_tty")
+
     def test_set_appropriate_serial_tty_old_kernel(self):
         tempdir = self.useFixture(CreateTempDirFixture()).tempdir
         boot_dir = os.path.join(tempdir, 'boot')
@@ -316,7 +343,12 @@ 
         self.assertEqual(expected, boot_cmd)
 
     def test_panda(self):
-        boot_cmd = board_configs['panda']._get_boot_cmd(
+        # XXX: To fix bug 697824 we have to change class attributes of our
+        # OMAP board configs, and some tests do that so to make sure they
+        # don't interfere with us we'll reset that before doing anything.
+        config = board_configs['panda']
+        config.serial_tty = config._serial_tty
+        boot_cmd = config._get_boot_cmd(
             is_live=False, is_lowmem=False, consoles=None,
             rootfs_uuid="deadbeef")
         expected = (
@@ -347,7 +379,12 @@ 
         self.assertEqual(expected, boot_cmd)
 
     def test_overo(self):
-        boot_cmd = board_configs['overo']._get_boot_cmd(
+        # XXX: To fix bug 697824 we have to change class attributes of our
+        # OMAP board configs, and some tests do that so to make sure they
+        # don't interfere with us we'll reset that before doing anything.
+        config = board_configs['overo']
+        config.serial_tty = config._serial_tty
+        boot_cmd = config._get_boot_cmd(
             is_live=False, is_lowmem=False, consoles=None,
             rootfs_uuid="deadbeef")
         expected = (