Merge pull request #2836 from ddunbar/eliminate-SwiftBuildSupport-subprocess-functions

[build-script] Eliminate swift build support subprocess functions

Author: ddunbar

utils/SwiftBuildSupport.py

@@ -18,8 +18,6 @@
     import configparser as ConfigParser
 
 import os
-import platform
-import subprocess
 import sys
 
 sys.path.append(os.path.join(os.path.dirname(__file__), 'swift_build_support'))
@@ -70,44 +68,6 @@ def _get_default_source_root():
     "SWIFT_BUILD_ROOT", os.path.join(SWIFT_SOURCE_ROOT, "build"))
 
 
-def check_call(args, print_command=False, verbose=False, disable_sleep=False):
-    if disable_sleep:
-        if platform.system() == 'Darwin':
-            # Don't mutate the caller's copy of the arguments.
-            args = list(args)
-            args.insert(0, "caffeinate")
-
-    if print_command:
-        print(os.getcwd() + "$ " + shell.quote_command(args))
-        sys.stdout.flush()
-    try:
-        return subprocess.check_call(args)
-    except subprocess.CalledProcessError as e:
-        diagnostics.fatal(
-            "command terminated with a non-zero exit status " +
-            str(e.returncode) + ", aborting")
-    except OSError as e:
-        diagnostics.fatal(
-            "could not execute '" + shell.quote_command(args) +
-            "': " + e.strerror)
-
-
-def check_output(args, print_command=False, verbose=False):
-    if print_command:
-        print(os.getcwd() + "$ " + shell.quote_command(args))
-        sys.stdout.flush()
-    try:
-        return subprocess.check_output(args)
-    except subprocess.CalledProcessError as e:
-        diagnostics.fatal(
-            "command terminated with a non-zero exit status " +
-            str(e.returncode) + ", aborting")
-    except OSError as e:
-        diagnostics.fatal(
-            "could not execute '" + shell.quote_command(args) +
-            "': " + e.strerror)
-
-
 def _load_preset_files_impl(preset_file_names, substitutions={}):
     config = ConfigParser.SafeConfigParser(substitutions, allow_no_value=True)
     if config.read(preset_file_names) == []:

utils/build-script

@@ -28,7 +28,6 @@ from SwiftBuildSupport import (
     HOME,
     SWIFT_BUILD_ROOT,
     SWIFT_SOURCE_ROOT,
-    check_call,
     get_all_preset_names,
     get_preset_options,
 )  # noqa (E402 module level import not at top of file)
@@ -51,6 +50,21 @@ from swift_build_support.workspace import Workspace  # noqa(E402)
 from swift_build_support.workspace import compute_build_subdir  # noqa(E402)
 
 
+def call_without_sleeping(command, dry_run=False):
+    """
+    Execute a command during which system sleep is disabled.
+
+    By default, this ignores the state of the `shell.dry_run` flag.
+    """
+
+    # Disable system sleep, if possible.
+    if platform.system() == 'Darwin':
+        # Don't mutate the caller's copy of the arguments.
+        command = ["caffeinate"] + list(command)
+
+    shell.call(command, dry_run=dry_run, print_command=False)
+
+
 # Main entry point for the preset mode.
 def main_preset():
     parser = argparse.ArgumentParser(
@@ -124,7 +138,7 @@ def main_preset():
         "using preset '" + args.preset + "', which expands to \n\n" +
         shell.quote_command(build_script_args) + "\n")
 
-    check_call(build_script_args, disable_sleep=True)
+    call_without_sleeping(build_script_args)
 
     return 0
 
@@ -1435,8 +1449,7 @@ details of the setups of other systems or automated environments.""")
     if args.dry_run:
         build_script_impl_args += ["--dry-run"]
 
-    check_call([build_script_impl] + build_script_impl_args,
-               disable_sleep=True)
+    call_without_sleeping([build_script_impl] + build_script_impl_args)
 
     if args.symbols_package:
         print('--- Creating symbols package ---')

utils/profdata_merge/process.py

@@ -19,17 +19,16 @@
 from multiprocessing import Process
 
 
-# hack to import SwiftBuildSupport and swift_build_support
-parent_dir = os.path.join(os.path.dirname(os.path.abspath(__file__)), '..')
-sys.path.append(parent_dir)
-support_dir = os.path.join(parent_dir, 'swift_build_support')
-sys.path.append(support_dir)
+# Allow importing swift_build_support.
+sys.path.append(os.path.join(os.path.dirname(os.path.abspath(__file__)), '..',
+                             'swift_build_support'))
+from swift_build_support import shell  # noqa (E402)
 from swift_build_support.toolchain import host_toolchain  # noqa (E402)
-from SwiftBuildSupport import check_output, check_call  # noqa (E402)
 
 toolchain = host_toolchain()
 LLVM_PROFDATA_PATH = toolchain.llvm_profdata
-_profdata_help = check_output([LLVM_PROFDATA_PATH, 'merge', '-help'])
+_profdata_help = shell.capture([LLVM_PROFDATA_PATH, 'merge', '-help'],
+                               print_command=False)
 LLVM_PROFDATA_SUPPORTS_SPARSE = 'sparse' in _profdata_help
 
 
@@ -65,7 +64,7 @@ def merge_file_buffer(self):
             llvm_cmd.append("-sparse")
         llvm_cmd += cleaned_files
         self.report(llvm_cmd)
-        ret = check_call(llvm_cmd)
+        ret = shell.call(llvm_cmd, print_command=False)
         if ret != 0:
             self.report("llvm profdata command failed -- Exited with code %d"
                         % ret, level=logging.ERROR)

utils/recursive-lipo

@@ -7,8 +7,11 @@ import filecmp
 import os
 import os.path
 import shutil
+import sys
 
-from SwiftBuildSupport import check_call
+sys.path.append(os.path.join(os.path.dirname(__file__), 'swift_build_support'))
+
+from swift_build_support import shell  # noqa (E402)
 
 
 def merge_file_lists(src_root_dirs, skip_files, skip_subpaths):
@@ -89,8 +92,8 @@ def merge_lipo_files(src_root_dirs, file_list, copy_verbatim_subpaths,
                     print("-- Running lipo %s to %s" % (file_paths, dest_path))
                 else:
                     print("-- Running lipo %s" % dest_path)
-                check_call(lipo_cmd + ["-output", dest_path] + file_paths,
-                           print_command=verbose, verbose=verbose)
+                shell.call(lipo_cmd + ["-output", dest_path] + file_paths,
+                           print_command=verbose)
             else:
                 # Multiple instances are different, and they're not executable.
                 print(

utils/swift_build_support/swift_build_support/shell.py

@@ -59,9 +59,17 @@ def _print_command(dry_run, command, env=None, prompt="+ "):
     file.flush()
 
 
-def call(command, stderr=None, env=None, dry_run=None):
+def call(command, stderr=None, env=None, dry_run=None, print_command=True):
+    """
+    call(command, ...) -> str
+
+    Execute the given command.
+
+    This function will raise an exception on any command failure.
+    """
     dry_run = _coerce_dry_run(dry_run)
-    _print_command(dry_run, command, env=env)
+    if dry_run or print_command:
+        _print_command(dry_run, command, env=env)
     if dry_run:
         return
     _env = None
@@ -80,40 +88,74 @@ def call(command, stderr=None, env=None, dry_run=None):
             "': " + e.strerror)
 
 
+def capture(command, stderr=None, env=None, dry_run=None, print_command=True):
+    """
+    capture(command, ...) -> str
+
+    Execute the given command and return the standard output.
+
+    This function will raise an exception on any command failure.
+    """
+    dry_run = _coerce_dry_run(dry_run)
+    if dry_run or print_command:
+        _print_command(dry_run, command, env=env)
+    if dry_run:
+        return
+    _env = None
+    if env is not None:
+        _env = dict(os.environ)
+        _env.update(env)
+    try:
+        return subprocess.check_output(command, env=_env, stderr=stderr)
+    except subprocess.CalledProcessError as e:
+        diagnostics.fatal(
+            "command terminated with a non-zero exit status " +
+            str(e.returncode) + ", aborting")
+    except OSError as e:
+        diagnostics.fatal(
+            "could not execute '" + quote_command(command) +
+            "': " + e.strerror)
+
+
 @contextmanager
-def pushd(path, dry_run=None):
+def pushd(path, dry_run=None, print_command=True):
     dry_run = _coerce_dry_run(dry_run)
     old_dir = os.getcwd()
-    _print_command(dry_run, ["pushd", path])
+    if dry_run or print_command:
+        _print_command(dry_run, ["pushd", path])
     if not dry_run:
         os.chdir(path)
     yield
-    _print_command(dry_run, ["popd"])
+    if dry_run or print_command:
+        _print_command(dry_run, ["popd"])
     if not dry_run:
         os.chdir(old_dir)
 
 
-def makedirs(path, dry_run=None):
+def makedirs(path, dry_run=None, print_command=True):
     dry_run = _coerce_dry_run(dry_run)
-    _print_command(dry_run, ['mkdir', '-p', path])
+    if dry_run or print_command:
+        _print_command(dry_run, ['mkdir', '-p', path])
     if dry_run:
         return
     if not os.path.isdir(path):
         os.makedirs(path)
 
 
-def rmtree(path, dry_run=None):
+def rmtree(path, dry_run=None, print_command=True):
     dry_run = _coerce_dry_run(dry_run)
-    _print_command(dry_run, ['rm', '-rf', path])
+    if dry_run or print_command:
+        _print_command(dry_run, ['rm', '-rf', path])
     if dry_run:
         return
     if os.path.exists(path):
         shutil.rmtree(path)
 
 
-def copytree(src, dest, dry_run=None):
+def copytree(src, dest, dry_run=None, print_command=True):
     dry_run = _coerce_dry_run(dry_run)
-    _print_command(dry_run, ['cp', '-r', src, dest])
+    if dry_run or print_command:
+        _print_command(dry_run, ['cp', '-r', src, dest])
     if dry_run:
         return
     shutil.copytree(src, dest)

utils/swift_build_support/tests/test_shell.py

@@ -63,6 +63,12 @@ def test_call(self):
 + cp {foo_file} {bar_file}
 '''.format(foo_file=foo_file, bar_file=bar_file))
 
+    def test_capture(self):
+        self.assertEqual(shell.capture(["echo", "hi"]), "hi\n")
+
+        with self.assertRaises(SystemExit):
+            shell.capture(["false"])
+
     def test_rmtree(self):
         shell.dry_run = False
         path = os.path.join(self.tmpdir, 'foo', 'bar')

utils/swift_build_support/tests/test_tar.py

@@ -10,7 +10,6 @@
 
 import os
 import platform
-import subprocess
 import sys
 import tempfile
 import unittest

utils/update-checkout

@@ -19,8 +19,6 @@ sys.path.append(os.path.dirname(__file__))
 
 from SwiftBuildSupport import (
     SWIFT_SOURCE_ROOT,
-    check_call,
-    check_output,
 )  # noqa (E402 module level import not at top of file)
 
 sys.path.append(os.path.join(os.path.dirname(__file__), 'swift_build_support'))
@@ -94,21 +92,23 @@ def update_working_copy(repo_path, branch):
         return
 
     print("--- Updating '" + repo_path + "' ---")
-    with shell.pushd(repo_path, dry_run=False):
+    with shell.pushd(repo_path, dry_run=False, print_command=False):
         if branch:
-            status = check_output(['git', 'status', '--porcelain'])
+            status = shell.capture(['git', 'status', '--porcelain'],
+                                   print_command=False)
             if status:
                 print("Please, commit your changes.")
                 print(status)
                 exit(1)
-            check_call(['git', 'checkout', branch])
+            shell.call(['git', 'checkout', branch], print_command=False)
 
         # Prior to Git 2.6, this is the way to do a "git pull
         # --rebase" that respects rebase.autostash.  See
         # http://stackoverflow.com/a/30209750/125349
-        check_call(["git", "fetch"])
-        check_call(["git", "rebase", "FETCH_HEAD"])
-        check_call(["git", "submodule", "update", "--recursive"])
+        shell.call(["git", "fetch"], print_command=False)
+        shell.call(["git", "rebase", "FETCH_HEAD"], print_command=False)
+        shell.call(["git", "submodule", "update", "--recursive"],
+                   print_command=False)
 
 
 def obtain_additional_swift_sources(
@@ -117,19 +117,20 @@ def obtain_additional_swift_sources(
         if dir_name in skip_repositories:
             print("--- Skipping '" + dir_name + "' ---")
             continue
-        with shell.pushd(SWIFT_SOURCE_ROOT, dry_run=False):
+        with shell.pushd(SWIFT_SOURCE_ROOT, dry_run=False,
+                         print_command=False):
             if not os.path.isdir(os.path.join(dir_name, ".git")):
                 print("--- Cloning '" + dir_name + "' ---")
                 if with_ssh is True:
                     remote = "[email protected]:" + repo + '.git'
                 else:
                     remote = "https://github.com/" + repo + '.git'
                 if skip_history:
-                    check_call(['git', 'clone', '--recursive', '--depth', '1',
-                                remote, dir_name])
+                    shell.call(['git', 'clone', '--recursive', '--depth', '1',
+                                remote, dir_name], print_command=False)
                 else:
-                    check_call(['git', 'clone', '--recursive', remote,
-                                dir_name])
+                    shell.call(['git', 'clone', '--recursive', remote,
+                                dir_name], print_command=False)
                 if branch:
                     if branch == "master" or branch == "stable":
                         repo_branch = MASTER_BRANCHES[dir_name]
@@ -142,9 +143,9 @@ def obtain_additional_swift_sources(
                         repo_branch = branch
                     src_path = SWIFT_SOURCE_ROOT + "/" + dir_name + "/" + \
                         ".git"
-                    check_call(['git', '--git-dir', src_path, '--work-tree',
+                    shell.call(['git', '--git-dir', src_path, '--work-tree',
                                 os.path.join(SWIFT_SOURCE_ROOT, dir_name),
-                                'checkout', repo_branch])
+                                'checkout', repo_branch], print_command=False)
 
 
 def main():