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
diff --git a/lib/lp/scripts/utilities/withxvfb.py b/lib/lp/scripts/utilities/withxvfb.py
index c466d82..992e830 100755
--- a/lib/lp/scripts/utilities/withxvfb.py
+++ b/lib/lp/scripts/utilities/withxvfb.py
@@ -11,16 +11,15 @@ Follows sinzui's advice to the launchpad-dev list:
11"""11"""
1212
13import os13import os
14import shutil
14import sys15import sys
1516
16from lp.services.osutils import find_on_path
17
1817
19def main():18def main():
20 # Look for the command to run in this script's directory if it's not19 # Look for the command to run in this script's directory if it's not
21 # found along the default PATH.20 # found along the default PATH.
22 args = sys.argv[1:]21 args = sys.argv[1:]
23 if args and not find_on_path(args[0]):22 if args and not shutil.which(args[0]):
24 nearby = os.path.join(os.path.dirname(sys.argv[0]), args[0])23 nearby = os.path.join(os.path.dirname(sys.argv[0]), args[0])
25 if os.access(nearby, os.X_OK):24 if os.access(nearby, os.X_OK):
26 args[0] = nearby25 args[0] = nearby
diff --git a/lib/lp/services/osutils.py b/lib/lp/services/osutils.py
index 75abdef..1a4e193 100644
--- a/lib/lp/services/osutils.py
+++ b/lib/lp/services/osutils.py
@@ -5,7 +5,6 @@
55
6__all__ = [6__all__ = [
7 'ensure_directory_exists',7 'ensure_directory_exists',
8 'find_on_path',
9 'get_pid_from_file',8 'get_pid_from_file',
10 'kill_by_pidfile',9 'kill_by_pidfile',
11 'open_for_writing',10 'open_for_writing',
@@ -192,20 +191,6 @@ def write_file(path, content):
192 f.write(content)191 f.write(content)
193192
194193
195def find_on_path(command):
196 """Is 'command' on the executable search path?"""
197 if "PATH" not in os.environ:
198 return False
199 path = os.environ["PATH"]
200 for element in path.split(os.pathsep):
201 if not element:
202 continue
203 filename = os.path.join(element, command)
204 if os.path.isfile(filename) and os.access(filename, os.X_OK):
205 return True
206 return False
207
208
209def process_exists(pid):194def process_exists(pid):
210 """Return True if the specified process already exists."""195 """Return True if the specified process already exists."""
211 try:196 try:
diff --git a/lib/lp/services/tests/test_osutils.py b/lib/lp/services/tests/test_osutils.py
index 4fca9f8..2b72da2 100644
--- a/lib/lp/services/tests/test_osutils.py
+++ b/lib/lp/services/tests/test_osutils.py
@@ -7,15 +7,11 @@ import errno
7import os7import os
8import tempfile8import tempfile
99
10from fixtures import (10from fixtures import MockPatch
11 EnvironmentVariable,
12 MockPatch,
13 )
14from testtools.matchers import FileContains11from testtools.matchers import FileContains
1512
16from lp.services.osutils import (13from lp.services.osutils import (
17 ensure_directory_exists,14 ensure_directory_exists,
18 find_on_path,
19 open_for_writing,15 open_for_writing,
20 process_exists,16 process_exists,
21 remove_tree,17 remove_tree,
@@ -117,29 +113,6 @@ class TestWriteFile(TestCase):
117 self.assertThat(filename, FileContains(content))113 self.assertThat(filename, FileContains(content))
118114
119115
120class TestFindOnPath(TestCase):
121
122 def test_missing_environment(self):
123 self.useFixture(EnvironmentVariable("PATH"))
124 self.assertFalse(find_on_path("ls"))
125
126 def test_present_executable(self):
127 temp_dir = self.makeTemporaryDirectory()
128 bin_dir = os.path.join(temp_dir, "bin")
129 program = os.path.join(bin_dir, "program")
130 write_file(program, b"")
131 os.chmod(program, 0o755)
132 self.useFixture(EnvironmentVariable("PATH", bin_dir))
133 self.assertTrue(find_on_path("program"))
134
135 def test_present_not_executable(self):
136 temp_dir = self.makeTemporaryDirectory()
137 bin_dir = os.path.join(temp_dir, "bin")
138 write_file(os.path.join(bin_dir, "program"), b"")
139 self.useFixture(EnvironmentVariable("PATH", bin_dir))
140 self.assertFalse(find_on_path("program"))
141
142
143class TestProcessExists(TestCase):116class TestProcessExists(TestCase):
144117
145 def test_with_process_running(self):118 def test_with_process_running(self):

Subscribers

People subscribed via source and target branches

to status/vote changes: