Merge ~cjwatson/launchpad:remove-find-on-path into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 374cbeec719ae318a59e20717b13998d7da1e781
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:remove-find-on-path
Merge into: launchpad:master
Diff against target: 107 lines (+3/-46)
3 files modified
lib/lp/scripts/utilities/withxvfb.py (+2/-3)
lib/lp/services/osutils.py (+0/-15)
lib/lp/services/tests/test_osutils.py (+1/-28)
Reviewer Review Type Date Requested Status
Jürgen Gmach Approve
Guruprasad Approve
Review via email: mp+411509@code.launchpad.net

Commit message

Remove find_on_path in favour of shutil.which

To post a comment you must log in.
Revision history for this message
Guruprasad (lgp171188) wrote :

These changes look straightforward and good to me. 👍

review: Approve
Revision history for this message
Jürgen Gmach (jugmac00) wrote :

LGTM

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/lib/lp/scripts/utilities/withxvfb.py b/lib/lp/scripts/utilities/withxvfb.py
2index c466d82..992e830 100755
3--- a/lib/lp/scripts/utilities/withxvfb.py
4+++ b/lib/lp/scripts/utilities/withxvfb.py
5@@ -11,16 +11,15 @@ Follows sinzui's advice to the launchpad-dev list:
6 """
7
8 import os
9+import shutil
10 import sys
11
12-from lp.services.osutils import find_on_path
13-
14
15 def main():
16 # Look for the command to run in this script's directory if it's not
17 # found along the default PATH.
18 args = sys.argv[1:]
19- if args and not find_on_path(args[0]):
20+ if args and not shutil.which(args[0]):
21 nearby = os.path.join(os.path.dirname(sys.argv[0]), args[0])
22 if os.access(nearby, os.X_OK):
23 args[0] = nearby
24diff --git a/lib/lp/services/osutils.py b/lib/lp/services/osutils.py
25index 75abdef..1a4e193 100644
26--- a/lib/lp/services/osutils.py
27+++ b/lib/lp/services/osutils.py
28@@ -5,7 +5,6 @@
29
30 __all__ = [
31 'ensure_directory_exists',
32- 'find_on_path',
33 'get_pid_from_file',
34 'kill_by_pidfile',
35 'open_for_writing',
36@@ -192,20 +191,6 @@ def write_file(path, content):
37 f.write(content)
38
39
40-def find_on_path(command):
41- """Is 'command' on the executable search path?"""
42- if "PATH" not in os.environ:
43- return False
44- path = os.environ["PATH"]
45- for element in path.split(os.pathsep):
46- if not element:
47- continue
48- filename = os.path.join(element, command)
49- if os.path.isfile(filename) and os.access(filename, os.X_OK):
50- return True
51- return False
52-
53-
54 def process_exists(pid):
55 """Return True if the specified process already exists."""
56 try:
57diff --git a/lib/lp/services/tests/test_osutils.py b/lib/lp/services/tests/test_osutils.py
58index 4fca9f8..2b72da2 100644
59--- a/lib/lp/services/tests/test_osutils.py
60+++ b/lib/lp/services/tests/test_osutils.py
61@@ -7,15 +7,11 @@ import errno
62 import os
63 import tempfile
64
65-from fixtures import (
66- EnvironmentVariable,
67- MockPatch,
68- )
69+from fixtures import MockPatch
70 from testtools.matchers import FileContains
71
72 from lp.services.osutils import (
73 ensure_directory_exists,
74- find_on_path,
75 open_for_writing,
76 process_exists,
77 remove_tree,
78@@ -117,29 +113,6 @@ class TestWriteFile(TestCase):
79 self.assertThat(filename, FileContains(content))
80
81
82-class TestFindOnPath(TestCase):
83-
84- def test_missing_environment(self):
85- self.useFixture(EnvironmentVariable("PATH"))
86- self.assertFalse(find_on_path("ls"))
87-
88- def test_present_executable(self):
89- temp_dir = self.makeTemporaryDirectory()
90- bin_dir = os.path.join(temp_dir, "bin")
91- program = os.path.join(bin_dir, "program")
92- write_file(program, b"")
93- os.chmod(program, 0o755)
94- self.useFixture(EnvironmentVariable("PATH", bin_dir))
95- self.assertTrue(find_on_path("program"))
96-
97- def test_present_not_executable(self):
98- temp_dir = self.makeTemporaryDirectory()
99- bin_dir = os.path.join(temp_dir, "bin")
100- write_file(os.path.join(bin_dir, "program"), b"")
101- self.useFixture(EnvironmentVariable("PATH", bin_dir))
102- self.assertFalse(find_on_path("program"))
103-
104-
105 class TestProcessExists(TestCase):
106
107 def test_with_process_running(self):

Subscribers

People subscribed via source and target branches

to status/vote changes: