Merge lp:~lool/linaro-image-tools/testsuite-when-installed into lp:linaro-image-tools/11.11
- testsuite-when-installed
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 284 |
Proposed branch: | lp:~lool/linaro-image-tools/testsuite-when-installed |
Merge into: | lp:linaro-image-tools/11.11 |
Prerequisite: | lp:~lool/linaro-image-tools/optional-sudo |
Diff against target: |
228 lines (+104/-13) (has conflicts) 4 files modified
linaro-media-create (+22/-1) linaro_media_create/hwpack.py (+11/-9) linaro_media_create/tests/test_media_create.py (+39/-3) linaro_media_create/utils.py (+32/-0) Text conflict in linaro-media-create |
To merge this branch: | bzr merge lp:~lool/linaro-image-tools/testsuite-when-installed |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
James Westby (community) | Approve | ||
Review via email: mp+48193@code.launchpad.net |
This proposal supersedes a proposal from 2011-02-01.
Commit message
Description of the change
Loïc Minier (lool) wrote : | # |
(Thanks for adding the dep on the sudo branch; I failed to push the sudo branch after merging it locally -- it's merged in trunk now)
The isbas(__file__) test is just a test for whether or not we're running from bzr.
Note that __file__ is the possibly relative pathname of the current module, so of hwpack.py or utils.py.
The assumptions are:
* if you're running from a checkout, __file__ is relative, you're in the top directory and commands are in the current directory
* if you're not running from a checkout, __file__ is absolute, commands are in the PATH
Loïc Minier (lool) wrote : | # |
Followup IRC chat:
15:09 < james_w> lool, We have to be very careful to use a matching
15:10 < james_w> lool, so when looking for that command, I think we should
15:10 < james_w> I think your change will approximate that in most cases, but
15:11 < lool> james_w: So you're saying, additionally to all things I've
where linaro-media-create is, then fallback to PATH?
15:13 < james_w> lool, that's what I think
15:13 < lool> james_w: It makes sense
15:14 < lool> james_w: albeit I can think that if you have multiple
don't point at the same versions in the same order
15:14 < james_w> lool, indeed
15:15 < lool> james_w: Do you still want this covered? We could do it either
by passing directories across calls, or by prepending PATH
15:15 < lool> But prepending PATH might have side effects when looking for
other commands
15:16 < james_w> lool, right, I imagine it should just be for
Loïc Minier (lool) wrote : | # |
Ok; I pushed up to r280 to implement support for preferring the dir with which linaro-media-create is run; it was a bit more intrusive than I wished for
- 278. By Loïc Minier
-
Add an optional prefer_dir arg to find_command to allow searching a specific
directory first. - 279. By Loïc Minier
-
Pass a tools dir (preferred directory) to install_hwpacks; should be the
directory of linaro-media-create when it's run with an explicit directory, or
"." when running from a bzr checkout. - 280. By Loïc Minier
-
More explicit handling of empty PATH components as "." in find_command.
Loïc Minier (lool) wrote : | # |
I tried supporting hwpack.tests as well, and had to add r281 for this; with r281 and this diff:
=== modified file 'hwpack/
--- hwpack/
+++ hwpack/
@@ -31,6 +31,7 @@
IsHardware
TestCaseWi
)
+from linaro_
class ScriptTests(
@@ -38,26 +39,16 @@
def setUp(self):
- self.script_path = self.find_script()
+
+ prefer_dir = None
+ # running from bzr checkout?
+ if not os.path.
+ prefer_dir = os.getcwd()
+
+ self.script_path = find_command(
+ 'linaro-
- def find_script(self):
- script_name = "linaro-
- this_path = os.path.
- parent_path = this_path
- for i in range(3):
- parent_path = os.path.
- possible_paths = [
- os.path.
- os.path.join("usr", "local", "bin", script_name),
- os.path.join("usr", "bin", script_name),
- ]
- for script_path in possible_paths:
- if os.path.
- return script_path
- raise AssertionError(
- "Could not find linaro-
-
def run_script(self, args, expected_
cmdline = [self.script_path] + args
proc = subprocess.Popen(
all tests pass from a bzr checkout and when installed.
- 281. By Loïc Minier
-
Default to os.getcwd() instead of "." as "." breaks with some
ChdirToTempdirFixture based tests.
James Westby (james-w) wrote : | # |
Looks good, aside from the merge conflicts.
Thanks,
James
Preview Diff
1 | === modified file 'linaro-media-create' |
2 | --- linaro-media-create 2011-02-02 12:08:05 +0000 |
3 | +++ linaro-media-create 2011-02-02 16:43:03 +0000 |
4 | @@ -62,6 +62,7 @@ |
5 | Before doing so, make sure BOOT_DISK and ROOT_DISK are not mounted. |
6 | """ |
7 | devnull = open('/dev/null', 'w') |
8 | +<<<<<<< TREE |
9 | # ignore non-zero return codes |
10 | for disk in BOOT_DISK, ROOT_DISK: |
11 | if disk is not None: |
12 | @@ -70,10 +71,26 @@ |
13 | stdout=devnull, stderr=devnull, as_root=True).wait() |
14 | except cmd_runner.SubcommandNonZeroReturnValue: |
15 | pass |
16 | +======= |
17 | + # ignore non-zero return codes |
18 | + try: |
19 | + if BOOT_DISK is not None: |
20 | + cmd_runner.Popen(['umount', BOOT_DISK], |
21 | + stdout=devnull, stderr=devnull, as_root=True).wait() |
22 | + if ROOT_DISK is not None: |
23 | + cmd_runner.Popen(['umount', ROOT_DISK], |
24 | + stdout=devnull, stderr=devnull, as_root=True).wait() |
25 | + except SubcommandNonZeroReturnValue: |
26 | + pass |
27 | +>>>>>>> MERGE-SOURCE |
28 | # Remove TMP_DIR as root because some files written there are |
29 | # owned by root. |
30 | if TMP_DIR is not None: |
31 | +<<<<<<< TREE |
32 | cmd_runner.run(['rm', '-rf', TMP_DIR], as_root=True).wait() |
33 | +======= |
34 | + cmd_runner.Popen(['rm', '-rf', TMP_DIR], as_root=True).wait() |
35 | +>>>>>>> MERGE-SOURCE |
36 | |
37 | |
38 | def ensure_required_commands(args): |
39 | @@ -121,7 +138,11 @@ |
40 | unpack_binary_tarball(args.binary, TMP_DIR) |
41 | |
42 | hwpacks = args.hwpacks |
43 | - install_hwpacks(ROOTFS_DIR, TMP_DIR, args.hwpack_force_yes, *hwpacks) |
44 | + lmc_dir = os.path.dirname(__file__) |
45 | + if lmc_dir == '': |
46 | + lmc_dir = None |
47 | + install_hwpacks( |
48 | + ROOTFS_DIR, TMP_DIR, lmc_dir, args.hwpack_force_yes, *hwpacks) |
49 | |
50 | boot_partition, root_partition = setup_partitions( |
51 | board_config, media, args.image_size, args.boot_label, args.rfs_label, |
52 | |
53 | === modified file 'linaro_media_create/hwpack.py' |
54 | --- linaro_media_create/hwpack.py 2011-02-01 17:07:04 +0000 |
55 | +++ linaro_media_create/hwpack.py 2011-02-02 16:43:03 +0000 |
56 | @@ -21,7 +21,10 @@ |
57 | import sys |
58 | |
59 | from linaro_media_create import cmd_runner |
60 | -from linaro_media_create.utils import is_arm_host |
61 | +from linaro_media_create.utils import ( |
62 | + is_arm_host, |
63 | + find_command, |
64 | + ) |
65 | |
66 | |
67 | # It'd be nice if we could use atexit here, but all the things we need to undo |
68 | @@ -29,7 +32,8 @@ |
69 | # functions would only be called after l-m-c.py exits. |
70 | local_atexit = [] |
71 | |
72 | -def install_hwpacks(chroot_dir, tmp_dir, hwpack_force_yes, *hwpack_files): |
73 | +def install_hwpacks( |
74 | + chroot_dir, tmp_dir, tools_dir, hwpack_force_yes, *hwpack_files): |
75 | """Install the given hwpacks onto the given chroot.""" |
76 | |
77 | chroot_etc = os.path.join(chroot_dir, 'etc') |
78 | @@ -40,13 +44,11 @@ |
79 | copy_file('/usr/bin/qemu-arm-static', |
80 | os.path.join(chroot_dir, 'usr', 'bin')) |
81 | |
82 | - # FIXME: This is an ugly hack to make sure we use the l-h-i script from |
83 | - # the current development tree when possible. |
84 | - here = os.path.dirname(__file__) |
85 | - linaro_hwpack_install_path = os.path.join( |
86 | - here, '..', 'linaro-hwpack-install') |
87 | - if not os.path.exists(linaro_hwpack_install_path): |
88 | - linaro_hwpack_install_path = '/usr/bin/linaro-hwpack-install' |
89 | + linaro_hwpack_install_path = find_command( |
90 | + 'linaro-hwpack-install', prefer_dir=tools_dir) |
91 | + # FIXME: shouldn't use chroot/usr/bin as this might conflict with installed |
92 | + # packages; would be best to use some custom directory like |
93 | + # chroot/linaro-image-tools/bin |
94 | copy_file(linaro_hwpack_install_path, |
95 | os.path.join(chroot_dir, 'usr', 'bin')) |
96 | |
97 | |
98 | === modified file 'linaro_media_create/tests/test_media_create.py' |
99 | --- linaro_media_create/tests/test_media_create.py 2011-02-01 17:29:25 +0000 |
100 | +++ linaro_media_create/tests/test_media_create.py 2011-02-02 16:43:03 +0000 |
101 | @@ -21,6 +21,7 @@ |
102 | import glob |
103 | import os |
104 | import random |
105 | +import stat |
106 | import string |
107 | import subprocess |
108 | import sys |
109 | @@ -82,6 +83,7 @@ |
110 | from linaro_media_create.unpack_binary_tarball import unpack_binary_tarball |
111 | from linaro_media_create.utils import ( |
112 | ensure_command, |
113 | + find_command, |
114 | install_package_providing, |
115 | UnableToFindPackageProvidingCommand, |
116 | ) |
117 | @@ -121,6 +123,32 @@ |
118 | utils, 'install_package_providing', mock_func)) |
119 | |
120 | |
121 | +class TestFindCommand(TestCaseWithFixtures): |
122 | + |
123 | + def test_preferred_dir(self): |
124 | + tempdir = self.useFixture(CreateTempDirFixture()).get_temp_dir() |
125 | + lmc = 'linaro-media-create' |
126 | + path = os.path.join(tempdir, lmc) |
127 | + open(path, 'w').close() |
128 | + os.chmod(path, stat.S_IXUSR) |
129 | + self.assertEquals(path, find_command(lmc, tempdir)) |
130 | + |
131 | + def test_existing_command(self): |
132 | + lmc = 'linaro-media-create' |
133 | + # running from bzr checkout? |
134 | + if os.path.isabs(__file__): |
135 | + expected, _ = cmd_runner.run( |
136 | + ['which', lmc, ], |
137 | + stdout=subprocess.PIPE).communicate() |
138 | + expected = expected.strip() |
139 | + else: |
140 | + expected = os.path.join(os.getcwd(), lmc) |
141 | + self.assertEquals(expected, find_command(lmc)) |
142 | + |
143 | + def test_nonexisting_command(self): |
144 | + self.assertEquals(find_command('linaro-moo'), None) |
145 | + |
146 | + |
147 | class TestInstallPackageProviding(TestCaseWithFixtures): |
148 | |
149 | def test_found_package(self): |
150 | @@ -1083,8 +1111,17 @@ |
151 | sys, 'stdout', open('/dev/null', 'w'))) |
152 | fixture = self.useFixture(MockCmdRunnerPopenFixture()) |
153 | force_yes = True |
154 | + |
155 | + prefer_dir = None |
156 | + # running from bzr checkout? |
157 | + if not os.path.isabs(__file__): |
158 | + prefer_dir = os.getcwd() |
159 | + |
160 | install_hwpacks( |
161 | - 'chroot', '/tmp/dir', force_yes, 'hwpack1.tgz', 'hwpack2.tgz') |
162 | + 'chroot', '/tmp/dir', prefer_dir, force_yes, 'hwpack1.tgz', |
163 | + 'hwpack2.tgz') |
164 | + linaro_hwpack_install = find_command( |
165 | + 'linaro-hwpack-install', prefer_dir=prefer_dir) |
166 | self.assertEquals( |
167 | [['sudo', 'mv', '-f', 'chroot/etc/resolv.conf', |
168 | '/tmp/dir/resolv.conf'], |
169 | @@ -1092,8 +1129,7 @@ |
170 | ['sudo', 'mv', '-f', 'chroot/etc/hosts', '/tmp/dir/hosts'], |
171 | ['sudo', 'cp', '/etc/hosts', 'chroot/etc'], |
172 | ['sudo', 'cp', '/usr/bin/qemu-arm-static', 'chroot/usr/bin'], |
173 | - ['sudo', 'cp', 'linaro_media_create/../linaro-hwpack-install', |
174 | - 'chroot/usr/bin'], |
175 | + ['sudo', 'cp', linaro_hwpack_install, 'chroot/usr/bin'], |
176 | ['sudo', 'mount', 'proc', 'chroot/proc', '-t', 'proc'], |
177 | ['sudo', 'cp', 'hwpack1.tgz', 'chroot'], |
178 | ['sudo', 'chroot', 'chroot', 'linaro-hwpack-install', |
179 | |
180 | === modified file 'linaro_media_create/utils.py' |
181 | --- linaro_media_create/utils.py 2011-01-28 19:50:48 +0000 |
182 | +++ linaro_media_create/utils.py 2011-02-02 16:43:03 +0000 |
183 | @@ -17,6 +17,7 @@ |
184 | # You should have received a copy of the GNU General Public License |
185 | # along with Linaro Image Tools. If not, see <http://www.gnu.org/licenses/>. |
186 | |
187 | +import os |
188 | import platform |
189 | |
190 | try: |
191 | @@ -61,6 +62,37 @@ |
192 | except cmd_runner.SubcommandNonZeroReturnValue: |
193 | install_package_providing(command) |
194 | |
195 | +def find_command(name, prefer_dir=None): |
196 | + """Finds a linaro-image-tools command. |
197 | + |
198 | + Prefers specified directory, otherwise searches only the current directory |
199 | + when running from a checkout, or only PATH when running from an installed |
200 | + version. |
201 | + """ |
202 | + assert name != "" |
203 | + assert os.path.dirname(name) == "" |
204 | + |
205 | + if not os.environ.has_key("PATH"): |
206 | + os.environ["PATH"] = ":/bin:usr/bin" |
207 | + |
208 | + # default to searching in current directory when running from a bzr |
209 | + # checkout |
210 | + dirs = [os.getcwd(),] |
211 | + if os.path.isabs(__file__): |
212 | + dirs = os.environ["PATH"].split(os.pathsep) |
213 | + # empty dir in PATH means current directory |
214 | + dirs = map(lambda x: x == '' and '.' or x, dirs) |
215 | + |
216 | + if prefer_dir is not None: |
217 | + dirs.insert(0, prefer_dir) |
218 | + |
219 | + for dir in dirs: |
220 | + path = os.path.join(dir, name) |
221 | + if os.path.exists(path) and os.access(path, os.X_OK): |
222 | + return path |
223 | + |
224 | + return None |
225 | + |
226 | |
227 | def is_arm_host(): |
228 | return platform.machine().startswith('arm') |
I'm not sure that the
116 + if os.path. isabs(_ _file__ ):
check is really what we want.
I think that if there is a linaro- hwpack- install alongside the linaro- media-create,
then we should always use it, regardless of $PATH, or how it is being executed.
That's probably not true of all commands though.
It's probably worth looking back over the discussion around using the correct hwpack- install from when the code to use one in the current dir was
linaro-
added.
Thanks,
James