Merge lp:~broder/ubuntu-dev-tools/fix-785854 into lp:~ubuntu-dev/ubuntu-dev-tools/trunk
- fix-785854
- Merge into trunk
Proposed by
Evan Broder
Status: | Superseded |
---|---|
Proposed branch: | lp:~broder/ubuntu-dev-tools/fix-785854 |
Merge into: | lp:~ubuntu-dev/ubuntu-dev-tools/trunk |
Diff against target: |
486 lines (+147/-19) (has conflicts) 21 files modified
404main (+2/-1) ack-sync (+1/-1) backportpackage (+1/-1) debian/changelog (+11/-0) dgetlp (+2/-1) get-branches (+1/-1) import-bug-from-debian (+1/-1) lp-project-upload (+2/-1) pbuilder-dist (+1/-1) syncpackage (+1/-1) ubuntu-iso (+2/-1) ubuntutools/archive.py (+1/-1) ubuntutools/builder.py (+1/-1) ubuntutools/misc.py (+1/-1) ubuntutools/requestsync/common.py (+2/-1) ubuntutools/requestsync/mail.py (+1/-1) ubuntutools/sponsor_patch/patch.py (+2/-1) ubuntutools/sponsor_patch/sponsor_patch.py (+1/-1) ubuntutools/subprocess.py (+109/-0) ubuntutools/test/test_help.py (+1/-1) ubuntutools/test/test_pylint.py (+3/-1) Text conflict in debian/changelog |
To merge this branch: | bzr merge lp:~broder/ubuntu-dev-tools/fix-785854 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Barry Warsaw (community) | Needs Fixing | ||
Review via email: mp+62180@code.launchpad.net |
This proposal has been superseded by a proposal from 2011-06-19.
Commit message
Description of the change
To post a comment you must log in.
- 1093. By Evan Broder
-
Reimplement ubuntutools.
subprocess. Popen as a class instead of a factory function.
Unmerged revisions
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file '404main' | |||
2 | --- 404main 2010-12-27 19:32:07 +0000 | |||
3 | +++ 404main 2011-06-19 21:23:31 +0000 | |||
4 | @@ -24,12 +24,13 @@ | |||
5 | 24 | # This script is used to check if a package and all its build | 24 | # This script is used to check if a package and all its build |
6 | 25 | # dependencies are in main or not. | 25 | # dependencies are in main or not. |
7 | 26 | 26 | ||
8 | 27 | import subprocess | ||
9 | 28 | import sys | 27 | import sys |
10 | 29 | 28 | ||
11 | 30 | import apt_pkg | 29 | import apt_pkg |
12 | 31 | import apt | 30 | import apt |
13 | 32 | 31 | ||
14 | 32 | from ubuntutools import subprocess | ||
15 | 33 | |||
16 | 33 | def process_deps(cache, deps): | 34 | def process_deps(cache, deps): |
17 | 34 | """Takes a list of (build) dependencies and processes it.""" | 35 | """Takes a list of (build) dependencies and processes it.""" |
18 | 35 | 36 | ||
19 | 36 | 37 | ||
20 | === modified file 'ack-sync' | |||
21 | --- ack-sync 2011-05-23 21:44:45 +0000 | |||
22 | +++ ack-sync 2011-06-19 21:23:31 +0000 | |||
23 | @@ -22,7 +22,6 @@ | |||
24 | 22 | import lazr.restfulclient | 22 | import lazr.restfulclient |
25 | 23 | import os | 23 | import os |
26 | 24 | import re | 24 | import re |
27 | 25 | import subprocess | ||
28 | 26 | import sys | 25 | import sys |
29 | 27 | import logging | 26 | import logging |
30 | 28 | import glob | 27 | import glob |
31 | @@ -31,6 +30,7 @@ | |||
32 | 31 | from launchpadlib.launchpad import Launchpad | 30 | from launchpadlib.launchpad import Launchpad |
33 | 32 | 31 | ||
34 | 33 | from ubuntutools.config import UDTConfig | 32 | from ubuntutools.config import UDTConfig |
35 | 33 | from ubuntutools import subprocess | ||
36 | 34 | 34 | ||
37 | 35 | COMMAND_LINE_SYNTAX_ERROR = 1 | 35 | COMMAND_LINE_SYNTAX_ERROR = 1 |
38 | 36 | VERSION_DETECTION_FAILED = 2 | 36 | VERSION_DETECTION_FAILED = 2 |
39 | 37 | 37 | ||
40 | === modified file 'backportpackage' | |||
41 | --- backportpackage 2011-05-28 17:41:07 +0000 | |||
42 | +++ backportpackage 2011-06-19 21:23:31 +0000 | |||
43 | @@ -21,7 +21,6 @@ | |||
44 | 21 | import optparse | 21 | import optparse |
45 | 22 | import os | 22 | import os |
46 | 23 | import shutil | 23 | import shutil |
47 | 24 | import subprocess | ||
48 | 25 | import sys | 24 | import sys |
49 | 26 | import tempfile | 25 | import tempfile |
50 | 27 | 26 | ||
51 | @@ -34,6 +33,7 @@ | |||
52 | 34 | from ubuntutools.config import UDTConfig, ubu_email | 33 | from ubuntutools.config import UDTConfig, ubu_email |
53 | 35 | from ubuntutools.builder import get_builder | 34 | from ubuntutools.builder import get_builder |
54 | 36 | from ubuntutools.question import YesNoQuestion | 35 | from ubuntutools.question import YesNoQuestion |
55 | 36 | from ubuntutools import subprocess | ||
56 | 37 | 37 | ||
57 | 38 | def error(msg): | 38 | def error(msg): |
58 | 39 | Logger.error(msg) | 39 | Logger.error(msg) |
59 | 40 | 40 | ||
60 | === modified file 'debian/changelog' | |||
61 | --- debian/changelog 2011-06-16 19:02:04 +0000 | |||
62 | +++ debian/changelog 2011-06-19 21:23:31 +0000 | |||
63 | @@ -47,7 +47,18 @@ | |||
64 | 47 | * mk-sbuild: | 47 | * mk-sbuild: |
65 | 48 | - maintainer_name isn't mandatory any more (LP: #787051) | 48 | - maintainer_name isn't mandatory any more (LP: #787051) |
66 | 49 | 49 | ||
67 | 50 | <<<<<<< TREE | ||
68 | 50 | -- Benjamin Drung <bdrung@debian.org> Wed, 25 May 2011 18:27:46 +0200 | 51 | -- Benjamin Drung <bdrung@debian.org> Wed, 25 May 2011 18:27:46 +0200 |
69 | 52 | ======= | ||
70 | 53 | [ Evan Broder ] | ||
71 | 54 | * ubuntutools.subprocess: | ||
72 | 55 | - New drop-in replacement wrapper module around subprocess that | ||
73 | 56 | backports the restore_signals kwarg and defaults close_fds=True | ||
74 | 57 | - Switch everything previously using subprocess to use | ||
75 | 58 | ubuntutools.subprocess instead (LP: #785854) | ||
76 | 59 | |||
77 | 60 | -- Evan Broder <evan@ebroder.net> Tue, 24 May 2011 20:21:47 +0200 | ||
78 | 61 | >>>>>>> MERGE-SOURCE | ||
79 | 51 | 62 | ||
80 | 52 | ubuntu-dev-tools (0.123) unstable; urgency=low | 63 | ubuntu-dev-tools (0.123) unstable; urgency=low |
81 | 53 | 64 | ||
82 | 54 | 65 | ||
83 | === modified file 'dgetlp' | |||
84 | --- dgetlp 2011-02-12 22:08:15 +0000 | |||
85 | +++ dgetlp 2011-06-19 21:23:31 +0000 | |||
86 | @@ -32,7 +32,6 @@ | |||
87 | 32 | import hashlib | 32 | import hashlib |
88 | 33 | import optparse | 33 | import optparse |
89 | 34 | import os | 34 | import os |
90 | 35 | import subprocess | ||
91 | 36 | import sys | 35 | import sys |
92 | 37 | import urllib2 | 36 | import urllib2 |
93 | 38 | 37 | ||
94 | @@ -43,6 +42,8 @@ | |||
95 | 43 | "use this utility.") | 42 | "use this utility.") |
96 | 44 | sys.exit(1) | 43 | sys.exit(1) |
97 | 45 | 44 | ||
98 | 45 | from ubuntutools import subprocess | ||
99 | 46 | |||
100 | 46 | USAGE = u"""Usage: %prog [-d|(-v|-q)] <Launchpad URL> | 47 | USAGE = u"""Usage: %prog [-d|(-v|-q)] <Launchpad URL> |
101 | 47 | 48 | ||
102 | 48 | This scripts simulates «dget»'s behaviour for files hosted at | 49 | This scripts simulates «dget»'s behaviour for files hosted at |
103 | 49 | 50 | ||
104 | === modified file 'get-branches' | |||
105 | --- get-branches 2010-12-27 19:49:00 +0000 | |||
106 | +++ get-branches 2011-06-19 21:23:31 +0000 | |||
107 | @@ -25,10 +25,10 @@ | |||
108 | 25 | # | 25 | # |
109 | 26 | 26 | ||
110 | 27 | import os | 27 | import os |
111 | 28 | import subprocess | ||
112 | 29 | import sys | 28 | import sys |
113 | 30 | from optparse import OptionParser | 29 | from optparse import OptionParser |
114 | 31 | from ubuntutools.lp.lpapicache import PersonTeam | 30 | from ubuntutools.lp.lpapicache import PersonTeam |
115 | 31 | from ubuntutools import subprocess | ||
116 | 32 | 32 | ||
117 | 33 | def main(): | 33 | def main(): |
118 | 34 | usage = "Usage: %prog [-d <directory>] -t <team> [-o <operation>]" | 34 | usage = "Usage: %prog [-d <directory>] -t <team> [-o <operation>]" |
119 | 35 | 35 | ||
120 | === modified file 'import-bug-from-debian' | |||
121 | --- import-bug-from-debian 2011-02-28 22:30:54 +0000 | |||
122 | +++ import-bug-from-debian 2011-06-19 21:23:31 +0000 | |||
123 | @@ -25,7 +25,6 @@ | |||
124 | 25 | from optparse import OptionParser, SUPPRESS_HELP | 25 | from optparse import OptionParser, SUPPRESS_HELP |
125 | 26 | import re | 26 | import re |
126 | 27 | import sys | 27 | import sys |
127 | 28 | import subprocess | ||
128 | 29 | 28 | ||
129 | 30 | try: | 29 | try: |
130 | 31 | import SOAPpy | 30 | import SOAPpy |
131 | @@ -37,6 +36,7 @@ | |||
132 | 37 | from launchpadlib.launchpad import Launchpad | 36 | from launchpadlib.launchpad import Launchpad |
133 | 38 | 37 | ||
134 | 39 | from ubuntutools.config import UDTConfig | 38 | from ubuntutools.config import UDTConfig |
135 | 39 | from ubuntutools import subprocess | ||
136 | 40 | 40 | ||
137 | 41 | def main(): | 41 | def main(): |
138 | 42 | bug_re = re.compile(r"bug=(\d+)") | 42 | bug_re = re.compile(r"bug=(\d+)") |
139 | 43 | 43 | ||
140 | === modified file 'lp-project-upload' | |||
141 | --- lp-project-upload 2011-06-09 11:36:06 +0000 | |||
142 | +++ lp-project-upload 2011-06-19 21:23:31 +0000 | |||
143 | @@ -22,13 +22,14 @@ | |||
144 | 22 | 22 | ||
145 | 23 | import datetime | 23 | import datetime |
146 | 24 | import os | 24 | import os |
147 | 25 | import subprocess | ||
148 | 26 | import sys | 25 | import sys |
149 | 27 | import tempfile | 26 | import tempfile |
150 | 28 | 27 | ||
151 | 29 | from launchpadlib.launchpad import Launchpad | 28 | from launchpadlib.launchpad import Launchpad |
152 | 30 | from launchpadlib.errors import HTTPError | 29 | from launchpadlib.errors import HTTPError |
153 | 31 | 30 | ||
154 | 31 | from ubuntutools import subprocess | ||
155 | 32 | |||
156 | 32 | def create_release(project, version): | 33 | def create_release(project, version): |
157 | 33 | '''Create new release and milestone for LP project.''' | 34 | '''Create new release and milestone for LP project.''' |
158 | 34 | 35 | ||
159 | 35 | 36 | ||
160 | === modified file 'pbuilder-dist' | |||
161 | --- pbuilder-dist 2011-05-23 21:41:00 +0000 | |||
162 | +++ pbuilder-dist 2011-06-19 21:23:31 +0000 | |||
163 | @@ -30,13 +30,13 @@ | |||
164 | 30 | # that the target distribution is always meant to be Ubuntu Hardy. | 30 | # that the target distribution is always meant to be Ubuntu Hardy. |
165 | 31 | 31 | ||
166 | 32 | import os | 32 | import os |
167 | 33 | import subprocess | ||
168 | 34 | import sys | 33 | import sys |
169 | 35 | 34 | ||
170 | 36 | from devscripts.logger import Logger | 35 | from devscripts.logger import Logger |
171 | 37 | 36 | ||
172 | 38 | from ubuntutools.distro_info import DebianDistroInfo | 37 | from ubuntutools.distro_info import DebianDistroInfo |
173 | 39 | import ubuntutools.misc | 38 | import ubuntutools.misc |
174 | 39 | from ubuntutools import subprocess | ||
175 | 40 | 40 | ||
176 | 41 | class PbuilderDist: | 41 | class PbuilderDist: |
177 | 42 | def __init__(self, builder): | 42 | def __init__(self, builder): |
178 | 43 | 43 | ||
179 | === modified file 'syncpackage' | |||
180 | --- syncpackage 2011-05-23 21:41:00 +0000 | |||
181 | +++ syncpackage 2011-06-19 21:23:31 +0000 | |||
182 | @@ -25,7 +25,6 @@ | |||
183 | 25 | import optparse | 25 | import optparse |
184 | 26 | import os | 26 | import os |
185 | 27 | import shutil | 27 | import shutil |
186 | 28 | import subprocess | ||
187 | 29 | import sys | 28 | import sys |
188 | 30 | 29 | ||
189 | 31 | from devscripts.logger import Logger | 30 | from devscripts.logger import Logger |
190 | @@ -38,6 +37,7 @@ | |||
191 | 38 | from ubuntutools.requestsync.lp import getDebianSrcPkg, getUbuntuSrcPkg | 37 | from ubuntutools.requestsync.lp import getDebianSrcPkg, getUbuntuSrcPkg |
192 | 39 | from ubuntutools.lp import udtexceptions | 38 | from ubuntutools.lp import udtexceptions |
193 | 40 | from ubuntutools.lp.lpapicache import Launchpad | 39 | from ubuntutools.lp.lpapicache import Launchpad |
194 | 40 | from ubuntutools import subprocess | ||
195 | 41 | 41 | ||
196 | 42 | 42 | ||
197 | 43 | class Version(debian.debian_support.Version): | 43 | class Version(debian.debian_support.Version): |
198 | 44 | 44 | ||
199 | === modified file 'ubuntu-iso' | |||
200 | --- ubuntu-iso 2010-12-27 13:01:03 +0000 | |||
201 | +++ ubuntu-iso 2011-06-19 21:23:31 +0000 | |||
202 | @@ -21,7 +21,8 @@ | |||
203 | 21 | # ################################################################## | 21 | # ################################################################## |
204 | 22 | 22 | ||
205 | 23 | import sys | 23 | import sys |
207 | 24 | import subprocess | 24 | |
208 | 25 | from ubuntutools import subprocess | ||
209 | 25 | 26 | ||
210 | 26 | def extract(iso, path): | 27 | def extract(iso, path): |
211 | 27 | command = ['isoinfo', '-R', '-i', iso, '-x', path] | 28 | command = ['isoinfo', '-R', '-i', iso, '-x', path] |
212 | 28 | 29 | ||
213 | === modified file 'ubuntutools/archive.py' | |||
214 | --- ubuntutools/archive.py 2011-05-30 13:55:15 +0000 | |||
215 | +++ ubuntutools/archive.py 2011-06-19 21:23:31 +0000 | |||
216 | @@ -31,7 +31,6 @@ | |||
217 | 31 | 31 | ||
218 | 32 | import hashlib | 32 | import hashlib |
219 | 33 | import os.path | 33 | import os.path |
220 | 34 | import subprocess | ||
221 | 35 | import urllib2 | 34 | import urllib2 |
222 | 36 | import urlparse | 35 | import urlparse |
223 | 37 | import re | 36 | import re |
224 | @@ -45,6 +44,7 @@ | |||
225 | 45 | from ubuntutools.config import UDTConfig | 44 | from ubuntutools.config import UDTConfig |
226 | 46 | from ubuntutools.lp.lpapicache import (Launchpad, Distribution, | 45 | from ubuntutools.lp.lpapicache import (Launchpad, Distribution, |
227 | 47 | SourcePackagePublishingHistory) | 46 | SourcePackagePublishingHistory) |
228 | 47 | from ubuntutools import subprocess | ||
229 | 48 | 48 | ||
230 | 49 | class DownloadError(Exception): | 49 | class DownloadError(Exception): |
231 | 50 | "Unable to pull a source package" | 50 | "Unable to pull a source package" |
232 | 51 | 51 | ||
233 | === modified file 'ubuntutools/builder.py' | |||
234 | --- ubuntutools/builder.py 2011-05-23 21:41:00 +0000 | |||
235 | +++ ubuntutools/builder.py 2011-06-19 21:23:31 +0000 | |||
236 | @@ -19,9 +19,9 @@ | |||
237 | 19 | # | 19 | # |
238 | 20 | 20 | ||
239 | 21 | import os | 21 | import os |
240 | 22 | import subprocess | ||
241 | 23 | 22 | ||
242 | 24 | from devscripts.logger import Logger | 23 | from devscripts.logger import Logger |
243 | 24 | from ubuntutools import subprocess | ||
244 | 25 | 25 | ||
245 | 26 | def _build_preparation(result_directory): | 26 | def _build_preparation(result_directory): |
246 | 27 | """prepares the builder for building a package""" | 27 | """prepares the builder for building a package""" |
247 | 28 | 28 | ||
248 | === modified file 'ubuntutools/misc.py' | |||
249 | --- ubuntutools/misc.py 2011-02-13 14:15:24 +0000 | |||
250 | +++ ubuntutools/misc.py 2011-06-19 21:23:31 +0000 | |||
251 | @@ -25,10 +25,10 @@ | |||
252 | 25 | import locale | 25 | import locale |
253 | 26 | import os | 26 | import os |
254 | 27 | import os.path | 27 | import os.path |
255 | 28 | from subprocess import Popen, PIPE | ||
256 | 29 | import sys | 28 | import sys |
257 | 30 | 29 | ||
258 | 31 | from ubuntutools.lp.udtexceptions import PocketDoesNotExistError | 30 | from ubuntutools.lp.udtexceptions import PocketDoesNotExistError |
259 | 31 | from ubuntutools.subprocess import Popen, PIPE | ||
260 | 32 | 32 | ||
261 | 33 | _system_distribution = None | 33 | _system_distribution = None |
262 | 34 | def system_distribution(): | 34 | def system_distribution(): |
263 | 35 | 35 | ||
264 | === modified file 'ubuntutools/requestsync/common.py' | |||
265 | --- ubuntutools/requestsync/common.py 2010-12-22 22:01:39 +0000 | |||
266 | +++ ubuntutools/requestsync/common.py 2011-06-19 21:23:31 +0000 | |||
267 | @@ -24,9 +24,10 @@ | |||
268 | 24 | import urllib2 | 24 | import urllib2 |
269 | 25 | import re | 25 | import re |
270 | 26 | import tempfile | 26 | import tempfile |
271 | 27 | import subprocess | ||
272 | 28 | from debian.changelog import Changelog | 27 | from debian.changelog import Changelog |
273 | 29 | 28 | ||
274 | 29 | from ubuntutools import subprocess | ||
275 | 30 | |||
276 | 30 | def raw_input_exit_on_ctrlc(*args, **kwargs): | 31 | def raw_input_exit_on_ctrlc(*args, **kwargs): |
277 | 31 | ''' | 32 | ''' |
278 | 32 | A wrapper around raw_input() to exit with a normalized message on Control-C | 33 | A wrapper around raw_input() to exit with a normalized message on Control-C |
279 | 33 | 34 | ||
280 | === modified file 'ubuntutools/requestsync/mail.py' | |||
281 | --- ubuntutools/requestsync/mail.py 2011-03-05 22:48:24 +0000 | |||
282 | +++ ubuntutools/requestsync/mail.py 2011-06-19 21:23:31 +0000 | |||
283 | @@ -22,13 +22,13 @@ | |||
284 | 22 | 22 | ||
285 | 23 | import os | 23 | import os |
286 | 24 | import sys | 24 | import sys |
287 | 25 | import subprocess | ||
288 | 26 | import smtplib | 25 | import smtplib |
289 | 27 | import socket | 26 | import socket |
290 | 28 | from debian.changelog import Version | 27 | from debian.changelog import Version |
291 | 29 | from ubuntutools.archive import rmadison, FakeSPPH | 28 | from ubuntutools.archive import rmadison, FakeSPPH |
292 | 30 | from ubuntutools.distro_info import DebianDistroInfo | 29 | from ubuntutools.distro_info import DebianDistroInfo |
293 | 31 | from ubuntutools.requestsync.common import raw_input_exit_on_ctrlc | 30 | from ubuntutools.requestsync.common import raw_input_exit_on_ctrlc |
294 | 31 | from ubuntutools import subprocess | ||
295 | 32 | from ubuntutools.lp.udtexceptions import PackageNotFoundException | 32 | from ubuntutools.lp.udtexceptions import PackageNotFoundException |
296 | 33 | 33 | ||
297 | 34 | __all__ = [ | 34 | __all__ = [ |
298 | 35 | 35 | ||
299 | === modified file 'ubuntutools/sponsor_patch/patch.py' | |||
300 | --- ubuntutools/sponsor_patch/patch.py 2010-12-27 14:20:49 +0000 | |||
301 | +++ ubuntutools/sponsor_patch/patch.py 2011-06-19 21:23:31 +0000 | |||
302 | @@ -16,7 +16,8 @@ | |||
303 | 16 | # OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. | 16 | # OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. |
304 | 17 | 17 | ||
305 | 18 | import os | 18 | import os |
307 | 19 | import subprocess | 19 | |
308 | 20 | from ubuntutools import subprocess | ||
309 | 20 | 21 | ||
310 | 21 | class Patch(object): | 22 | class Patch(object): |
311 | 22 | def __init__(self, patch_file): | 23 | def __init__(self, patch_file): |
312 | 23 | 24 | ||
313 | === modified file 'ubuntutools/sponsor_patch/sponsor_patch.py' | |||
314 | --- ubuntutools/sponsor_patch/sponsor_patch.py 2011-06-03 17:18:38 +0000 | |||
315 | +++ ubuntutools/sponsor_patch/sponsor_patch.py 2011-06-19 21:23:31 +0000 | |||
316 | @@ -19,7 +19,6 @@ | |||
317 | 19 | import pwd | 19 | import pwd |
318 | 20 | import re | 20 | import re |
319 | 21 | import shutil | 21 | import shutil |
320 | 22 | import subprocess | ||
321 | 23 | import sys | 22 | import sys |
322 | 24 | 23 | ||
323 | 25 | import debian.changelog | 24 | import debian.changelog |
324 | @@ -28,6 +27,7 @@ | |||
325 | 28 | 27 | ||
326 | 29 | from devscripts.logger import Logger | 28 | from devscripts.logger import Logger |
327 | 30 | 29 | ||
328 | 30 | from ubuntutools import subprocess | ||
329 | 31 | from ubuntutools.update_maintainer import update_maintainer | 31 | from ubuntutools.update_maintainer import update_maintainer |
330 | 32 | from ubuntutools.question import Question, YesNoQuestion, input_number | 32 | from ubuntutools.question import Question, YesNoQuestion, input_number |
331 | 33 | 33 | ||
332 | 34 | 34 | ||
333 | === added file 'ubuntutools/subprocess.py' | |||
334 | --- ubuntutools/subprocess.py 1970-01-01 00:00:00 +0000 | |||
335 | +++ ubuntutools/subprocess.py 2011-06-19 21:23:31 +0000 | |||
336 | @@ -0,0 +1,109 @@ | |||
337 | 1 | """Drop-in replacement for subprocess with better defaults | ||
338 | 2 | |||
339 | 3 | This is an API-compatible replacement for the built-in subprocess | ||
340 | 4 | module whose defaults better line up with our tastes. | ||
341 | 5 | |||
342 | 6 | In particular, it: | ||
343 | 7 | - Adds support for the restore_signals flag if subprocess itself | ||
344 | 8 | doesn't support it | ||
345 | 9 | - Defaults close_fds to True | ||
346 | 10 | """ | ||
347 | 11 | |||
348 | 12 | |||
349 | 13 | from __future__ import absolute_import | ||
350 | 14 | |||
351 | 15 | import inspect | ||
352 | 16 | import signal | ||
353 | 17 | import subprocess | ||
354 | 18 | |||
355 | 19 | from subprocess import PIPE, STDOUT, CalledProcessError | ||
356 | 20 | |||
357 | 21 | __all__ = ['Popen', 'call', 'check_call', 'check_output', 'CalledProcessError', 'PIPE', 'STDOUT'] | ||
358 | 22 | |||
359 | 23 | |||
360 | 24 | class Popen(subprocess.Popen): | ||
361 | 25 | def __init__(self, *args, **kwargs): | ||
362 | 26 | kwargs.setdefault('close_fds', True) | ||
363 | 27 | |||
364 | 28 | if 'restore_signals' not in inspect.getargspec(subprocess.Popen.__init__)[0]: | ||
365 | 29 | given_preexec_fn = kwargs.pop('preexec_fn', None) | ||
366 | 30 | restore_signals = kwargs.pop('restore_signals', True) | ||
367 | 31 | def preexec_fn(): | ||
368 | 32 | if restore_signals: | ||
369 | 33 | for sig in ('SIGPIPE', 'SIGXFZ', 'SIGXFSZ'): | ||
370 | 34 | if hasattr(signal, sig): | ||
371 | 35 | signal.signal(getattr(signal, sig), | ||
372 | 36 | signal.SIG_DFL) | ||
373 | 37 | |||
374 | 38 | if given_preexec_fn: | ||
375 | 39 | given_preexec_fn() | ||
376 | 40 | kwargs['preexec_fn'] = preexec_fn | ||
377 | 41 | |||
378 | 42 | subprocess.Popen.__init__(self, *args, **kwargs) | ||
379 | 43 | |||
380 | 44 | |||
381 | 45 | # call, check_call, and check_output are copied directly from the | ||
382 | 46 | # subprocess module shipped with Python 2.7.1-5ubuntu2 | ||
383 | 47 | |||
384 | 48 | |||
385 | 49 | def call(*popenargs, **kwargs): | ||
386 | 50 | """Run command with arguments. Wait for command to complete, then | ||
387 | 51 | return the returncode attribute. | ||
388 | 52 | |||
389 | 53 | The arguments are the same as for the Popen constructor. Example: | ||
390 | 54 | |||
391 | 55 | retcode = call(["ls", "-l"]) | ||
392 | 56 | """ | ||
393 | 57 | return Popen(*popenargs, **kwargs).wait() | ||
394 | 58 | |||
395 | 59 | |||
396 | 60 | def check_call(*popenargs, **kwargs): | ||
397 | 61 | """Run command with arguments. Wait for command to complete. If | ||
398 | 62 | the exit code was zero then return, otherwise raise | ||
399 | 63 | CalledProcessError. The CalledProcessError object will have the | ||
400 | 64 | return code in the returncode attribute. | ||
401 | 65 | |||
402 | 66 | The arguments are the same as for the Popen constructor. Example: | ||
403 | 67 | |||
404 | 68 | check_call(["ls", "-l"]) | ||
405 | 69 | """ | ||
406 | 70 | retcode = call(*popenargs, **kwargs) | ||
407 | 71 | if retcode: | ||
408 | 72 | cmd = kwargs.get("args") | ||
409 | 73 | if cmd is None: | ||
410 | 74 | cmd = popenargs[0] | ||
411 | 75 | raise CalledProcessError(retcode, cmd) | ||
412 | 76 | return 0 | ||
413 | 77 | |||
414 | 78 | |||
415 | 79 | def check_output(*popenargs, **kwargs): | ||
416 | 80 | r"""Run command with arguments and return its output as a byte string. | ||
417 | 81 | |||
418 | 82 | If the exit code was non-zero it raises a CalledProcessError. The | ||
419 | 83 | CalledProcessError object will have the return code in the returncode | ||
420 | 84 | attribute and output in the output attribute. | ||
421 | 85 | |||
422 | 86 | The arguments are the same as for the Popen constructor. Example: | ||
423 | 87 | |||
424 | 88 | >>> check_output(["ls", "-l", "/dev/null"]) | ||
425 | 89 | 'crw-rw-rw- 1 root root 1, 3 Oct 18 2007 /dev/null\n' | ||
426 | 90 | |||
427 | 91 | The stdout argument is not allowed as it is used internally. | ||
428 | 92 | To capture standard error in the result, use stderr=STDOUT. | ||
429 | 93 | |||
430 | 94 | >>> check_output(["/bin/sh", "-c", | ||
431 | 95 | ... "ls -l non_existent_file ; exit 0"], | ||
432 | 96 | ... stderr=STDOUT) | ||
433 | 97 | 'ls: non_existent_file: No such file or directory\n' | ||
434 | 98 | """ | ||
435 | 99 | if 'stdout' in kwargs: | ||
436 | 100 | raise ValueError('stdout argument not allowed, it will be overridden.') | ||
437 | 101 | process = Popen(stdout=PIPE, *popenargs, **kwargs) | ||
438 | 102 | output, unused_err = process.communicate() | ||
439 | 103 | retcode = process.poll() | ||
440 | 104 | if retcode: | ||
441 | 105 | cmd = kwargs.get("args") | ||
442 | 106 | if cmd is None: | ||
443 | 107 | cmd = popenargs[0] | ||
444 | 108 | raise CalledProcessError(retcode, cmd, output=output) | ||
445 | 109 | return output | ||
446 | 0 | 110 | ||
447 | === modified file 'ubuntutools/test/test_help.py' | |||
448 | --- ubuntutools/test/test_help.py 2011-05-21 15:53:16 +0000 | |||
449 | +++ ubuntutools/test/test_help.py 2011-06-19 21:23:31 +0000 | |||
450 | @@ -18,10 +18,10 @@ | |||
451 | 18 | import os | 18 | import os |
452 | 19 | import select | 19 | import select |
453 | 20 | import signal | 20 | import signal |
454 | 21 | import subprocess | ||
455 | 22 | import time | 21 | import time |
456 | 23 | 22 | ||
457 | 24 | import setup | 23 | import setup |
458 | 24 | from ubuntutools import subprocess | ||
459 | 25 | from ubuntutools.test import unittest | 25 | from ubuntutools.test import unittest |
460 | 26 | 26 | ||
461 | 27 | BLACKLIST = { | 27 | BLACKLIST = { |
462 | 28 | 28 | ||
463 | === modified file 'ubuntutools/test/test_pylint.py' | |||
464 | --- ubuntutools/test/test_pylint.py 2011-05-30 13:55:15 +0000 | |||
465 | +++ ubuntutools/test/test_pylint.py 2011-06-19 21:23:31 +0000 | |||
466 | @@ -15,10 +15,10 @@ | |||
467 | 15 | # PERFORMANCE OF THIS SOFTWARE. | 15 | # PERFORMANCE OF THIS SOFTWARE. |
468 | 16 | 16 | ||
469 | 17 | import re | 17 | import re |
470 | 18 | import subprocess | ||
471 | 19 | 18 | ||
472 | 20 | import setup | 19 | import setup |
473 | 21 | from ubuntutools.test import unittest | 20 | from ubuntutools.test import unittest |
474 | 21 | from ubuntutools import subprocess | ||
475 | 22 | 22 | ||
476 | 23 | WHITELIST = [re.compile(': %s$' % x) for x in ( | 23 | WHITELIST = [re.compile(': %s$' % x) for x in ( |
477 | 24 | # Wildcard import: | 24 | # Wildcard import: |
478 | @@ -28,6 +28,8 @@ | |||
479 | 28 | # mox: | 28 | # mox: |
480 | 29 | r"Instance of '.+' has no '(WithSideEffects|MultipleTimes|AndReturn)' " | 29 | r"Instance of '.+' has no '(WithSideEffects|MultipleTimes|AndReturn)' " |
481 | 30 | r"member", | 30 | r"member", |
482 | 31 | # pylint doesn't like *args/**kwargs | ||
483 | 32 | r"Instance of 'Popen' has no '.*' member", | ||
484 | 31 | )] | 33 | )] |
485 | 32 | 34 | ||
486 | 33 | class PylintTestCase(unittest.TestCase): | 35 | class PylintTestCase(unittest.TestCase): |
Hi Evan,
I totally see why you've added this, and changed the code to use it. Thanks for making ubuntu-dev-tools better! Of course, once we migrate u-d-t to Python 3.2 this change won't be necessary <wink>.
I'm a little concerned that what you've written isn't entirely API compatible with Python's version. Python's subprocess.Popen is a class while ubuntutools. subprocess. Popen is a function. I have no idea whether people actually try to subclass subprocess.Popen, and within u-d-t it's easy enough to guarantee this, but is it possible there are would be external clients that could be broken by this? It makes me uncomfortable that your docs claim API compatibility when it's really not.
Two thoughts: could this be implemented as a subprocess.Popen subclass?
Or, would it make sense to just use a different API rather than promoting something that looks like stdlib subprocess.Popen but isn't quite? The way you have it does mean that existing code in u-d-t doesn't have to change, except for the import statement. I'm not sure whether that's good or not ;).
You might also call inspect. getargspec( ) at module scope and then not redefine, but simply expose the stdlib subprocess functions directly in ubuntutools. subprocess. Popen.
Anyway, just some thoughts. At the least, please do indicate in the docs about this difference. I'm happy to chat about alternatives in IRC if you want.