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

Proposed by Guilherme Salgado
Status: Merged
Merged at revision: 377
Proposed branch: lp:~salgado/linaro-image-tools/bug-789093
Merge into: lp:linaro-image-tools/11.11
Diff against target: 85 lines (+40/-1)
2 files modified
linaro_image_tools/media_create/rootfs.py (+15/-1)
linaro_image_tools/media_create/tests/test_media_create.py (+25/-0)
To merge this branch: bzr merge lp:~salgado/linaro-image-tools/bug-789093
Reviewer Review Type Date Requested Status
James Tunnicliffe (community) Approve
Linaro Maintainers Pending
Review via email: mp+68266@code.launchpad.net

Description of the change

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

To post a comment you must log in.
Revision history for this message
James Tunnicliffe (dooferlad) wrote :

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

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
1919
20import glob20import glob
21import os21import os
22import subprocess
22import tempfile23import tempfile
2324
24from linaro_image_tools import cmd_runner25from linaro_image_tools import cmd_runner
@@ -107,13 +108,26 @@
107 flash_kernel, "UBOOT_PART=%s" % target_boot_dev)108 flash_kernel, "UBOOT_PART=%s" % target_boot_dev)
108109
109110
111def _list_files(directory):
112 """List the files and dirs under the given directory.
113
114 Runs as root because we want to list everything, including stuff that may
115 not be world-readable.
116 """
117 p = cmd_runner.run(
118 ['find', directory, '-maxdepth', '1', '-mindepth', '1'],
119 stdout=subprocess.PIPE, as_root=True)
120 stdout, _ = p.communicate()
121 return stdout.split()
122
123
110def move_contents(from_, root_disk):124def move_contents(from_, root_disk):
111 """Move everything under from_ to the given root disk.125 """Move everything under from_ to the given root disk.
112126
113 Uses sudo for moving.127 Uses sudo for moving.
114 """128 """
115 assert os.path.isdir(from_), "%s is not a directory" % from_129 assert os.path.isdir(from_), "%s is not a directory" % from_
116 files = glob.glob(os.path.join(from_, '*'))130 files = _list_files(from_)
117 mv_cmd = ['mv']131 mv_cmd = ['mv']
118 mv_cmd.extend(sorted(files))132 mv_cmd.extend(sorted(files))
119 mv_cmd.append(root_disk)133 mv_cmd.append(root_disk)
120134
=== 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 @@
2121 os.makedirs(contents_bin)2121 os.makedirs(contents_bin)
2122 os.makedirs(contents_etc)2122 os.makedirs(contents_etc)
21232123
2124 # Must mock rootfs._list_files() because populate_rootfs() uses its
2125 # return value but since we mock cmd_runner.run() _list_files() would
2126 # return an invalid value.
2127 def mock_list_files(directory):
2128 return [contents_bin, contents_etc]
2129 self.useFixture(MockSomethingFixture(
2130 rootfs, '_list_files', mock_list_files))
2131
2124 populate_rootfs(2132 populate_rootfs(
2125 contents_dir, root_disk, partition='/dev/rootfs',2133 contents_dir, root_disk, partition='/dev/rootfs',
2126 rootfs_type='ext3', rootfs_uuid='uuid', should_create_swap=True,2134 rootfs_type='ext3', rootfs_uuid='uuid', should_create_swap=True,
@@ -2162,10 +2170,27 @@
2162 fixture.mock.commands_executed[0])2170 fixture.mock.commands_executed[0])
2163 self.assertEqual('UBOOT_PART=/dev/mmcblk0p1', open(tmpfile).read())2171 self.assertEqual('UBOOT_PART=/dev/mmcblk0p1', open(tmpfile).read())
21642172
2173 def test_list_files(self):
2174 tempdir = self.useFixture(CreateTempDirFixture()).tempdir
2175 # We don't want to mock cmd_runner.run() because we're testing the
2176 # command that it runs, but we need to monkey-patch SUDO_ARGS because
2177 # we don't want to use 'sudo' in tests.
2178 orig_sudo_args = cmd_runner.SUDO_ARGS
2179 def restore_sudo_args():
2180 cmd_runner.SUDO_ARGS = orig_sudo_args
2181 self.addCleanup(restore_sudo_args)
2182 cmd_runner.SUDO_ARGS = []
2183 file1 = self.createTempFileAsFixture(dir=tempdir)
2184 self.assertEqual([file1], rootfs._list_files(tempdir))
2185
2165 def test_move_contents(self):2186 def test_move_contents(self):
2166 tempdir = self.useFixture(CreateTempDirFixture()).tempdir2187 tempdir = self.useFixture(CreateTempDirFixture()).tempdir
2167 popen_fixture = self.useFixture(MockCmdRunnerPopenFixture())2188 popen_fixture = self.useFixture(MockCmdRunnerPopenFixture())
2168 file1 = self.createTempFileAsFixture(dir=tempdir)2189 file1 = self.createTempFileAsFixture(dir=tempdir)
2190 def mock_list_files(directory):
2191 return [file1]
2192 self.useFixture(MockSomethingFixture(
2193 rootfs, '_list_files', mock_list_files))
21692194
2170 move_contents(tempdir, '/tmp/')2195 move_contents(tempdir, '/tmp/')
21712196

Subscribers

People subscribed via source and target branches