Merge pull request #383 from LLNL/features/better-executable-args
Refactor args for Executable.__call__
This commit is contained in:
commit
1da672df7d
11 changed files with 116 additions and 60 deletions
|
@ -42,7 +42,7 @@ def get_origin_url():
|
|||
git = which('git', required=True)
|
||||
origin_url = git(
|
||||
'--git-dir=%s' % git_dir, 'config', '--get', 'remote.origin.url',
|
||||
return_output=True)
|
||||
output=str)
|
||||
return origin_url.strip()
|
||||
|
||||
|
||||
|
|
|
@ -132,7 +132,7 @@ def __call__(self, stage):
|
|||
# Peek inside the tarball.
|
||||
tar = which('tar')
|
||||
output = tar(
|
||||
"--exclude=*/*/*", "-tf", stage.archive_file, return_output=True)
|
||||
"--exclude=*/*/*", "-tf", stage.archive_file, output=str)
|
||||
lines = output.split("\n")
|
||||
|
||||
# Set the configure line to the one that matched.
|
||||
|
|
|
@ -79,7 +79,7 @@ def list_packages(rev):
|
|||
git = get_git()
|
||||
relpath = spack.packages_path[len(spack.prefix + os.path.sep):] + os.path.sep
|
||||
output = git('ls-tree', '--full-tree', '--name-only', rev, relpath,
|
||||
return_output=True)
|
||||
output=str)
|
||||
return sorted(line[len(relpath):] for line in output.split('\n') if line)
|
||||
|
||||
|
||||
|
|
|
@ -24,7 +24,6 @@
|
|||
##############################################################################
|
||||
import os
|
||||
import re
|
||||
import subprocess
|
||||
import itertools
|
||||
from datetime import datetime
|
||||
|
||||
|
@ -52,7 +51,7 @@ def _verify_executables(*paths):
|
|||
def get_compiler_version(compiler_path, version_arg, regex='(.*)'):
|
||||
if not compiler_path in _version_cache:
|
||||
compiler = Executable(compiler_path)
|
||||
output = compiler(version_arg, return_output=True, error=subprocess.STDOUT)
|
||||
output = compiler(version_arg, output=str, error=str)
|
||||
|
||||
match = re.search(regex, output)
|
||||
_version_cache[compiler_path] = match.group(1) if match else 'unknown'
|
||||
|
|
|
@ -154,7 +154,7 @@ def fetch(self):
|
|||
|
||||
# Run curl but grab the mime type from the http headers
|
||||
headers = spack.curl(
|
||||
*curl_args, return_output=True, fail_on_error=False)
|
||||
*curl_args, output=str, fail_on_error=False)
|
||||
|
||||
if spack.curl.returncode != 0:
|
||||
# clean up archive on failure.
|
||||
|
@ -375,7 +375,7 @@ def __init__(self, **kwargs):
|
|||
|
||||
@property
|
||||
def git_version(self):
|
||||
vstring = self.git('--version', return_output=True).lstrip('git version ')
|
||||
vstring = self.git('--version', output=str).lstrip('git version ')
|
||||
return Version(vstring)
|
||||
|
||||
|
||||
|
@ -518,7 +518,7 @@ def fetch(self):
|
|||
|
||||
def _remove_untracked_files(self):
|
||||
"""Removes untracked files in an svn repository."""
|
||||
status = self.svn('status', '--no-ignore', return_output=True)
|
||||
status = self.svn('status', '--no-ignore', output=str)
|
||||
self.svn('status', '--no-ignore')
|
||||
for line in status.split('\n'):
|
||||
if not re.match('^[I?]', line):
|
||||
|
|
|
@ -65,17 +65,17 @@ def setUp(self):
|
|||
|
||||
def check_cc(self, command, args, expected):
|
||||
os.environ['SPACK_TEST_COMMAND'] = command
|
||||
self.assertEqual(self.cc(*args, return_output=True).strip(), expected)
|
||||
self.assertEqual(self.cc(*args, output=str).strip(), expected)
|
||||
|
||||
|
||||
def check_ld(self, command, args, expected):
|
||||
os.environ['SPACK_TEST_COMMAND'] = command
|
||||
self.assertEqual(self.ld(*args, return_output=True).strip(), expected)
|
||||
self.assertEqual(self.ld(*args, output=str).strip(), expected)
|
||||
|
||||
|
||||
def check_cpp(self, command, args, expected):
|
||||
os.environ['SPACK_TEST_COMMAND'] = command
|
||||
self.assertEqual(self.cpp(*args, return_output=True).strip(), expected)
|
||||
self.assertEqual(self.cpp(*args, output=str).strip(), expected)
|
||||
|
||||
|
||||
def test_vcheck_mode(self):
|
||||
|
|
|
@ -56,47 +56,47 @@ def tearDown(self):
|
|||
|
||||
def test_make_normal(self):
|
||||
make = MakeExecutable('make', 8)
|
||||
self.assertEqual(make(return_output=True).strip(), '-j8')
|
||||
self.assertEqual(make('install', return_output=True).strip(), '-j8 install')
|
||||
self.assertEqual(make(output=str).strip(), '-j8')
|
||||
self.assertEqual(make('install', output=str).strip(), '-j8 install')
|
||||
|
||||
|
||||
def test_make_explicit(self):
|
||||
make = MakeExecutable('make', 8)
|
||||
self.assertEqual(make(parallel=True, return_output=True).strip(), '-j8')
|
||||
self.assertEqual(make('install', parallel=True, return_output=True).strip(), '-j8 install')
|
||||
self.assertEqual(make(parallel=True, output=str).strip(), '-j8')
|
||||
self.assertEqual(make('install', parallel=True, output=str).strip(), '-j8 install')
|
||||
|
||||
|
||||
def test_make_one_job(self):
|
||||
make = MakeExecutable('make', 1)
|
||||
self.assertEqual(make(return_output=True).strip(), '')
|
||||
self.assertEqual(make('install', return_output=True).strip(), 'install')
|
||||
self.assertEqual(make(output=str).strip(), '')
|
||||
self.assertEqual(make('install', output=str).strip(), 'install')
|
||||
|
||||
|
||||
def test_make_parallel_false(self):
|
||||
make = MakeExecutable('make', 8)
|
||||
self.assertEqual(make(parallel=False, return_output=True).strip(), '')
|
||||
self.assertEqual(make('install', parallel=False, return_output=True).strip(), 'install')
|
||||
self.assertEqual(make(parallel=False, output=str).strip(), '')
|
||||
self.assertEqual(make('install', parallel=False, output=str).strip(), 'install')
|
||||
|
||||
|
||||
def test_make_parallel_disabled(self):
|
||||
make = MakeExecutable('make', 8)
|
||||
|
||||
os.environ['SPACK_NO_PARALLEL_MAKE'] = 'true'
|
||||
self.assertEqual(make(return_output=True).strip(), '')
|
||||
self.assertEqual(make('install', return_output=True).strip(), 'install')
|
||||
self.assertEqual(make(output=str).strip(), '')
|
||||
self.assertEqual(make('install', output=str).strip(), 'install')
|
||||
|
||||
os.environ['SPACK_NO_PARALLEL_MAKE'] = '1'
|
||||
self.assertEqual(make(return_output=True).strip(), '')
|
||||
self.assertEqual(make('install', return_output=True).strip(), 'install')
|
||||
self.assertEqual(make(output=str).strip(), '')
|
||||
self.assertEqual(make('install', output=str).strip(), 'install')
|
||||
|
||||
# These don't disable (false and random string)
|
||||
os.environ['SPACK_NO_PARALLEL_MAKE'] = 'false'
|
||||
self.assertEqual(make(return_output=True).strip(), '-j8')
|
||||
self.assertEqual(make('install', return_output=True).strip(), '-j8 install')
|
||||
self.assertEqual(make(output=str).strip(), '-j8')
|
||||
self.assertEqual(make('install', output=str).strip(), '-j8 install')
|
||||
|
||||
os.environ['SPACK_NO_PARALLEL_MAKE'] = 'foobar'
|
||||
self.assertEqual(make(return_output=True).strip(), '-j8')
|
||||
self.assertEqual(make('install', return_output=True).strip(), '-j8 install')
|
||||
self.assertEqual(make(output=str).strip(), '-j8')
|
||||
self.assertEqual(make('install', output=str).strip(), '-j8 install')
|
||||
|
||||
del os.environ['SPACK_NO_PARALLEL_MAKE']
|
||||
|
||||
|
@ -106,20 +106,20 @@ def test_make_parallel_precedence(self):
|
|||
|
||||
# These should work
|
||||
os.environ['SPACK_NO_PARALLEL_MAKE'] = 'true'
|
||||
self.assertEqual(make(parallel=True, return_output=True).strip(), '')
|
||||
self.assertEqual(make('install', parallel=True, return_output=True).strip(), 'install')
|
||||
self.assertEqual(make(parallel=True, output=str).strip(), '')
|
||||
self.assertEqual(make('install', parallel=True, output=str).strip(), 'install')
|
||||
|
||||
os.environ['SPACK_NO_PARALLEL_MAKE'] = '1'
|
||||
self.assertEqual(make(parallel=True, return_output=True).strip(), '')
|
||||
self.assertEqual(make('install', parallel=True, return_output=True).strip(), 'install')
|
||||
self.assertEqual(make(parallel=True, output=str).strip(), '')
|
||||
self.assertEqual(make('install', parallel=True, output=str).strip(), 'install')
|
||||
|
||||
# These don't disable (false and random string)
|
||||
os.environ['SPACK_NO_PARALLEL_MAKE'] = 'false'
|
||||
self.assertEqual(make(parallel=True, return_output=True).strip(), '-j8')
|
||||
self.assertEqual(make('install', parallel=True, return_output=True).strip(), '-j8 install')
|
||||
self.assertEqual(make(parallel=True, output=str).strip(), '-j8')
|
||||
self.assertEqual(make('install', parallel=True, output=str).strip(), '-j8 install')
|
||||
|
||||
os.environ['SPACK_NO_PARALLEL_MAKE'] = 'foobar'
|
||||
self.assertEqual(make(parallel=True, return_output=True).strip(), '-j8')
|
||||
self.assertEqual(make('install', parallel=True, return_output=True).strip(), '-j8 install')
|
||||
self.assertEqual(make(parallel=True, output=str).strip(), '-j8')
|
||||
self.assertEqual(make('install', parallel=True, output=str).strip(), '-j8 install')
|
||||
|
||||
del os.environ['SPACK_NO_PARALLEL_MAKE']
|
||||
|
|
|
@ -141,7 +141,7 @@ def __init__(self):
|
|||
self.url = self.path
|
||||
|
||||
def rev_hash(self, rev):
|
||||
return git('rev-parse', rev, return_output=True).strip()
|
||||
return git('rev-parse', rev, output=str).strip()
|
||||
|
||||
|
||||
class MockSvnRepo(MockVCSRepo):
|
||||
|
@ -193,4 +193,4 @@ def __init__(self):
|
|||
|
||||
def get_rev(self):
|
||||
"""Get current mercurial revision."""
|
||||
return hg('id', '-i', return_output=True).strip()
|
||||
return hg('id', '-i', output=str).strip()
|
||||
|
|
|
@ -65,7 +65,7 @@ def tearDown(self):
|
|||
def assert_rev(self, rev):
|
||||
"""Check that the current revision is equal to the supplied rev."""
|
||||
def get_rev():
|
||||
output = svn('info', return_output=True)
|
||||
output = svn('info', output=str)
|
||||
self.assertTrue("Revision" in output)
|
||||
for line in output.split('\n'):
|
||||
match = re.match(r'Revision: (\d+)', line)
|
||||
|
|
|
@ -55,24 +55,80 @@ def command(self):
|
|||
|
||||
|
||||
def __call__(self, *args, **kwargs):
|
||||
"""Run the executable with subprocess.check_output, return output."""
|
||||
return_output = kwargs.get("return_output", False)
|
||||
fail_on_error = kwargs.get("fail_on_error", True)
|
||||
ignore_errors = kwargs.get("ignore_errors", ())
|
||||
"""Run this executable in a subprocess.
|
||||
|
||||
Arguments
|
||||
args
|
||||
command line arguments to the executable to run.
|
||||
|
||||
Optional arguments
|
||||
|
||||
fail_on_error
|
||||
|
||||
Raise an exception if the subprocess returns an
|
||||
error. Default is True. When not set, the return code is
|
||||
avaiale as `exe.returncode`.
|
||||
|
||||
ignore_errors
|
||||
|
||||
An optional list/tuple of error codes that can be
|
||||
*ignored*. i.e., if these codes are returned, this will
|
||||
not raise an exception when `fail_on_error` is `True`.
|
||||
|
||||
output, error
|
||||
|
||||
These arguments allow you to specify new stdout and stderr
|
||||
values. They default to `None`, which means the
|
||||
subprocess will inherit the parent's file descriptors.
|
||||
|
||||
You can set these to:
|
||||
- python streams, e.g. open Python file objects, or os.devnull;
|
||||
- filenames, which will be automatically opened for writing; or
|
||||
- `str`, as in the Python string type. If you set these to `str`,
|
||||
output and error will be written to pipes and returned as
|
||||
a string. If both `output` and `error` are set to `str`,
|
||||
then one string is returned containing output concatenated
|
||||
with error.
|
||||
|
||||
input
|
||||
|
||||
Same as output, error, but `str` is not an allowed value.
|
||||
|
||||
Deprecated arguments
|
||||
|
||||
return_output[=False]
|
||||
|
||||
Setting this to True is the same as setting output=str.
|
||||
This argument may be removed in future Spack versions.
|
||||
|
||||
"""
|
||||
fail_on_error = kwargs.pop("fail_on_error", True)
|
||||
ignore_errors = kwargs.pop("ignore_errors", ())
|
||||
|
||||
# TODO: This is deprecated. Remove in a future version.
|
||||
return_output = kwargs.pop("return_output", False)
|
||||
|
||||
# Default values of None says to keep parent's file descriptors.
|
||||
output = kwargs.get("output", None)
|
||||
error = kwargs.get("error", None)
|
||||
input = kwargs.get("input", None)
|
||||
if return_output:
|
||||
output = str
|
||||
else:
|
||||
output = kwargs.pop("output", None)
|
||||
|
||||
error = kwargs.pop("error", None)
|
||||
input = kwargs.pop("input", None)
|
||||
if input is str:
|
||||
raise ValueError("Cannot use `str` as input stream.")
|
||||
|
||||
def streamify(arg, mode):
|
||||
if isinstance(arg, basestring):
|
||||
return open(arg, mode), True
|
||||
elif arg is str:
|
||||
return subprocess.PIPE, False
|
||||
else:
|
||||
return arg, False
|
||||
output, ostream = streamify(output, 'w')
|
||||
error, estream = streamify(error, 'w')
|
||||
input, istream = streamify(input, 'r')
|
||||
ostream, close_ostream = streamify(output, 'w')
|
||||
estream, close_estream = streamify(error, 'w')
|
||||
istream, close_istream = streamify(input, 'r')
|
||||
|
||||
# if they just want to ignore one error code, make it a tuple.
|
||||
if isinstance(ignore_errors, int):
|
||||
|
@ -92,19 +148,20 @@ def streamify(arg, mode):
|
|||
tty.debug(cmd_line)
|
||||
|
||||
try:
|
||||
if return_output:
|
||||
output = subprocess.PIPE
|
||||
|
||||
proc = subprocess.Popen(
|
||||
cmd, stdin=input, stderr=error, stdout=output)
|
||||
cmd, stdin=istream, stderr=estream, stdout=ostream)
|
||||
out, err = proc.communicate()
|
||||
|
||||
rc = self.returncode = proc.returncode
|
||||
if fail_on_error and rc != 0 and (rc not in ignore_errors):
|
||||
raise ProcessError("Command exited with status %d:"
|
||||
% proc.returncode, cmd_line)
|
||||
if return_output:
|
||||
return out
|
||||
|
||||
if output is str or error is str:
|
||||
result = ''
|
||||
if output is str: result += out
|
||||
if error is str: result += err
|
||||
return result
|
||||
|
||||
except OSError, e:
|
||||
raise ProcessError(
|
||||
|
@ -119,9 +176,9 @@ def streamify(arg, mode):
|
|||
% (proc.returncode, cmd_line))
|
||||
|
||||
finally:
|
||||
if ostream: output.close()
|
||||
if estream: error.close()
|
||||
if istream: input.close()
|
||||
if close_ostream: output.close()
|
||||
if close_estream: error.close()
|
||||
if close_istream: input.close()
|
||||
|
||||
|
||||
def __eq__(self, other):
|
||||
|
|
|
@ -121,7 +121,7 @@ def write_rpath_specs(self):
|
|||
return
|
||||
|
||||
gcc = Executable(join_path(self.prefix.bin, 'gcc'))
|
||||
lines = gcc('-dumpspecs', return_output=True).strip().split("\n")
|
||||
lines = gcc('-dumpspecs', output=str).strip().split("\n")
|
||||
specs_file = join_path(self.spec_dir, 'specs')
|
||||
with closing(open(specs_file, 'w')) as out:
|
||||
for line in lines:
|
||||
|
|
Loading…
Reference in a new issue