Merge lp:~salgado/linaro-image-tools/bug-789093 into lp:linaro-image-tools/11.11

[Guilherme Salgado]

Make sure rootfs.move_contents() doesn't skip files that are not
world-readable

[James Tunnicliffe (dooferlad)]

Looks good to me.

Prompts me to wonder again why we have any part of this process running as root other than writing an image to a device, but I will move that conversation to an email thread.

review: Approve

=== modified file 'linaro_image_tools/media_create/rootfs.py'
--- linaro_image_tools/media_create/rootfs.py	2011-04-05 09:26:47 +0000
+++ linaro_image_tools/media_create/rootfs.py	2011-07-18 15:18:40 +0000
@@ -19,6 +19,7 @@
 
 import glob
 import os
+import subprocess
 import tempfile
 
 from linaro_image_tools import cmd_runner
@@ -107,13 +108,26 @@
         flash_kernel, "UBOOT_PART=%s" % target_boot_dev)
 
 
+def _list_files(directory):
+    """List the files and dirs under the given directory.
+
+    Runs as root because we want to list everything, including stuff that may
+    not be world-readable.
+    """
+    p = cmd_runner.run(
+        ['find', directory, '-maxdepth', '1', '-mindepth', '1'],
+        stdout=subprocess.PIPE, as_root=True)
+    stdout, _ = p.communicate()
+    return stdout.split()
+
+
 def move_contents(from_, root_disk):
     """Move everything under from_ to the given root disk.
 
     Uses sudo for moving.
     """
     assert os.path.isdir(from_), "%s is not a directory" % from_
-    files = glob.glob(os.path.join(from_, '*'))
+    files = _list_files(from_)
     mv_cmd = ['mv']
     mv_cmd.extend(sorted(files))
     mv_cmd.append(root_disk)

=== modified file 'linaro_image_tools/media_create/tests/test_media_create.py'
--- linaro_image_tools/media_create/tests/test_media_create.py	2011-07-05 15:00:22 +0000
+++ linaro_image_tools/media_create/tests/test_media_create.py	2011-07-18 15:18:40 +0000
@@ -2121,6 +2121,14 @@
         os.makedirs(contents_bin)
         os.makedirs(contents_etc)
 
+        # Must mock rootfs._list_files() because populate_rootfs() uses its
+        # return value but since we mock cmd_runner.run() _list_files() would
+        # return an invalid value.
+        def mock_list_files(directory):
+            return [contents_bin, contents_etc]
+        self.useFixture(MockSomethingFixture(
+            rootfs, '_list_files', mock_list_files))
+
         populate_rootfs(
             contents_dir, root_disk, partition='/dev/rootfs',
             rootfs_type='ext3', rootfs_uuid='uuid', should_create_swap=True,
@@ -2162,10 +2170,27 @@
             fixture.mock.commands_executed[0])
         self.assertEqual('UBOOT_PART=/dev/mmcblk0p1', open(tmpfile).read())
 
+    def test_list_files(self):
+        tempdir = self.useFixture(CreateTempDirFixture()).tempdir
+        # We don't want to mock cmd_runner.run() because we're testing the
+        # command that it runs, but we need to monkey-patch SUDO_ARGS because
+        # we don't want to use 'sudo' in tests.
+        orig_sudo_args = cmd_runner.SUDO_ARGS
+        def restore_sudo_args():
+            cmd_runner.SUDO_ARGS = orig_sudo_args
+        self.addCleanup(restore_sudo_args)
+        cmd_runner.SUDO_ARGS = []
+        file1 = self.createTempFileAsFixture(dir=tempdir)
+        self.assertEqual([file1], rootfs._list_files(tempdir))
+
     def test_move_contents(self):
         tempdir = self.useFixture(CreateTempDirFixture()).tempdir
         popen_fixture = self.useFixture(MockCmdRunnerPopenFixture())
         file1 = self.createTempFileAsFixture(dir=tempdir)
+        def mock_list_files(directory):
+            return [file1]
+        self.useFixture(MockSomethingFixture(
+            rootfs, '_list_files', mock_list_files))
 
         move_contents(tempdir, '/tmp/')