refactor: subprocess.run (#592)

* change check_call to run

* refactor: change check_output to run

* Apply suggestions from code review

Co-authored-by: Joe Rickerby <joerick@mac.com>

Co-authored-by: Joe Rickerby <joerick@mac.com>

bin/run_tests.py

@@ -14,7 +14,7 @@ if __name__ == '__main__':
     # run the docker unit tests only on Linux
     if sys.platform.startswith('linux'):
         unit_test_args += ['--run-docker']
-    subprocess.check_call(unit_test_args)
+    subprocess.run(unit_test_args, check=True)
 
     # run the integration tests
-    subprocess.check_call([sys.executable, '-m', 'pytest', '-x', '--durations', '0', '--timeout=2400', 'test'])
+    subprocess.run([sys.executable, '-m', 'pytest', '-x', '--durations', '0', '--timeout=2400', 'test'], check=True)

bin/update_dependencies.py

@@ -20,20 +20,20 @@ PYTHON_VERSIONS = ['27', '35', '36', '37', '38', '39']
 
 if '--no-docker' in sys.argv:
     for python_version in PYTHON_VERSIONS:
-        subprocess.check_call([
+        subprocess.run([
             f'./env{python_version}/bin/pip-compile',
             '--allow-unsafe',
             '--upgrade',
             'cibuildwheel/resources/constraints.in',
             '--output-file', f'cibuildwheel/resources/constraints-python{python_version}.txt'
-        ])
+        ], check=True)
 else:
     image_runner = 'quay.io/pypa/manylinux2010_x86_64:latest'
-    subprocess.check_call(['docker', 'pull', image_runner])
+    subprocess.run(['docker', 'pull', image_runner], check=True)
     for python_version in PYTHON_VERSIONS:
         abi_flags = '' if int(python_version) >= 38 else 'm'
         python_path = f'/opt/python/cp{python_version}-cp{python_version}{abi_flags}/bin/'
-        subprocess.check_call([
+        subprocess.run([
             'docker', 'run', '--rm',
             '-e', 'CUSTOM_COMPILE_COMMAND',
             '-v', f'{os.getcwd()}:/volume',
@@ -43,7 +43,7 @@ else:
             f'{python_path}pip-compile --allow-unsafe --upgrade '
             'cibuildwheel/resources/constraints.in '
             f'--output-file cibuildwheel/resources/constraints-python{python_version}.txt'
-        ])
+        ], check=True)
 
 # default constraints.txt
 shutil.copyfile(f'cibuildwheel/resources/constraints-python{PYTHON_VERSIONS[-1]}.txt', 'cibuildwheel/resources/constraints.txt')

cibuildwheel/bashlex_eval.py

@@ -8,7 +8,7 @@ EnvironmentExecutor = Callable[[List[str], Dict[str, str]], str]
 
 
 def local_environment_executor(command: List[str], env: Dict[str, str]) -> str:
-    return subprocess.check_output(command, env=env, universal_newlines=True)
+    return subprocess.run(command, env=env, universal_newlines=True, stdout=subprocess.PIPE, check=True).stdout
 
 
 class NodeExecutionContext(NamedTuple):

cibuildwheel/linux.py

@@ -49,7 +49,8 @@ def get_python_configurations(
 
 def build(options: BuildOptions) -> None:
     try:
-        subprocess.check_output(['docker', '--version'])
+        # check docker is installed
+        subprocess.run(['docker', '--version'], check=True, stdout=subprocess.DEVNULL)
     except Exception:
         print('cibuildwheel: Docker not found. Docker is required to run Linux builds. '
               'If you\'re building on Travis CI, add `services: [docker]` to your .travis.yml.'

cibuildwheel/macos.py

@@ -28,14 +28,14 @@ from .util import (
 )
 
 
-def call(args: Sequence[PathOrStr], env: Optional[Dict[str, str]] = None, cwd: Optional[str] = None, shell: bool = False) -> int:
+def call(args: Sequence[PathOrStr], env: Optional[Dict[str, str]] = None, cwd: Optional[str] = None, shell: bool = False) -> None:
     # print the command executing for the logs
     if shell:
         print(f'+ {args}')
     else:
         print('+ ' + ' '.join(shlex.quote(str(a)) for a in args))
 
-    return subprocess.check_call(args, env=env, cwd=cwd, shell=shell)
+    subprocess.run(args, env=env, cwd=cwd, shell=shell, check=True)
 
 
 def get_macos_version() -> Tuple[int, int]:
@@ -53,10 +53,12 @@ def get_macos_version() -> Tuple[int, int]:
 
 
 def get_macos_sdks() -> List[str]:
-    output = subprocess.check_output(
+    output = subprocess.run(
         ['xcodebuild', '-showsdks'],
         universal_newlines=True,
-    )
+        check=True,
+        stdout=subprocess.PIPE,
+    ).stdout
 
     return [m.group(1) for m in re.finditer(r'-sdk (macosx\S+)', output)]
 
@@ -125,7 +127,7 @@ def make_symlinks(installation_bin_path: Path, python_executable: str, pip_execu
 
 
 def install_cpython(version: str, url: str) -> Path:
-    installed_system_packages = subprocess.check_output(['pkgutil', '--pkgs'], universal_newlines=True).splitlines()
+    installed_system_packages = subprocess.run(['pkgutil', '--pkgs'], universal_newlines=True, check=True, stdout=subprocess.PIPE).stdout.splitlines()
 
     # if this version of python isn't installed, get it from python.org and install
     python_package_identifier = f'org.python.Python.PythonFramework-{version}'
@@ -211,7 +213,7 @@ def setup_python(python_configuration: PythonConfiguration,
     # check what version we're on
     call(['which', 'python'], env=env)
     call(['python', '--version'], env=env)
-    which_python = subprocess.check_output(['which', 'python'], env=env, universal_newlines=True).strip()
+    which_python = subprocess.run(['which', 'python'], env=env, universal_newlines=True, check=True, stdout=subprocess.PIPE).stdout.strip()
     if which_python != '/tmp/cibw_bin/python':
         print("cibuildwheel: python available on PATH doesn't match our installed instance. If you have modified PATH, ensure that you don't overwrite cibuildwheel's entry or insert python above it.", file=sys.stderr)
         sys.exit(1)
@@ -221,7 +223,7 @@ def setup_python(python_configuration: PythonConfiguration,
     assert (installation_bin_path / 'pip').exists()
     call(['which', 'pip'], env=env)
     call(['pip', '--version'], env=env)
-    which_pip = subprocess.check_output(['which', 'pip'], env=env, universal_newlines=True).strip()
+    which_pip = subprocess.run(['which', 'pip'], env=env, universal_newlines=True, check=True, stdout=subprocess.PIPE).stdout.strip()
     if which_pip != '/tmp/cibw_bin/pip':
         print("cibuildwheel: pip available on PATH doesn't match our installed instance. If you have modified PATH, ensure that you don't overwrite cibuildwheel's entry or insert pip above it.", file=sys.stderr)
         sys.exit(1)
@@ -413,12 +415,12 @@ def build(options: BuildOptions) -> None:
                             raise RuntimeError("don't know how to emulate {testing_arch} on {machine_arch}")
 
                     # define a custom 'call' function that adds the arch prefix each time
-                    def call_with_arch(args: Sequence[PathOrStr], **kwargs: Any) -> int:
+                    def call_with_arch(args: Sequence[PathOrStr], **kwargs: Any) -> None:
                         if isinstance(args, str):
                             args = ' '.join(arch_prefix) + ' ' + args
                         else:
                             args = [*arch_prefix, *args]
-                        return call(args, **kwargs)
+                        call(args, **kwargs)
 
                     # Use --no-download to ensure determinism by using seed libraries
                     # built into virtualenv

cibuildwheel/windows.py

@@ -33,12 +33,12 @@ def call(args: Sequence[PathOrStr], env: Optional[Dict[str, str]] = None,
     print('+ ' + ' '.join(str(a) for a in args))
     # we use shell=True here, even though we don't need a shell due to a bug
     # https://bugs.python.org/issue8557
-    subprocess.check_call([str(a) for a in args], env=env, cwd=cwd, shell=True)
+    subprocess.run([str(a) for a in args], env=env, cwd=cwd, shell=True, check=True)
 
 
 def shell(command: str, env: Optional[Dict[str, str]] = None, cwd: Optional[str] = None) -> None:
     print(f'+ {command}')
-    subprocess.check_call(command, env=env, cwd=cwd, shell=True)
+    subprocess.run(command, env=env, cwd=cwd, shell=True, check=True)
 
 
 def get_nuget_args(version: str, arch: str) -> List[str]:
@@ -152,7 +152,7 @@ def setup_python(python_configuration: PythonConfiguration, dependency_constrain
     call(['where', 'python'], env=env)
     call(['python', '--version'], env=env)
     call(['python', '-c', '"import struct; print(struct.calcsize(\'P\') * 8)"'], env=env)
-    where_python = subprocess.check_output(['where', 'python'], env=env, universal_newlines=True).splitlines()[0].strip()
+    where_python = subprocess.run(['where', 'python'], env=env, universal_newlines=True, check=True, stdout=subprocess.PIPE).stdout.splitlines()[0].strip()
     if where_python != str(installation_path / 'python.exe'):
         print("cibuildwheel: python available on PATH doesn't match our installed instance. If you have modified PATH, ensure that you don't overwrite cibuildwheel's entry or insert python above it.", file=sys.stderr)
         sys.exit(1)
@@ -161,7 +161,7 @@ def setup_python(python_configuration: PythonConfiguration, dependency_constrain
     if not (installation_path / 'Scripts' / 'pip.exe').exists():
         call(['python', get_pip_script, *dependency_constraint_flags], env=env, cwd="C:\\cibw")
     assert (installation_path / 'Scripts' / 'pip.exe').exists()
-    where_pip = subprocess.check_output(['where', 'pip'], env=env, universal_newlines=True).splitlines()[0].strip()
+    where_pip = subprocess.run(['where', 'pip'], env=env, universal_newlines=True, check=True, stdout=subprocess.PIPE).stdout.splitlines()[0].strip()
     if where_pip.strip() != str(installation_path / 'Scripts' / 'pip.exe'):
         print("cibuildwheel: pip available on PATH doesn't match our installed instance. If you have modified PATH, ensure that you don't overwrite cibuildwheel's entry or insert pip above it.", file=sys.stderr)
         sys.exit(1)

test/test_macos_archs.py

@@ -15,10 +15,12 @@ ALL_MACOS_WHEELS = (
 
 
 def get_xcode_version() -> Tuple[int, int]:
-    output = subprocess.check_output(
+    output = subprocess.run(
         ['xcodebuild', '-version'],
         universal_newlines=True,
-    )
+        check=True,
+        stdout=subprocess.PIPE,
+    ).stdout
     lines = output.splitlines()
     _, version_str = lines[0].split()
 

test/test_projects/__main__.py

@@ -34,11 +34,11 @@ def main():
 
     if options.open:
         if sys.platform == 'darwin':
-            subprocess.check_call(['open', '--', project_dir])
+            subprocess.run(['open', '--', project_dir], check=True)
         elif sys.platform == 'linux2':
-            subprocess.check_call(['xdg-open', '--', project_dir])
+            subprocess.run(['xdg-open', '--', project_dir], check=True)
         elif sys.platform == 'win32':
-            subprocess.check_call(['explorer', project_dir])
+            subprocess.run(['explorer', project_dir], check=True)
 
 
 if __name__ == '__main__':

test/utils.py

@@ -31,11 +31,13 @@ def cibuildwheel_get_build_identifiers(project_path, env=None):
     Returns the list of build identifiers that cibuildwheel will try to build
     for the current platform.
     '''
-    cmd_output = subprocess.check_output(
+    cmd_output = subprocess.run(
         [sys.executable, '-m', 'cibuildwheel', '--print-build-identifiers', str(project_path)],
         universal_newlines=True,
         env=env,
-    )
+        check=True,
+        stdout=subprocess.PIPE,
+    ).stdout
 
     return cmd_output.strip().split('\n')
 
@@ -64,10 +66,11 @@ def cibuildwheel_run(project_path, package_dir='.', env=None, add_env=None, outp
         env.update(add_env)
 
     with TemporaryDirectoryIfNone(output_dir) as _output_dir:
-        subprocess.check_call(
+        subprocess.run(
             [sys.executable, '-m', 'cibuildwheel', '--output-dir', str(_output_dir), str(package_dir)],
             env=env,
             cwd=project_path,
+            check=True,
         )
         wheels = os.listdir(_output_dir)
     return wheels