Merge pull request #2858 from rintaro/build-script-capture

[build-script] Consistent use of shell.capture in swift_build_support

Author: gribozavr

utils/swift_build_support/swift_build_support/cmake.py

@@ -14,10 +14,12 @@
 #
 # ----------------------------------------------------------------------------
 
-import subprocess
+from __future__ import absolute_import
 
 from numbers import Number
 
+from . import shell
+
 
 class CMakeOptions(object):
     """List like object used to define cmake options
@@ -125,8 +127,8 @@ def build_args(self):
         toolchain = self.toolchain
         jobs = args.build_jobs
         if args.distcc:
-            jobs = str(subprocess.check_output(
-                [toolchain.distcc, '-j']).decode()).rstrip()
+            jobs = shell.capture([toolchain.distcc, '-j'],
+                                 dry_run=False, echo=False).rstrip()
 
         build_args = list(args.build_args)
 

utils/swift_build_support/swift_build_support/debug.py

@@ -14,29 +14,29 @@
 #
 # ----------------------------------------------------------------------------
 
+from __future__ import absolute_import
 from __future__ import print_function
 
-import subprocess
 import sys
 
-
-def _output(args):
-    try:
-        out = subprocess.check_output(args, stderr=subprocess.PIPE)
-        return out.rstrip().decode()
-    except subprocess.CalledProcessError:
-        return None
+from . import shell
 
 
 def print_xcodebuild_versions(file=sys.stdout):
     """
     Print the host machine's `xcodebuild` version, as well as version
     information for all available SDKs.
     """
-    print(u'{}\n'.format(_output(['xcodebuild', '-version'])), file=file)
-    print(u'--- SDK versions ---', file=file)
-    print(u'{}\n'.format(_output(['xcodebuild', '-version', '-sdk'])),
-          file=file)
+    version = shell.capture(
+        ['xcodebuild', '-version'], dry_run=False, echo=False).rstrip()
+    sdks = shell.capture(
+        ['xcodebuild', '-version', '-sdk'], dry_run=False, echo=False).rstrip()
+    fmt = """\
+{version}
+
+--- SDK versions ---
+{sdks}
+
+"""
+    print(fmt.format(version=version, sdks=sdks), file=file)
     file.flush()
-    # You can't test beyond this because each developer's machines may have
-    # a different set of SDKs installed.

utils/swift_build_support/swift_build_support/shell.py

@@ -24,6 +24,9 @@
 
 from . import diagnostics
 
+
+DEVNULL = getattr(subprocess, 'DEVNULL', subprocess.PIPE)
+
 dry_run = False
 
 
@@ -88,7 +91,8 @@ def call(command, stderr=None, env=None, dry_run=None, echo=True):
             "': " + e.strerror)
 
 
-def capture(command, stderr=None, env=None, dry_run=None, echo=True):
+def capture(command, stderr=None, env=None, dry_run=None, echo=True,
+            optional=False):
     """
     capture(command, ...) -> str
 
@@ -106,8 +110,12 @@ def capture(command, stderr=None, env=None, dry_run=None, echo=True):
         _env = dict(os.environ)
         _env.update(env)
     try:
-        return subprocess.check_output(command, env=_env, stderr=stderr)
+        out = subprocess.check_output(command, env=_env, stderr=stderr)
+        # Coerce to `str` hack. not py3 `byte`, not py2 `unicode`.
+        return str(out.decode())
     except subprocess.CalledProcessError as e:
+        if optional:
+            return None
         diagnostics.fatal(
             "command terminated with a non-zero exit status " +
             str(e.returncode) + ", aborting")

utils/swift_build_support/swift_build_support/tar.py

@@ -11,7 +11,6 @@
 from __future__ import absolute_import
 
 import platform
-import subprocess
 
 from . import shell
 
@@ -29,7 +28,7 @@ def tar(source, destination):
     if platform.system() != 'Darwin':
         args += ['--owner=0', '--group=0']
 
-    # Capture stderr output such as 'tar: Failed to open ...'. We'll detect
-    # these cases using the exit code, which should cause 'check_call' to
+    # Discard stderr output such as 'tar: Failed to open ...'. We'll detect
+    # these cases using the exit code, which should cause 'shell.call' to
     # raise.
-    shell.call(args + [source], stderr=subprocess.PIPE)
+    shell.call(args + [source], stderr=shell.DEVNULL)

utils/swift_build_support/swift_build_support/toolchain.py

@@ -17,9 +17,9 @@
 from __future__ import absolute_import
 
 import platform
-import subprocess
 
 from . import cache_util
+from . import shell
 from . import xcrun
 from .which import which
 
@@ -151,13 +151,13 @@ def _release_date():
         """Return the release date for FreeBSD operating system on this host.
         If the release date cannot be ascertained, return None.
         """
-        try:
-            # For details on `sysctl`, see:
-            # http://www.freebsd.org/cgi/man.cgi?sysctl(8)
-            out = subprocess.check_output(['sysctl', '-n', 'kern.osreldate'])
-            return int(out)
-        except subprocess.CalledProcessError:
+        # For details on `sysctl`, see:
+        # http://www.freebsd.org/cgi/man.cgi?sysctl(8)
+        out = shell.capture(['sysctl', '-n', 'kern.osreldate'],
+                            dry_run=False, echo=False, optional=True)
+        if out is None:
             return None
+        return int(out)
 
 
 class Cygwin(Linux):

utils/swift_build_support/swift_build_support/which.py

@@ -18,9 +18,8 @@
 
 from __future__ import absolute_import
 
-import subprocess
-
 from . import cache_util
+from . import shell
 
 
 @cache_util.cached
@@ -35,8 +34,8 @@ def which(cmd):
     We provide our own implementation because shutil.which() has not
     been backported to Python 2.7, which we support.
     """
-    try:
-        ret = subprocess.check_output(['which', cmd])
-        return str(ret.rstrip().decode())
-    except subprocess.CalledProcessError:
+    out = shell.capture(['which', cmd],
+                        dry_run=False, echo=False, optional=True)
+    if out is None:
         return None
+    return out.rstrip()

utils/swift_build_support/swift_build_support/xcrun.py

@@ -16,9 +16,8 @@
 
 from __future__ import absolute_import
 
-import subprocess
-
 from . import cache_util
+from . import shell
 
 
 @cache_util.cached
@@ -35,14 +34,14 @@ def find(tool, sdk=None, toolchain=None):
     if toolchain is not None:
         command += ['--toolchain', toolchain]
 
-    try:
-        # `xcrun --find` prints to stderr when it fails to find the
-        # given tool. We swallow that output with a pipe.
-        out = subprocess.check_output(command,
-                                      stderr=subprocess.PIPE)
-        return str(out.rstrip().decode())
-    except subprocess.CalledProcessError:
+    # `xcrun --find` prints to stderr when it fails to find the
+    # given tool. We swallow that output with a pipe.
+    out = shell.capture(
+        command,
+        stderr=shell.DEVNULL, dry_run=False, echo=False, optional=True)
+    if out is None:
         return None
+    return out.rstrip()
 
 
 @cache_util.cached
@@ -53,8 +52,7 @@ def sdk_path(sdk):
     If `xcrun --show-sdk-path` cannot find the SDK, return None.
     """
     command = ['xcrun', '--sdk', sdk, '--show-sdk-path']
-    try:
-        out = subprocess.check_output(command)
-        return str(out.rstrip().decode())
-    except subprocess.CalledProcessError:
+    out = shell.capture(command, dry_run=False, echo=False, optional=True)
+    if out is None:
         return None
+    return out.rstrip()

utils/swift_build_support/tests/test_arguments.py

@@ -22,8 +22,8 @@
     from io import StringIO
 
 from swift_build_support.arguments import (
-    type as argtype,
     action as argaction,
+    type as argtype,
 )
 
 

utils/swift_build_support/tests/test_debug.py

@@ -10,12 +10,12 @@
 
 import platform
 import unittest
-
-# StringIO import path differs across Python 2 and 3.
 try:
-    from io import StringIO
+    # py2
+    from StringIO import StringIO
 except ImportError:
-    from cStringIO import StringIO
+    # py3
+    from io import StringIO
 
 from swift_build_support import debug
 
@@ -33,6 +33,8 @@ def test_outputs_xcode_version(self):
         self.assertTrue(actual[0].startswith('Xcode '))
         self.assertTrue(actual[1].startswith('Build version '))
         self.assertEqual(actual[3], '--- SDK versions ---')
+        # You can't test beyond this because each developer's machines may have
+        # a different set of SDKs installed.
 
 
 if __name__ == '__main__':

utils/swift_build_support/tests/test_shell.py

@@ -69,6 +69,11 @@ def test_capture(self):
         with self.assertRaises(SystemExit):
             shell.capture(["false"])
 
+        self.assertIsNone(shell.capture(["false"], optional=True))
+
+        with self.assertRaises(SystemExit):
+            shell.capture(["**not-a-command**"], optional=True)
+
     def test_rmtree(self):
         shell.dry_run = False
         path = os.path.join(self.tmpdir, 'foo', 'bar')