From e417791ae4a366c75b3b5fbb0ae1a2d895e06a56 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Tue, 23 Jun 2020 19:46:23 +0100 Subject: [PATCH 01/25] Remove the linux bash script, replaced with a `docker exec` approach --- cibuildwheel/bashlex_eval.py | 17 +- cibuildwheel/environment.py | 12 +- cibuildwheel/linux.py | 445 +++++++++++++++++++---------------- requirements-dev.txt | 2 + 4 files changed, 261 insertions(+), 215 deletions(-) diff --git a/cibuildwheel/bashlex_eval.py b/cibuildwheel/bashlex_eval.py index 60ce9a81..e4865d6d 100644 --- a/cibuildwheel/bashlex_eval.py +++ b/cibuildwheel/bashlex_eval.py @@ -1,17 +1,26 @@ import shlex import subprocess -from typing import Dict, NamedTuple +from typing import Dict, NamedTuple, Callable, Optional import bashlex # type: ignore +# a function that takes a shell command and the environment, and returns the result +EnvironmentExecutor = Callable[[str, Dict[str, str]], str] + + +def local_environment_executor(command: str, env: Dict[str, str]) -> str: + return subprocess.check_output(shlex.split(command), env=env, universal_newlines=True) + + class NodeExecutionContext(NamedTuple): environment: Dict[str, str] input: str + executor: EnvironmentExecutor -def evaluate(value: str, environment: Dict[str, str]) -> str: +def evaluate(value: str, environment: Dict[str, str], executor: Optional[EnvironmentExecutor] = None) -> str: if not value: # empty string evaluates to empty string # (but trips up bashlex) @@ -26,7 +35,7 @@ def evaluate(value: str, environment: Dict[str, str]) -> str: return evaluate_node( value_word_node, - context=NodeExecutionContext(environment=environment, input=value) + context=NodeExecutionContext(environment=environment, input=value, executor=executor or local_environment_executor) ) @@ -67,7 +76,7 @@ def evaluate_word_node(node: bashlex.ast.node, context: NodeExecutionContext) -> def evaluate_command_node(node: bashlex.ast.node, context: NodeExecutionContext) -> str: words = [evaluate_node(part, context=context) for part in node.parts] command = ' '.join(words) - return subprocess.check_output(shlex.split(command), env=context.environment, universal_newlines=True) + return context.executor(command, context.environment) def evaluate_parameter_node(node: bashlex.ast.node, context: NodeExecutionContext) -> str: diff --git a/cibuildwheel/environment.py b/cibuildwheel/environment.py index 9f7ea2c4..5ecd8c68 100644 --- a/cibuildwheel/environment.py +++ b/cibuildwheel/environment.py @@ -1,6 +1,6 @@ import bashlex # type: ignore -from typing import Dict, List, Mapping +from typing import Dict, List, Mapping, Optional from . import bashlex_eval @@ -46,9 +46,9 @@ class EnvironmentAssignment: self.name = name self.value = value - def evaluated_value(self, environment: Dict[str, str]) -> str: + def evaluated_value(self, environment: Dict[str, str], executor: Optional[bashlex_eval.EnvironmentExecutor] = None) -> str: '''Returns the value of this assignment, as evaluated in the environment''' - return bashlex_eval.evaluate(self.value, environment=environment) + return bashlex_eval.evaluate(self.value, environment=environment, executor=executor) def as_shell_assignment(self) -> str: return f'export {self.name}={self.value}' @@ -61,11 +61,13 @@ class ParsedEnvironment: def __init__(self, assignments: List[EnvironmentAssignment]): self.assignments = assignments - def as_dictionary(self, prev_environment: Mapping[str, str]) -> Dict[str, str]: + def as_dictionary(self, + prev_environment: Mapping[str, str], + executor: Optional[bashlex_eval.EnvironmentExecutor] = None) -> Dict[str, str]: environment = dict(**prev_environment) for assignment in self.assignments: - value = assignment.evaluated_value(environment=environment) + value = assignment.evaluated_value(environment=environment, executor=executor) environment[assignment.name] = value return environment diff --git a/cibuildwheel/linux.py b/cibuildwheel/linux.py index 6e42f47f..f66cc5e1 100644 --- a/cibuildwheel/linux.py +++ b/cibuildwheel/linux.py @@ -1,27 +1,16 @@ -import os +import json import platform import shlex import subprocess import sys import textwrap import uuid +from os import PathLike from pathlib import Path, PurePath +from typing import Dict, List, NamedTuple, Optional, Union, Sequence -from typing import List, NamedTuple, Optional, Union - -from .util import ( - BuildOptions, - BuildSelector, - get_build_verbosity_extra_flags, - prepare_command, -) - - -def call(args: List[str], input: Optional[Union[str, bytes]] = None, universal_newlines: bool = False) -> None: - print('+ ' + ' '.join(shlex.quote(a) for a in args)) - subprocess.run( - args, input=input, universal_newlines=universal_newlines, check=True - ) +from .util import (BuildOptions, BuildSelector, + get_build_verbosity_extra_flags, prepare_command) def matches_platform(identifier: str) -> bool: @@ -48,37 +37,41 @@ def matches_platform(identifier: str) -> bool: class PythonConfiguration(NamedTuple): version: str identifier: str - path: str + path_str: str + + @property + def path(self): + return PurePath(self.path_str) def get_python_configurations(build_selector: BuildSelector) -> List[PythonConfiguration]: python_configurations = [ - PythonConfiguration(version='2.7', identifier='cp27-manylinux_x86_64', path='/opt/python/cp27-cp27m'), - PythonConfiguration(version='2.7', identifier='cp27-manylinux_x86_64', path='/opt/python/cp27-cp27mu'), - PythonConfiguration(version='3.5', identifier='cp35-manylinux_x86_64', path='/opt/python/cp35-cp35m'), - PythonConfiguration(version='3.6', identifier='cp36-manylinux_x86_64', path='/opt/python/cp36-cp36m'), - PythonConfiguration(version='3.7', identifier='cp37-manylinux_x86_64', path='/opt/python/cp37-cp37m'), - PythonConfiguration(version='3.8', identifier='cp38-manylinux_x86_64', path='/opt/python/cp38-cp38'), - PythonConfiguration(version='2.7', identifier='cp27-manylinux_i686', path='/opt/python/cp27-cp27m'), - PythonConfiguration(version='2.7', identifier='cp27-manylinux_i686', path='/opt/python/cp27-cp27mu'), - PythonConfiguration(version='3.5', identifier='cp35-manylinux_i686', path='/opt/python/cp35-cp35m'), - PythonConfiguration(version='3.6', identifier='cp36-manylinux_i686', path='/opt/python/cp36-cp36m'), - PythonConfiguration(version='3.7', identifier='cp37-manylinux_i686', path='/opt/python/cp37-cp37m'), - PythonConfiguration(version='3.8', identifier='cp38-manylinux_i686', path='/opt/python/cp38-cp38'), - PythonConfiguration(version='2.7', identifier='pp27-manylinux_x86_64', path='/opt/python/pp27-pypy_73'), - PythonConfiguration(version='3.6', identifier='pp36-manylinux_x86_64', path='/opt/python/pp36-pypy36_pp73'), - PythonConfiguration(version='3.5', identifier='cp35-manylinux_aarch64', path='/opt/python/cp35-cp35m'), - PythonConfiguration(version='3.6', identifier='cp36-manylinux_aarch64', path='/opt/python/cp36-cp36m'), - PythonConfiguration(version='3.7', identifier='cp37-manylinux_aarch64', path='/opt/python/cp37-cp37m'), - PythonConfiguration(version='3.8', identifier='cp38-manylinux_aarch64', path='/opt/python/cp38-cp38'), - PythonConfiguration(version='3.5', identifier='cp35-manylinux_ppc64le', path='/opt/python/cp35-cp35m'), - PythonConfiguration(version='3.6', identifier='cp36-manylinux_ppc64le', path='/opt/python/cp36-cp36m'), - PythonConfiguration(version='3.7', identifier='cp37-manylinux_ppc64le', path='/opt/python/cp37-cp37m'), - PythonConfiguration(version='3.8', identifier='cp38-manylinux_ppc64le', path='/opt/python/cp38-cp38'), - PythonConfiguration(version='3.5', identifier='cp35-manylinux_s390x', path='/opt/python/cp35-cp35m'), - PythonConfiguration(version='3.6', identifier='cp36-manylinux_s390x', path='/opt/python/cp36-cp36m'), - PythonConfiguration(version='3.7', identifier='cp37-manylinux_s390x', path='/opt/python/cp37-cp37m'), - PythonConfiguration(version='3.8', identifier='cp38-manylinux_s390x', path='/opt/python/cp38-cp38'), + PythonConfiguration(version='2.7', identifier='cp27-manylinux_x86_64', path_str='/opt/python/cp27-cp27m'), + PythonConfiguration(version='2.7', identifier='cp27-manylinux_x86_64', path_str='/opt/python/cp27-cp27mu'), + PythonConfiguration(version='3.5', identifier='cp35-manylinux_x86_64', path_str='/opt/python/cp35-cp35m'), + PythonConfiguration(version='3.6', identifier='cp36-manylinux_x86_64', path_str='/opt/python/cp36-cp36m'), + PythonConfiguration(version='3.7', identifier='cp37-manylinux_x86_64', path_str='/opt/python/cp37-cp37m'), + PythonConfiguration(version='3.8', identifier='cp38-manylinux_x86_64', path_str='/opt/python/cp38-cp38'), + PythonConfiguration(version='2.7', identifier='cp27-manylinux_i686', path_str='/opt/python/cp27-cp27m'), + PythonConfiguration(version='2.7', identifier='cp27-manylinux_i686', path_str='/opt/python/cp27-cp27mu'), + PythonConfiguration(version='3.5', identifier='cp35-manylinux_i686', path_str='/opt/python/cp35-cp35m'), + PythonConfiguration(version='3.6', identifier='cp36-manylinux_i686', path_str='/opt/python/cp36-cp36m'), + PythonConfiguration(version='3.7', identifier='cp37-manylinux_i686', path_str='/opt/python/cp37-cp37m'), + PythonConfiguration(version='3.8', identifier='cp38-manylinux_i686', path_str='/opt/python/cp38-cp38'), + PythonConfiguration(version='2.7', identifier='pp27-manylinux_x86_64', path_str='/opt/python/pp27-pypy_73'), + PythonConfiguration(version='3.6', identifier='pp36-manylinux_x86_64', path_str='/opt/python/pp36-pypy36_pp73'), + PythonConfiguration(version='3.5', identifier='cp35-manylinux_aarch64', path_str='/opt/python/cp35-cp35m'), + PythonConfiguration(version='3.6', identifier='cp36-manylinux_aarch64', path_str='/opt/python/cp36-cp36m'), + PythonConfiguration(version='3.7', identifier='cp37-manylinux_aarch64', path_str='/opt/python/cp37-cp37m'), + PythonConfiguration(version='3.8', identifier='cp38-manylinux_aarch64', path_str='/opt/python/cp38-cp38'), + PythonConfiguration(version='3.5', identifier='cp35-manylinux_ppc64le', path_str='/opt/python/cp35-cp35m'), + PythonConfiguration(version='3.6', identifier='cp36-manylinux_ppc64le', path_str='/opt/python/cp36-cp36m'), + PythonConfiguration(version='3.7', identifier='cp37-manylinux_ppc64le', path_str='/opt/python/cp37-cp37m'), + PythonConfiguration(version='3.8', identifier='cp38-manylinux_ppc64le', path_str='/opt/python/cp38-cp38'), + PythonConfiguration(version='3.5', identifier='cp35-manylinux_s390x', path_str='/opt/python/cp35-cp35m'), + PythonConfiguration(version='3.6', identifier='cp36-manylinux_s390x', path_str='/opt/python/cp36-cp36m'), + PythonConfiguration(version='3.7', identifier='cp37-manylinux_s390x', path_str='/opt/python/cp37-cp37m'), + PythonConfiguration(version='3.8', identifier='cp38-manylinux_s390x', path_str='/opt/python/cp38-cp38'), ] # skip builds as required return [c for c in python_configurations if matches_platform(c.identifier) and build_selector(c.identifier)] @@ -111,201 +104,133 @@ def build(options: BuildOptions) -> None: raise Exception('package_dir must be inside the working directory') container_package_dir = PurePath('/project') / abs_package_dir.relative_to(cwd) + container_output_dir = PurePath('/output') for implementation, platform_tag, docker_image in platforms: platform_configs = [c for c in python_configurations if c.identifier.startswith(implementation) and c.identifier.endswith(platform_tag)] if not platform_configs: continue - shell_cmd = ['linux32', '/bin/bash'] if platform_tag.endswith("i686") else ['/bin/bash'] - - container_name = f'cibuildwheel-{uuid.uuid4()}' - call(['docker', 'create', - '--env', 'CIBUILDWHEEL', - '--name', container_name, - '-i', - '-v', '/:/host', # ignored on CircleCI - docker_image, - '/bin/bash']) - try: - call(['docker', 'cp', '.', container_name + ':/project']) + with DockerContainer(docker_image, simulate_32_bit=platform_tag.endswith('i686')) as docker: + docker.copy_into(Path.cwd(), Path('/project')) - call(['docker', 'start', container_name]) + if options.before_all: + env = docker.get_environment() + env['PATH'] = f'/opt/python/cp38-cp38:{env["PATH"]}' + env = options.environment.as_dictionary(env, executor=docker.environment_executor) - if options.before_all: - call( - ['docker', 'exec', '-i', container_name] + shell_cmd, - universal_newlines=True, - input=''' - PS4=' + ' + before_all_prepared = prepare_command(options.before_all, project='/project', package=container_package_dir) + docker.call(['sh', '-c', before_all_prepared], env=env) - set -o errexit - set -o xtrace + for config in platform_configs: + dependency_constraint_flags: List[Union[str, PathLike]] = [] - # add a modern Python interpreter to PATH so it can be used by BEFORE_ALL - # commands - export PATH=/opt/python/cp38-cp38:$PATH + if options.dependency_constraints: + constraints_file = options.dependency_constraints.get_for_python_version(config.version) + container_constraints_file = PurePath('/constraints.txt') - {environment_exports} + docker.copy_into(constraints_file, container_constraints_file) + dependency_constraint_flags = ['-c', container_constraints_file] - sh -c {before_all} + env = docker.get_environment() - '''.format( - environment_exports='\n'.join(options.environment.as_shell_commands()), - before_all=shlex.quote(prepare_command(options.before_all, project='/project', package=container_package_dir)) + # put this config's python top of the list + python_bin = config.path / 'bin' + env['PATH'] = f'{str(python_bin)}:{env["PATH"]}' + + env = options.environment.as_dictionary(env, executor=docker.environment_executor) + + # check config python and pip are still on PATH + which_python = docker.call(['which', 'python'], env=env, capture_output=True).strip() + if PurePath(which_python) != python_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) + exit(1) + + which_pip = docker.call(['which', 'pip'], env=env, capture_output=True).strip() + if PurePath(which_pip) != python_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) + exit(1) + + if options.before_build: + before_build_prepared = prepare_command(options.before_build, project='/project', package=container_package_dir) + docker.call(['sh', '-c', before_build_prepared], env=env) + + temp_dir = PurePath('/tmp/cibuildwheel') + built_wheel_dir = temp_dir / 'built_wheel' + docker.call(['rm', '-rf', built_wheel_dir]) + docker.call(['mkdir', '-p', built_wheel_dir]) + + docker.call( + [ + 'pip', 'wheel', + container_package_dir, + '-w', built_wheel_dir, + '--no-deps', + *get_build_verbosity_extra_flags(options.build_verbosity) + ], + env=env, ) - ) - for config in platform_configs: - if options.dependency_constraints: - constraints_file = options.dependency_constraints.get_for_python_version(config.version) + built_wheel = docker.glob(built_wheel_dir / '*.whl')[0] - # `docker cp` causes 'no space left on device' error when - # a container is running and the host filesystem is - # mounted. https://github.com/moby/moby/issues/38995 - # Use `docker exec` instead. - with open(constraints_file, 'rb') as f: - call( - ['docker', 'exec', '-i', container_name, 'sh', '-c', 'cat > /constraints.txt'], - input=f.read(), - ) + repaired_wheel_dir = temp_dir / 'repaired_wheel' + docker.call(['rm', '-rf', repaired_wheel_dir]) + docker.call(['mkdir', '-p', repaired_wheel_dir]) - call( - ['docker', 'exec', '-i', container_name] + shell_cmd, - universal_newlines=True, - input=''' - # give xtrace output an extra level of indent inside docker - PS4=' + ' + if built_wheel.name.endswith('none-any.whl') or not options.repair_command: + docker.call(['mv', built_wheel, repaired_wheel_dir]) + else: + repair_command_prepared = prepare_command(options.repair_command, wheel=built_wheel, dest_dir=repaired_wheel_dir) + docker.call(['sh', '-c', repair_command_prepared], env=env) - set -o errexit - set -o xtrace - mkdir -p /output - cd /project + repaired_wheels = docker.glob(repaired_wheel_dir / '*.whl') - PYBIN="{config_python_bin}" + if options.test_command: + # set up a virtual environment to install and test from, to make sure + # there are no dependencies that were pulled in at build time. + docker.call(['pip', 'install', 'virtualenv', *dependency_constraint_flags], env=env) + venv_dir = PurePath(docker.call(['mktemp', '-d'], capture_output=True).strip()) / 'venv' - export PATH="$PYBIN:$PATH" - {environment_exports} + docker.call(['python', '-m', 'virtualenv', '--no-download', venv_dir], env=env) - # check the active python and pip are in PYBIN - if [ "$(which pip)" != "$PYBIN/pip" ]; then - echo "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." - exit 1 - fi - if [ "$(which python)" != "$PYBIN/python" ]; then - echo "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." - exit 1 - fi + virtualenv_env = env.copy() + virtualenv_env['PATH'] = f"{venv_dir / 'bin'}:{virtualenv_env['PATH']}" - if [ ! -z {before_build} ]; then - sh -c {before_build} - fi + if options.before_test: + before_test_prepared = prepare_command(options.before_test, project='/project', package=container_package_dir) + docker.call(['sh', '-c', before_test_prepared], env=virtualenv_env) - # Build the wheel - rm -rf /tmp/built_wheel - mkdir /tmp/built_wheel - pip wheel {package_dir} -w /tmp/built_wheel --no-deps {build_verbosity_flag} - built_wheel=(/tmp/built_wheel/*.whl) + # Install the wheel we just built + # Note: If auditwheel produced two wheels, it's because the earlier produced wheel + # conforms to multiple manylinux standards. These multiple versions of the wheel are + # functionally the same, differing only in name, wheel metadata, and possibly include + # different external shared libraries. so it doesn't matter which one we run the tests on. + # Let's just pick the first one. + wheel_to_test = repaired_wheels[0] + docker.call(['pip', 'install', str(wheel_to_test) + options.test_extras], env=virtualenv_env) - # repair the wheel - rm -rf /tmp/repaired_wheels - mkdir /tmp/repaired_wheels - # NOTE: 'built_wheel' here is a bash array of glob matches; "$built_wheel" returns - # the first element - if [[ "$built_wheel" == *none-any.whl ]] || [ -z {repair_command} ]; then - # pure Python wheel or empty repair command - mv "$built_wheel" /tmp/repaired_wheels - else - sh -c {repair_command} repair_command "$built_wheel" - fi - repaired_wheels=(/tmp/repaired_wheels/*.whl) + # Install any requirements to run the tests + if options.test_requires: + docker.call(['pip', 'install', *options.test_requires], env=virtualenv_env) - if [ ! -z {test_command} ]; then - # Set up a virtual environment to install and test from, to make sure - # there are no dependencies that were pulled in at build time. - pip install {dependency_install_flags} virtualenv - venv_dir=`mktemp -d`/venv - python -m virtualenv --no-download "$venv_dir" + # Run the tests from a different directory + test_command_prepared = prepare_command(options.test_command, project='/project', package=container_package_dir) + docker.call(['sh', '-c', test_command_prepared], cwd='/root', env=virtualenv_env) - # run the tests in a subshell to keep that `activate` - # script from polluting the env - ( - source "$venv_dir/bin/activate" + # clean up test environment + docker.call(['rm', '-rf', venv_dir]) - echo "Running tests using `which python`" + # move repaired wheels to output + docker.call(['mkdir', '-p', container_output_dir]) + docker.call(['mv', *repaired_wheels, container_output_dir]) - if [ ! -z {before_test} ]; then - sh -c {before_test} - fi - - # Install the wheel we just built - # Note: If auditwheel produced two wheels, it's because the earlier produced wheel - # conforms to multiple manylinux standards. These multiple versions of the wheel are - # functionally the same, differing only in name, wheel metadata, and possibly include - # different external shared libraries. so it doesn't matter which one we run the tests on. - # Let's just pick the first one. - pip install "${{repaired_wheels[0]}}"{test_extras} - - # Install any requirements to run the tests - if [ ! -z "{test_requires}" ]; then - pip install {test_requires} - fi - - # Run the tests from a different directory - pushd $HOME - sh -c {test_command} - popd - ) - # exit if tests failed (needed for older bash versions) - if [ $? -ne 0 ]; then - exit 1; - fi - - # clean up - rm -rf "$venv_dir" - fi - - # we're all done here; move it to output - mv "${{repaired_wheels[@]}}" /output - for repaired_wheel in "${{repaired_wheels[@]}}"; do - chown {uid}:{gid} "/output/$(basename "$repaired_wheel")" - done - '''.format( - config_python_bin=config.path + '/bin', - package_dir=container_package_dir, - test_requires=' '.join(options.test_requires), - test_extras=options.test_extras, - test_command=shlex.quote( - prepare_command(options.test_command, project='/project', package=container_package_dir) if options.test_command else '' - ), - before_build=shlex.quote( - prepare_command(options.before_build, project='/project', package=container_package_dir) if options.before_build else '' - ), - build_verbosity_flag=' '.join(get_build_verbosity_extra_flags(options.build_verbosity)), - repair_command=shlex.quote( - prepare_command(options.repair_command, wheel='"$1"', dest_dir='/tmp/repaired_wheels') if options.repair_command else '' - ), - environment_exports='\n'.join(options.environment.as_shell_commands()), - uid=os.getuid(), - gid=os.getgid(), - before_test=shlex.quote( - prepare_command(options.before_test, project='/project', package=container_package_dir) if options.before_test else '' - ), - dependency_install_flags='-c /constraints.txt' if options.dependency_constraints else '', - ) - ) - - # copy the output back into the host - call(['docker', 'cp', - container_name + ':/output/.', - str(options.output_dir.resolve())]) + # copy the output back into the host + docker.copy_out(container_output_dir, options.output_dir) except subprocess.CalledProcessError as error: + print(f'Command {error.cmd} failed with code {error.returncode}. {error.stdout}') troubleshoot(options.package_dir, error) exit(1) - finally: - # Still gets executed, even when 'exit(1)' gets called - call(['docker', 'rm', '--force', '-v', container_name]) def troubleshoot(package_dir: Path, error: Exception) -> None: @@ -328,3 +253,111 @@ def troubleshoot(package_dir: Path, error: Exception) -> None: print(' Files detected:') print('\n'.join([f' {f}' for f in so_files])) print('') + + +class DockerContainer: + UTILITY_PYTHON = '/opt/python/cp38-cp38/bin/python' + + def __init__(self, docker_image, simulate_32_bit=False): + self.docker_image = docker_image + self.simulate_32_bit = simulate_32_bit + + def __enter__(self) -> 'DockerContainer': + self.container_name = f'cibuildwheel-{uuid.uuid4()}' + subprocess.run( + [ + 'docker', 'create', + '--env', 'CIBUILDWHEEL', + '--name', self.container_name, + '-i', + '-v', '/:/host', # ignored on CircleCI + self.docker_image, + '/bin/bash' + ], + check=True, + ) + subprocess.run(['docker', 'start', self.container_name], check=True) + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + subprocess.run(['docker', 'rm', '--force', '-v', self.container_name]) + self.container_name = None + + def copy_into(self, from_path: Path, to_path: PurePath) -> None: + # `docker cp` causes 'no space left on device' error when + # a container is running and the host filesystem is + # mounted. https://github.com/moby/moby/issues/38995 + # Use `docker exec` instead. + if from_path.is_dir(): + self.call(['mkdir', '-p', to_path]) + subprocess.run( + f'tar cf - . | docker exec -i {self.container_name} tar -xC {to_path} -f -', + shell=True, + check=True, + cwd=from_path) + else: + subprocess.run( + f'cat {from_path} | docker exec -i {self.container_name} sh -c "cat > {to_path}"', + shell=True, + check=True) + + def copy_out(self, from_path: PurePath, to_path: Path) -> None: + # note: we assume from_path is a dir + to_path.mkdir(parents=True, exist_ok=True) + + subprocess.run( + f'docker exec -i -w {from_path} {self.container_name} tar cf - . | tar -xf -', + shell=True, + check=True, + cwd=to_path + ) + + def glob(self, pattern: PurePath) -> List[PurePath]: + path_strs = json.loads(self.call([ + self.UTILITY_PYTHON, + '-c', + f'import sys, json, glob; json.dump(glob.glob({str(pattern)!r}), sys.stdout)' + ], capture_output=True)) + + return [PurePath(p) for p in path_strs] + + def call(self, args: Sequence[Union[str, PathLike]], env: Dict[str, str] = {}, + capture_output=False, cwd: Optional[Union[str, PathLike]] = None) -> str: + # ensure paths are converted to strs + args = [str(a) for a in args] + + if self.simulate_32_bit: + args = ['linux32', *args] + + options = ['-i'] + + for key, value in env.items(): + options += ['--env', f'{key}={value}'] + + if cwd: + options += ['-w', str(cwd)] + + print('+ ' + ' '.join(shlex.quote(str(a)) for a in args)) + + process = subprocess.run( + ['docker', 'exec', *options, self.container_name, *args], + check=True, + universal_newlines=True, + stdout=subprocess.PIPE if capture_output else None + ) + + if capture_output: + return process.stdout + else: + return '' + + def get_environment(self) -> Dict[str, str]: + return json.loads(self.call([ + self.UTILITY_PYTHON, + '-c', + 'import sys, json, os; json.dump(os.environ.copy(), sys.stdout)' + ], capture_output=True)) + + def environment_executor(self, command: str, environment: Dict[str, str]) -> str: + # used as an EnvironmentExecutor to evaluate commands and capture output + return self.call(shlex.split(command), env=environment) diff --git a/requirements-dev.txt b/requirements-dev.txt index 44043506..e6129684 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -6,3 +6,5 @@ pymdown-extensions pip-tools requests click +mypy +typing-extensions From 53c00a0af686399ae12ecb9f0fed01245fadb93f Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Wed, 24 Jun 2020 16:39:20 +0100 Subject: [PATCH 02/25] Use a long-running remote bash shell, rather than `docker exec` --- cibuildwheel/linux.py | 102 +++++++++++++++++++++++++++++++----------- setup.cfg | 2 +- 2 files changed, 78 insertions(+), 26 deletions(-) diff --git a/cibuildwheel/linux.py b/cibuildwheel/linux.py index f66cc5e1..14c63adb 100644 --- a/cibuildwheel/linux.py +++ b/cibuildwheel/linux.py @@ -1,3 +1,4 @@ +import io import json import platform import shlex @@ -7,7 +8,8 @@ import textwrap import uuid from os import PathLike from pathlib import Path, PurePath -from typing import Dict, List, NamedTuple, Optional, Union, Sequence +from typing import (IO, Dict, List, NamedTuple, Optional, Sequence, TextIO, + Union) from .util import (BuildOptions, BuildSelector, get_build_verbosity_extra_flags, prepare_command) @@ -258,12 +260,17 @@ def troubleshoot(package_dir: Path, error: Exception) -> None: class DockerContainer: UTILITY_PYTHON = '/opt/python/cp38-cp38/bin/python' - def __init__(self, docker_image, simulate_32_bit=False): + process: subprocess.Popen + bash_stdin: IO[str] + bash_stdout: IO[str] + + def __init__(self, docker_image: str, simulate_32_bit=False): self.docker_image = docker_image self.simulate_32_bit = simulate_32_bit def __enter__(self) -> 'DockerContainer': self.container_name = f'cibuildwheel-{uuid.uuid4()}' + shell_args = ['linux32', '/bin/bash'] if self.simulate_32_bit else ['/bin/bash'] subprocess.run( [ 'docker', 'create', @@ -272,14 +279,34 @@ class DockerContainer: '-i', '-v', '/:/host', # ignored on CircleCI self.docker_image, - '/bin/bash' + *shell_args ], check=True, ) - subprocess.run(['docker', 'start', self.container_name], check=True) + process = subprocess.Popen( + [ + 'docker', 'start', + '--attach', '--interactive', + self.container_name, + ], + encoding='utf8', + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + # make the input buffer large enough to carry a lot of environment + # variables. We choose 256kB. + bufsize=262144, + ) + self.process = process + assert process.stdin and process.stdout + self.bash_stdin = process.stdin + self.bash_stdout = process.stdout return self def __exit__(self, exc_type, exc_val, exc_tb): + self.bash_stdin.close() + self.process.terminate() + self.process.wait() + subprocess.run(['docker', 'rm', '--force', '-v', self.container_name]) self.container_name = None @@ -323,33 +350,58 @@ class DockerContainer: def call(self, args: Sequence[Union[str, PathLike]], env: Dict[str, str] = {}, capture_output=False, cwd: Optional[Union[str, PathLike]] = None) -> str: - # ensure paths are converted to strs - args = [str(a) for a in args] + env_exports = '\n'.join(f'export {k}={v}' for k, v in env.items()) + chdir = f'cd {cwd}' if cwd else '' + command = ' '.join(shlex.quote(str(a)) for a in args) + end_of_message = str(uuid.uuid4()) - if self.simulate_32_bit: - args = ['linux32', *args] + # log the command we're executing + print(f' + {command}') - options = ['-i'] - - for key, value in env.items(): - options += ['--env', f'{key}={value}'] - - if cwd: - options += ['-w', str(cwd)] - - print('+ ' + ' '.join(shlex.quote(str(a)) for a in args)) - - process = subprocess.run( - ['docker', 'exec', *options, self.container_name, *args], - check=True, - universal_newlines=True, - stdout=subprocess.PIPE if capture_output else None + # Write a command to the remote shell. First we write the + # environment variables, exported inside the subshell. We change the + # cwd, if that's required. Then, the command is written. Finally, the + # remote shell is told to write a footer - this will show up in the + # output so we know when to stop reading, and will include the + # returncode of `command`. + self.bash_stdin.write(f'''( + {env_exports} + {chdir} + {command} + printf "%04d%s\n" $? {end_of_message} ) + ''') + self.bash_stdin.flush() if capture_output: - return process.stdout + output_io: TextIO = io.StringIO() else: - return '' + output_io = sys.stdout + + while True: + line = self.bash_stdout.readline() + + if line.endswith(end_of_message+'\n'): + footer_offset = ( + len(line) + - 1 # newline character + - len(end_of_message) # delimiter + - 4 # 4 returncode decimals + ) + returncode_str = line[footer_offset:footer_offset+4] + returncode = int(returncode_str) + # add the last line to output, without the footer + output_io.write(line[0:footer_offset]) + break + else: + output_io.write(line) + + output = output_io.getvalue() if isinstance(output_io, io.StringIO) else None + + if returncode != 0: + raise subprocess.CalledProcessError(returncode, args, output) + + return output if output else '' def get_environment(self) -> Dict[str, str]: return json.loads(self.call([ diff --git a/setup.cfg b/setup.cfg index d54a42bb..e98764a2 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,5 +1,5 @@ [flake8] -ignore = E501,W503,E741 +ignore = E501,W503,E741,E226 application-import-names = cibuildwheel exclude = cibuildwheel/resources/, From 31339ba30e8e73e8d0956e13f62399d627f066b1 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Wed, 24 Jun 2020 16:43:33 +0100 Subject: [PATCH 03/25] Extract DockerContainer into its own file --- cibuildwheel/docker_container.py | 177 +++++++++++++++++++++++++++++++ cibuildwheel/linux.py | 166 +---------------------------- 2 files changed, 179 insertions(+), 164 deletions(-) create mode 100644 cibuildwheel/docker_container.py diff --git a/cibuildwheel/docker_container.py b/cibuildwheel/docker_container.py new file mode 100644 index 00000000..9ba7509d --- /dev/null +++ b/cibuildwheel/docker_container.py @@ -0,0 +1,177 @@ +import io +import json +import shlex +import subprocess +import sys +import uuid +from os import PathLike +from pathlib import Path, PurePath +from typing import IO, Dict, List, Optional, Sequence, TextIO, Union + + +class DockerContainer: + ''' + An object that represents a running Docker container. + + Intended for use as a context manager e.g. + `with DockerContainer('ubuntu') as docker:` + + A bash shell is running in the remote container. When `call()` is invoked, + the command is relayed to the remote shell, and the results are streamed + back to cibuildwheel. + ''' + UTILITY_PYTHON = '/opt/python/cp38-cp38/bin/python' + + process: subprocess.Popen + bash_stdin: IO[str] + bash_stdout: IO[str] + + def __init__(self, docker_image: str, simulate_32_bit=False): + self.docker_image = docker_image + self.simulate_32_bit = simulate_32_bit + + def __enter__(self) -> 'DockerContainer': + self.container_name = f'cibuildwheel-{uuid.uuid4()}' + shell_args = ['linux32', '/bin/bash'] if self.simulate_32_bit else ['/bin/bash'] + subprocess.run( + [ + 'docker', 'create', + '--env', 'CIBUILDWHEEL', + '--name', self.container_name, + '-i', + '-v', '/:/host', # ignored on CircleCI + self.docker_image, + *shell_args + ], + check=True, + ) + process = subprocess.Popen( + [ + 'docker', 'start', + '--attach', '--interactive', + self.container_name, + ], + encoding='utf8', + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + # make the input buffer large enough to carry a lot of environment + # variables. We choose 256kB. + bufsize=262144, + ) + self.process = process + assert process.stdin and process.stdout + self.bash_stdin = process.stdin + self.bash_stdout = process.stdout + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + self.bash_stdin.close() + self.process.terminate() + self.process.wait() + + subprocess.run(['docker', 'rm', '--force', '-v', self.container_name]) + self.container_name = None + + def copy_into(self, from_path: Path, to_path: PurePath) -> None: + # `docker cp` causes 'no space left on device' error when + # a container is running and the host filesystem is + # mounted. https://github.com/moby/moby/issues/38995 + # Use `docker exec` instead. + if from_path.is_dir(): + self.call(['mkdir', '-p', to_path]) + subprocess.run( + f'tar cf - . | docker exec -i {self.container_name} tar -xC {to_path} -f -', + shell=True, + check=True, + cwd=from_path) + else: + subprocess.run( + f'cat {from_path} | docker exec -i {self.container_name} sh -c "cat > {to_path}"', + shell=True, + check=True) + + def copy_out(self, from_path: PurePath, to_path: Path) -> None: + # note: we assume from_path is a dir + to_path.mkdir(parents=True, exist_ok=True) + + subprocess.run( + f'docker exec -i -w {from_path} {self.container_name} tar cf - . | tar -xf -', + shell=True, + check=True, + cwd=to_path + ) + + def glob(self, pattern: PurePath) -> List[PurePath]: + path_strs = json.loads(self.call([ + self.UTILITY_PYTHON, + '-c', + f'import sys, json, glob; json.dump(glob.glob({str(pattern)!r}), sys.stdout)' + ], capture_output=True)) + + return [PurePath(p) for p in path_strs] + + def call(self, args: Sequence[Union[str, PathLike]], env: Dict[str, str] = {}, + capture_output=False, cwd: Optional[Union[str, PathLike]] = None) -> str: + env_exports = '\n'.join(f'export {k}={v}' for k, v in env.items()) + chdir = f'cd {cwd}' if cwd else '' + command = ' '.join(shlex.quote(str(a)) for a in args) + end_of_message = str(uuid.uuid4()) + + # log the command we're executing + print(f' + {command}') + + # Write a command to the remote shell. First we write the + # environment variables, exported inside the subshell. We change the + # cwd, if that's required. Then, the command is written. Finally, the + # remote shell is told to write a footer - this will show up in the + # output so we know when to stop reading, and will include the + # returncode of `command`. + self.bash_stdin.write(f'''( + {env_exports} + {chdir} + {command} + printf "%04d%s\n" $? {end_of_message} + ) + ''') + self.bash_stdin.flush() + + if capture_output: + output_io: TextIO = io.StringIO() + else: + output_io = sys.stdout + + while True: + line = self.bash_stdout.readline() + + if line.endswith(end_of_message+'\n'): + footer_offset = ( + len(line) + - 1 # newline character + - len(end_of_message) # delimiter + - 4 # 4 returncode decimals + ) + returncode_str = line[footer_offset:footer_offset+4] + returncode = int(returncode_str) + # add the last line to output, without the footer + output_io.write(line[0:footer_offset]) + break + else: + output_io.write(line) + + output = output_io.getvalue() if isinstance(output_io, io.StringIO) else None + + if returncode != 0: + raise subprocess.CalledProcessError(returncode, args, output) + + return output if output else '' + + def get_environment(self) -> Dict[str, str]: + return json.loads(self.call([ + self.UTILITY_PYTHON, + '-c', + 'import sys, json, os; json.dump(os.environ.copy(), sys.stdout)' + ], capture_output=True)) + + def environment_executor(self, command: str, environment: Dict[str, str]) -> str: + # used as an EnvironmentExecutor to evaluate commands and capture output + return self.call(shlex.split(command), env=environment) diff --git a/cibuildwheel/linux.py b/cibuildwheel/linux.py index 14c63adb..30481008 100644 --- a/cibuildwheel/linux.py +++ b/cibuildwheel/linux.py @@ -1,16 +1,12 @@ -import io -import json import platform -import shlex import subprocess import sys import textwrap -import uuid from os import PathLike from pathlib import Path, PurePath -from typing import (IO, Dict, List, NamedTuple, Optional, Sequence, TextIO, - Union) +from typing import List, NamedTuple, Union +from .docker_container import DockerContainer from .util import (BuildOptions, BuildSelector, get_build_verbosity_extra_flags, prepare_command) @@ -255,161 +251,3 @@ def troubleshoot(package_dir: Path, error: Exception) -> None: print(' Files detected:') print('\n'.join([f' {f}' for f in so_files])) print('') - - -class DockerContainer: - UTILITY_PYTHON = '/opt/python/cp38-cp38/bin/python' - - process: subprocess.Popen - bash_stdin: IO[str] - bash_stdout: IO[str] - - def __init__(self, docker_image: str, simulate_32_bit=False): - self.docker_image = docker_image - self.simulate_32_bit = simulate_32_bit - - def __enter__(self) -> 'DockerContainer': - self.container_name = f'cibuildwheel-{uuid.uuid4()}' - shell_args = ['linux32', '/bin/bash'] if self.simulate_32_bit else ['/bin/bash'] - subprocess.run( - [ - 'docker', 'create', - '--env', 'CIBUILDWHEEL', - '--name', self.container_name, - '-i', - '-v', '/:/host', # ignored on CircleCI - self.docker_image, - *shell_args - ], - check=True, - ) - process = subprocess.Popen( - [ - 'docker', 'start', - '--attach', '--interactive', - self.container_name, - ], - encoding='utf8', - stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - # make the input buffer large enough to carry a lot of environment - # variables. We choose 256kB. - bufsize=262144, - ) - self.process = process - assert process.stdin and process.stdout - self.bash_stdin = process.stdin - self.bash_stdout = process.stdout - return self - - def __exit__(self, exc_type, exc_val, exc_tb): - self.bash_stdin.close() - self.process.terminate() - self.process.wait() - - subprocess.run(['docker', 'rm', '--force', '-v', self.container_name]) - self.container_name = None - - def copy_into(self, from_path: Path, to_path: PurePath) -> None: - # `docker cp` causes 'no space left on device' error when - # a container is running and the host filesystem is - # mounted. https://github.com/moby/moby/issues/38995 - # Use `docker exec` instead. - if from_path.is_dir(): - self.call(['mkdir', '-p', to_path]) - subprocess.run( - f'tar cf - . | docker exec -i {self.container_name} tar -xC {to_path} -f -', - shell=True, - check=True, - cwd=from_path) - else: - subprocess.run( - f'cat {from_path} | docker exec -i {self.container_name} sh -c "cat > {to_path}"', - shell=True, - check=True) - - def copy_out(self, from_path: PurePath, to_path: Path) -> None: - # note: we assume from_path is a dir - to_path.mkdir(parents=True, exist_ok=True) - - subprocess.run( - f'docker exec -i -w {from_path} {self.container_name} tar cf - . | tar -xf -', - shell=True, - check=True, - cwd=to_path - ) - - def glob(self, pattern: PurePath) -> List[PurePath]: - path_strs = json.loads(self.call([ - self.UTILITY_PYTHON, - '-c', - f'import sys, json, glob; json.dump(glob.glob({str(pattern)!r}), sys.stdout)' - ], capture_output=True)) - - return [PurePath(p) for p in path_strs] - - def call(self, args: Sequence[Union[str, PathLike]], env: Dict[str, str] = {}, - capture_output=False, cwd: Optional[Union[str, PathLike]] = None) -> str: - env_exports = '\n'.join(f'export {k}={v}' for k, v in env.items()) - chdir = f'cd {cwd}' if cwd else '' - command = ' '.join(shlex.quote(str(a)) for a in args) - end_of_message = str(uuid.uuid4()) - - # log the command we're executing - print(f' + {command}') - - # Write a command to the remote shell. First we write the - # environment variables, exported inside the subshell. We change the - # cwd, if that's required. Then, the command is written. Finally, the - # remote shell is told to write a footer - this will show up in the - # output so we know when to stop reading, and will include the - # returncode of `command`. - self.bash_stdin.write(f'''( - {env_exports} - {chdir} - {command} - printf "%04d%s\n" $? {end_of_message} - ) - ''') - self.bash_stdin.flush() - - if capture_output: - output_io: TextIO = io.StringIO() - else: - output_io = sys.stdout - - while True: - line = self.bash_stdout.readline() - - if line.endswith(end_of_message+'\n'): - footer_offset = ( - len(line) - - 1 # newline character - - len(end_of_message) # delimiter - - 4 # 4 returncode decimals - ) - returncode_str = line[footer_offset:footer_offset+4] - returncode = int(returncode_str) - # add the last line to output, without the footer - output_io.write(line[0:footer_offset]) - break - else: - output_io.write(line) - - output = output_io.getvalue() if isinstance(output_io, io.StringIO) else None - - if returncode != 0: - raise subprocess.CalledProcessError(returncode, args, output) - - return output if output else '' - - def get_environment(self) -> Dict[str, str]: - return json.loads(self.call([ - self.UTILITY_PYTHON, - '-c', - 'import sys, json, os; json.dump(os.environ.copy(), sys.stdout)' - ], capture_output=True)) - - def environment_executor(self, command: str, environment: Dict[str, str]) -> str: - # used as an EnvironmentExecutor to evaluate commands and capture output - return self.call(shlex.split(command), env=environment) From a052c71aec1fcf17ee8da22166ed6258877fb94d Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Wed, 24 Jun 2020 16:50:08 +0100 Subject: [PATCH 04/25] Remove hard-coded shell info to use the docker ENTRYPOINT instead This lets us use the 'linux32 bash' that's specified by the i686 docker images, without specifying it explicitly. --- cibuildwheel/docker_container.py | 7 ++----- cibuildwheel/linux.py | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/cibuildwheel/docker_container.py b/cibuildwheel/docker_container.py index 9ba7509d..66eee1e0 100644 --- a/cibuildwheel/docker_container.py +++ b/cibuildwheel/docker_container.py @@ -26,13 +26,11 @@ class DockerContainer: bash_stdin: IO[str] bash_stdout: IO[str] - def __init__(self, docker_image: str, simulate_32_bit=False): + def __init__(self, docker_image: str): self.docker_image = docker_image - self.simulate_32_bit = simulate_32_bit def __enter__(self) -> 'DockerContainer': self.container_name = f'cibuildwheel-{uuid.uuid4()}' - shell_args = ['linux32', '/bin/bash'] if self.simulate_32_bit else ['/bin/bash'] subprocess.run( [ 'docker', 'create', @@ -40,8 +38,7 @@ class DockerContainer: '--name', self.container_name, '-i', '-v', '/:/host', # ignored on CircleCI - self.docker_image, - *shell_args + self.docker_image ], check=True, ) diff --git a/cibuildwheel/linux.py b/cibuildwheel/linux.py index 30481008..529e1e5b 100644 --- a/cibuildwheel/linux.py +++ b/cibuildwheel/linux.py @@ -110,7 +110,7 @@ def build(options: BuildOptions) -> None: continue try: - with DockerContainer(docker_image, simulate_32_bit=platform_tag.endswith('i686')) as docker: + with DockerContainer(docker_image) as docker: docker.copy_into(Path.cwd(), Path('/project')) if options.before_all: From 850d109582e80a821c89b1e91ccf5fadc48e4853 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Wed, 24 Jun 2020 17:12:06 +0100 Subject: [PATCH 05/25] Don't use `docker exec -w` because CircleCI doesn't support it --- cibuildwheel/docker_container.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cibuildwheel/docker_container.py b/cibuildwheel/docker_container.py index 66eee1e0..a3609890 100644 --- a/cibuildwheel/docker_container.py +++ b/cibuildwheel/docker_container.py @@ -92,7 +92,7 @@ class DockerContainer: to_path.mkdir(parents=True, exist_ok=True) subprocess.run( - f'docker exec -i -w {from_path} {self.container_name} tar cf - . | tar -xf -', + f'docker exec -i {self.container_name} tar -cC {from_path} -f - . | tar -xf -', shell=True, check=True, cwd=to_path From dd4fc69e7c25b9b964ef9424e265d82d1f5867bc Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Thu, 25 Jun 2020 10:39:26 +0100 Subject: [PATCH 06/25] Revert "Remove hard-coded shell info to use the docker ENTRYPOINT instead" This reverts commit a052c71aec1fcf17ee8da22166ed6258877fb94d. --- cibuildwheel/docker_container.py | 7 +++++-- cibuildwheel/linux.py | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/cibuildwheel/docker_container.py b/cibuildwheel/docker_container.py index a3609890..cef31d57 100644 --- a/cibuildwheel/docker_container.py +++ b/cibuildwheel/docker_container.py @@ -26,11 +26,13 @@ class DockerContainer: bash_stdin: IO[str] bash_stdout: IO[str] - def __init__(self, docker_image: str): + def __init__(self, docker_image: str, simulate_32_bit=False): self.docker_image = docker_image + self.simulate_32_bit = simulate_32_bit def __enter__(self) -> 'DockerContainer': self.container_name = f'cibuildwheel-{uuid.uuid4()}' + shell_args = ['linux32', '/bin/bash'] if self.simulate_32_bit else ['/bin/bash'] subprocess.run( [ 'docker', 'create', @@ -38,7 +40,8 @@ class DockerContainer: '--name', self.container_name, '-i', '-v', '/:/host', # ignored on CircleCI - self.docker_image + self.docker_image, + *shell_args ], check=True, ) diff --git a/cibuildwheel/linux.py b/cibuildwheel/linux.py index 529e1e5b..30481008 100644 --- a/cibuildwheel/linux.py +++ b/cibuildwheel/linux.py @@ -110,7 +110,7 @@ def build(options: BuildOptions) -> None: continue try: - with DockerContainer(docker_image) as docker: + with DockerContainer(docker_image, simulate_32_bit=platform_tag.endswith('i686')) as docker: docker.copy_into(Path.cwd(), Path('/project')) if options.before_all: From fefe128a15c6a278f0d69e3afd8b11808aceab83 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Thu, 25 Jun 2020 11:21:10 +0100 Subject: [PATCH 07/25] Fix issues with spaces in environment variables --- cibuildwheel/docker_container.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/cibuildwheel/docker_container.py b/cibuildwheel/docker_container.py index cef31d57..73529fff 100644 --- a/cibuildwheel/docker_container.py +++ b/cibuildwheel/docker_container.py @@ -112,24 +112,25 @@ class DockerContainer: def call(self, args: Sequence[Union[str, PathLike]], env: Dict[str, str] = {}, capture_output=False, cwd: Optional[Union[str, PathLike]] = None) -> str: - env_exports = '\n'.join(f'export {k}={v}' for k, v in env.items()) chdir = f'cd {cwd}' if cwd else '' + env_assignments = ' '.join(f'{shlex.quote(k)}={shlex.quote(v)}' + for k, v in env.items()) command = ' '.join(shlex.quote(str(a)) for a in args) end_of_message = str(uuid.uuid4()) # log the command we're executing print(f' + {command}') - # Write a command to the remote shell. First we write the - # environment variables, exported inside the subshell. We change the - # cwd, if that's required. Then, the command is written. Finally, the - # remote shell is told to write a footer - this will show up in the - # output so we know when to stop reading, and will include the - # returncode of `command`. + # Write a command to the remote shell. First we change the + # cwd, if that's required. Then, we use the `env` utility to run + # `command` inside the specified environment. We use `env` because it + # can cope with spaces and strange characters in the name or value. + # Finally, the remote shell is told to write a footer - this will show + # up in the output so we know when to stop reading, and will include + # the returncode of `command`. self.bash_stdin.write(f'''( - {env_exports} {chdir} - {command} + env {env_assignments} {command} printf "%04d%s\n" $? {end_of_message} ) ''') From 68d8f89fb3bc01240eae889f1ba3458c1db4018f Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Fri, 26 Jun 2020 10:59:47 +0100 Subject: [PATCH 08/25] Style tweaks --- cibuildwheel/docker_container.py | 9 ++++----- cibuildwheel/linux.py | 19 ++++++++----------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/cibuildwheel/docker_container.py b/cibuildwheel/docker_container.py index 73529fff..3855c733 100644 --- a/cibuildwheel/docker_container.py +++ b/cibuildwheel/docker_container.py @@ -45,7 +45,7 @@ class DockerContainer: ], check=True, ) - process = subprocess.Popen( + self.process = subprocess.Popen( [ 'docker', 'start', '--attach', '--interactive', @@ -58,10 +58,9 @@ class DockerContainer: # variables. We choose 256kB. bufsize=262144, ) - self.process = process - assert process.stdin and process.stdout - self.bash_stdin = process.stdin - self.bash_stdout = process.stdout + assert self.process.stdin and self.process.stdout + self.bash_stdin = self.process.stdin + self.bash_stdout = self.process.stdout return self def __exit__(self, exc_type, exc_val, exc_tb): diff --git a/cibuildwheel/linux.py b/cibuildwheel/linux.py index 30481008..60de4743 100644 --- a/cibuildwheel/linux.py +++ b/cibuildwheel/linux.py @@ -135,7 +135,7 @@ def build(options: BuildOptions) -> None: # put this config's python top of the list python_bin = config.path / 'bin' - env['PATH'] = f'{str(python_bin)}:{env["PATH"]}' + env['PATH'] = f'{python_bin}:{env["PATH"]}' env = options.environment.as_dictionary(env, executor=docker.environment_executor) @@ -159,16 +159,13 @@ def build(options: BuildOptions) -> None: docker.call(['rm', '-rf', built_wheel_dir]) docker.call(['mkdir', '-p', built_wheel_dir]) - docker.call( - [ - 'pip', 'wheel', - container_package_dir, - '-w', built_wheel_dir, - '--no-deps', - *get_build_verbosity_extra_flags(options.build_verbosity) - ], - env=env, - ) + docker.call([ + 'pip', 'wheel', + container_package_dir, + '-w', built_wheel_dir, + '--no-deps', + *get_build_verbosity_extra_flags(options.build_verbosity) + ], env=env) built_wheel = docker.glob(built_wheel_dir / '*.whl')[0] From 1d9333689f40caadac92c92f227737207562982c Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Fri, 26 Jun 2020 12:55:38 +0100 Subject: [PATCH 09/25] Explicit buffer size isn't necessary --- cibuildwheel/docker_container.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/cibuildwheel/docker_container.py b/cibuildwheel/docker_container.py index 3855c733..fd0166e4 100644 --- a/cibuildwheel/docker_container.py +++ b/cibuildwheel/docker_container.py @@ -54,8 +54,6 @@ class DockerContainer: encoding='utf8', stdin=subprocess.PIPE, stdout=subprocess.PIPE, - # make the input buffer large enough to carry a lot of environment - # variables. We choose 256kB. bufsize=262144, ) assert self.process.stdin and self.process.stdout From df69dda44ad21a629eb5dd02491f84fe956bcc4f Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Fri, 26 Jun 2020 12:59:26 +0100 Subject: [PATCH 10/25] Fix broken PATH test - it was erroring for a different reason --- cibuildwheel/bashlex_eval.py | 33 ++++++++++++++++++++++++++++++--- test/test_environment.py | 7 +++++-- unit_test/environment_test.py | 8 ++++++++ 3 files changed, 43 insertions(+), 5 deletions(-) diff --git a/cibuildwheel/bashlex_eval.py b/cibuildwheel/bashlex_eval.py index e4865d6d..c055a0a6 100644 --- a/cibuildwheel/bashlex_eval.py +++ b/cibuildwheel/bashlex_eval.py @@ -1,7 +1,7 @@ import shlex import subprocess -from typing import Dict, NamedTuple, Callable, Optional +from typing import Dict, NamedTuple, Callable, Optional, List, Sequence import bashlex # type: ignore @@ -47,7 +47,7 @@ def evaluate_node(node: bashlex.ast.node, context: NodeExecutionContext) -> str: elif node.kind == 'parameter': return evaluate_parameter_node(node, context=context) else: - raise ValueError(f'Unsupported bash construct: "{node.word}"') + raise ValueError(f'Unsupported bash construct: "{node.kind}"') def evaluate_word_node(node: bashlex.ast.node, context: NodeExecutionContext) -> str: @@ -74,7 +74,34 @@ def evaluate_word_node(node: bashlex.ast.node, context: NodeExecutionContext) -> def evaluate_command_node(node: bashlex.ast.node, context: NodeExecutionContext) -> str: - words = [evaluate_node(part, context=context) for part in node.parts] + if any(n.kind == 'operator' for n in node.parts): + return evaluate_nodes_as_compound_command(node.parts, context=context) + else: + return evaluate_nodes_as_simple_command(node.parts, context=context) + + +def evaluate_nodes_as_compound_command(nodes: Sequence[bashlex.ast.node], context: NodeExecutionContext) -> str: + # bashlex doesn't support any operators besides ';' inside command + # substitutions, so we only need to handle that case. We do so assuming + # that `set -o errexit` is on, because it's easier to code! + + result = '' + for node in nodes: + if node.kind == 'command': + result += evaluate_command_node(node, context=context) + elif node.kind == 'operator': + if node.op == ';': + pass + else: + raise ValueError(f'Unsupported bash operator: "{node.op}"') + else: + raise ValueError(f'Unsupported bash node in compound command: "{node.kind}"') + + return result + + +def evaluate_nodes_as_simple_command(nodes: List[bashlex.ast.node], context: NodeExecutionContext): + words = [evaluate_node(part, context=context) for part in nodes] command = ' '.join(words) return context.executor(command, context.environment) diff --git a/test/test_environment.py b/test/test_environment.py index 470a151f..7e5f8d10 100644 --- a/test/test_environment.py +++ b/test/test_environment.py @@ -44,7 +44,7 @@ def test(tmp_path): assert set(actual_wheels) == set(expected_wheels) -def test_overridden_path(tmp_path): +def test_overridden_path(tmp_path, capfd): project_dir = tmp_path / 'project' output_dir = tmp_path / 'output' @@ -55,7 +55,10 @@ def test_overridden_path(tmp_path): # mess up PATH, somehow with pytest.raises(subprocess.CalledProcessError): utils.cibuildwheel_run(project_dir, output_dir=output_dir, add_env={ - 'CIBW_ENVIRONMENT': '''SOMETHING="$(mkdir new_path && touch new_path/python)" PATH="$(realpath new_path):$PATH"''', + 'CIBW_ENVIRONMENT': '''SOMETHING="$(mkdir -p /new_path ; touch /new_path/python ; chmod +x /new_path/python)" PATH="/new_path:$PATH"''', 'CIBW_ENVIRONMENT_WINDOWS': '''SOMETHING="$(mkdir new_path && type nul > new_path/python.exe)" PATH="$CD\\new_path;$PATH"''', }) + assert len(os.listdir(output_dir)) == 0 + captured = capfd.readouterr() + assert "python available on PATH doesn't match our installed instance" in captured.err diff --git a/unit_test/environment_test.py b/unit_test/environment_test.py index b8b961a8..850233e3 100644 --- a/unit_test/environment_test.py +++ b/unit_test/environment_test.py @@ -94,3 +94,11 @@ def test_no_vars_pass_through(): environment_dict = environment_recipe.as_dictionary(prev_environment={'CIBUILDWHEEL': 'awesome'}) assert environment_dict == {'CIBUILDWHEEL': 'awesome'} + + +def test_operators_inside_eval(): + environment_recipe = parse_environment('SOMETHING="$(echo a ; echo b ; echo c)"') + + environment_dict = environment_recipe.as_dictionary({}) + + assert environment_dict == {'SOMETHING': 'a\nb\nc'} From 2edb33662b46f6012785b57f3f249587412cd865 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Fri, 26 Jun 2020 20:22:49 +0100 Subject: [PATCH 11/25] Import organisation --- cibuildwheel/bashlex_eval.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cibuildwheel/bashlex_eval.py b/cibuildwheel/bashlex_eval.py index c055a0a6..f4f0641d 100644 --- a/cibuildwheel/bashlex_eval.py +++ b/cibuildwheel/bashlex_eval.py @@ -1,11 +1,9 @@ import shlex import subprocess - -from typing import Dict, NamedTuple, Callable, Optional, List, Sequence +from typing import Callable, Dict, List, NamedTuple, Optional, Sequence import bashlex # type: ignore - # a function that takes a shell command and the environment, and returns the result EnvironmentExecutor = Callable[[str, Dict[str, str]], str] From 762dc807cd1ffdfcffd32e0b6493e5744668ae57 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Fri, 26 Jun 2020 21:36:52 +0100 Subject: [PATCH 12/25] Convert DockerContainer to use binary PIPEs, to handle arbitrary data --- cibuildwheel/docker_container.py | 43 +++++++++++++++++--------------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/cibuildwheel/docker_container.py b/cibuildwheel/docker_container.py index fd0166e4..3131c782 100644 --- a/cibuildwheel/docker_container.py +++ b/cibuildwheel/docker_container.py @@ -1,3 +1,4 @@ +import codecs import io import json import shlex @@ -6,7 +7,7 @@ import sys import uuid from os import PathLike from pathlib import Path, PurePath -from typing import IO, Dict, List, Optional, Sequence, TextIO, Union +from typing import IO, Any, Dict, List, Optional, Sequence, TextIO, Union, cast class DockerContainer: @@ -23,21 +24,21 @@ class DockerContainer: UTILITY_PYTHON = '/opt/python/cp38-cp38/bin/python' process: subprocess.Popen - bash_stdin: IO[str] - bash_stdout: IO[str] + bash_stdin: IO[bytes] + bash_stdout: IO[bytes] def __init__(self, docker_image: str, simulate_32_bit=False): self.docker_image = docker_image self.simulate_32_bit = simulate_32_bit def __enter__(self) -> 'DockerContainer': - self.container_name = f'cibuildwheel-{uuid.uuid4()}' + self.name = f'cibuildwheel-{uuid.uuid4()}' shell_args = ['linux32', '/bin/bash'] if self.simulate_32_bit else ['/bin/bash'] subprocess.run( [ 'docker', 'create', '--env', 'CIBUILDWHEEL', - '--name', self.container_name, + '--name', self.name, '-i', '-v', '/:/host', # ignored on CircleCI self.docker_image, @@ -49,13 +50,12 @@ class DockerContainer: [ 'docker', 'start', '--attach', '--interactive', - self.container_name, + self.name, ], - encoding='utf8', stdin=subprocess.PIPE, stdout=subprocess.PIPE, - bufsize=262144, ) + assert self.process.stdin and self.process.stdout self.bash_stdin = self.process.stdin self.bash_stdout = self.process.stdout @@ -66,8 +66,8 @@ class DockerContainer: self.process.terminate() self.process.wait() - subprocess.run(['docker', 'rm', '--force', '-v', self.container_name]) - self.container_name = None + subprocess.run(['docker', 'rm', '--force', '-v', self.name]) + self.name = None def copy_into(self, from_path: Path, to_path: PurePath) -> None: # `docker cp` causes 'no space left on device' error when @@ -77,13 +77,13 @@ class DockerContainer: if from_path.is_dir(): self.call(['mkdir', '-p', to_path]) subprocess.run( - f'tar cf - . | docker exec -i {self.container_name} tar -xC {to_path} -f -', + f'tar cf - . | docker exec -i {self.name} tar -xC {to_path} -f -', shell=True, check=True, cwd=from_path) else: subprocess.run( - f'cat {from_path} | docker exec -i {self.container_name} sh -c "cat > {to_path}"', + f'cat {from_path} | docker exec -i {self.name} sh -c "cat > {to_path}"', shell=True, check=True) @@ -92,7 +92,7 @@ class DockerContainer: to_path.mkdir(parents=True, exist_ok=True) subprocess.run( - f'docker exec -i {self.container_name} tar -cC {from_path} -f - . | tar -xf -', + f'docker exec -i {self.name} tar -cC {from_path} -f - . | tar -xf -', shell=True, check=True, cwd=to_path @@ -125,23 +125,23 @@ class DockerContainer: # Finally, the remote shell is told to write a footer - this will show # up in the output so we know when to stop reading, and will include # the returncode of `command`. - self.bash_stdin.write(f'''( + self.bash_stdin.write(bytes(f'''( {chdir} env {env_assignments} {command} printf "%04d%s\n" $? {end_of_message} ) - ''') + ''', encoding='utf8', errors='surrogateescape')) self.bash_stdin.flush() if capture_output: - output_io: TextIO = io.StringIO() + output_io: IO[bytes] = io.BytesIO() else: - output_io = sys.stdout + output_io = sys.stdout.buffer while True: line = self.bash_stdout.readline() - if line.endswith(end_of_message+'\n'): + if line.endswith(b'%s\n' % (bytes(end_of_message, encoding='utf8'))): footer_offset = ( len(line) - 1 # newline character @@ -156,12 +156,15 @@ class DockerContainer: else: output_io.write(line) - output = output_io.getvalue() if isinstance(output_io, io.StringIO) else None + if isinstance(output_io, io.BytesIO): + output = str(output_io.getvalue(), encoding='utf8', errors='surrogateescape') + else: + output = '' if returncode != 0: raise subprocess.CalledProcessError(returncode, args, output) - return output if output else '' + return output def get_environment(self) -> Dict[str, str]: return json.loads(self.call([ From c956f118f6400f0d1f21c94557050efc41af72e5 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Fri, 26 Jun 2020 21:37:02 +0100 Subject: [PATCH 13/25] Add unit tests for DockerContainer --- bin/run_tests.py | 2 +- unit_test/conftest.py | 21 ++++++++ unit_test/docker_container_test.py | 85 ++++++++++++++++++++++++++++++ 3 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 unit_test/conftest.py create mode 100644 unit_test/docker_container_test.py diff --git a/bin/run_tests.py b/bin/run_tests.py index 117f8b9f..1a1895cf 100755 --- a/bin/run_tests.py +++ b/bin/run_tests.py @@ -10,7 +10,7 @@ if __name__ == '__main__': os.chdir(Path(__file__).resolve().parents[1]) # run the unit tests - subprocess.check_call([sys.executable, '-m', 'pytest', 'unit_test']) + subprocess.check_call([sys.executable, '-m', 'pytest', 'unit_test', '--runslow']) # run the integration tests subprocess.check_call([sys.executable, '-m', 'pytest', '-x', '--durations', '0', 'test']) diff --git a/unit_test/conftest.py b/unit_test/conftest.py new file mode 100644 index 00000000..e446d0a1 --- /dev/null +++ b/unit_test/conftest.py @@ -0,0 +1,21 @@ +import pytest + + +def pytest_addoption(parser): + parser.addoption( + "--runslow", action="store_true", default=False, help="run slow tests" + ) + + +def pytest_configure(config): + config.addinivalue_line("markers", "slow: mark test as slow to run") + + +def pytest_collection_modifyitems(config, items): + if config.getoption("--runslow"): + # --runslow given in cli: do not skip slow tests + return + skip_slow = pytest.mark.skip(reason="need --runslow option to run") + for item in items: + if "slow" in item.keywords: + item.add_marker(skip_slow) diff --git a/unit_test/docker_container_test.py b/unit_test/docker_container_test.py new file mode 100644 index 00000000..e8ddc7d1 --- /dev/null +++ b/unit_test/docker_container_test.py @@ -0,0 +1,85 @@ +import pytest +from cibuildwheel.docker_container import DockerContainer +import subprocess +import time +import textwrap + +DEFAULT_IMAGE = 'centos:6' + +@pytest.mark.slow +def test_simple(): + with DockerContainer(DEFAULT_IMAGE) as container: + assert container.call(['echo', 'hello'], capture_output=True) == 'hello\n' + +@pytest.mark.slow +def test_no_lf(): + with DockerContainer(DEFAULT_IMAGE) as container: + assert container.call(['printf', 'hello'], capture_output=True) == 'hello' + +@pytest.mark.slow +def test_environment(): + with DockerContainer(DEFAULT_IMAGE) as container: + assert container.call(['sh', '-c', 'echo $TEST_VAR'], env={'TEST_VAR': '1'}, capture_output=True) == '1\n' + +@pytest.mark.slow +def test_container_removed(): + start_time = time.time() + with DockerContainer(DEFAULT_IMAGE) as container: + container.call(['true']) + docker_containers_listing = subprocess.run('docker container ls', shell=True, check=True, capture_output=True, universal_newlines=True).stdout + assert container.name in docker_containers_listing + old_container_name = container.name + + docker_containers_listing = subprocess.run('docker container ls', shell=True, check=True, capture_output=True, universal_newlines=True).stdout + assert old_container_name not in docker_containers_listing + +@pytest.mark.slow +def test_large_environment(): + # max environment variable size is 128kB + long_env_var_length = 127*1024 + large_environment = { + 'a': '0'*long_env_var_length, + 'b': '0'*long_env_var_length, + 'c': '0'*long_env_var_length, + 'd': '0'*long_env_var_length, + } + + with DockerContainer(DEFAULT_IMAGE) as container: + # check the length of d + assert container.call(['sh', '-c', 'echo ${#d}'], env=large_environment, capture_output=True) == f'{long_env_var_length}\n' + +@pytest.mark.slow +def test_binary_output(): + with DockerContainer(DEFAULT_IMAGE) as container: + # the centos image only has python 2.6, so the below embedded snippets + # are in python2 + + # check that we can pass though arbitrary binary data without erroring + container.call(['/usr/bin/python2', '-c', textwrap.dedent(''' + import sys + sys.stdout.write(''.join(chr(n) for n in range(0, 256))) + ''')]) + + # check that we can capture arbitrary binary data + output = container.call(['/usr/bin/python2', '-c', textwrap.dedent(''' + import sys + sys.stdout.write(''.join(chr(n % 256) for n in range(0, 512))) + ''')], capture_output=True) + + data = bytes(output, encoding='utf8', errors='surrogateescape') + + for i in range(0, 512): + assert data[i] == i % 256 + + # check that environment variables can carry binary data, except null characters + # (https://www.gnu.org/software/libc/manual/html_node/Environment-Variables.html) + binary_data = bytes(n for n in range(1, 256)) + binary_data_string = str(binary_data, encoding='utf8', errors='surrogateescape') + output = container.call( + ['python2', '-c', 'import os, sys; sys.stdout.write(os.environ["TEST_VAR"])'], + env={'TEST_VAR': binary_data_string}, + capture_output=True, + ) + assert output == binary_data_string + + From ad871f886a550ca7cd2ff60afc07fd7efef627bd Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Fri, 26 Jun 2020 21:41:00 +0100 Subject: [PATCH 14/25] Fix style --- cibuildwheel/docker_container.py | 3 +-- unit_test/docker_container_test.py | 16 ++++++++++------ 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/cibuildwheel/docker_container.py b/cibuildwheel/docker_container.py index 3131c782..837c06c1 100644 --- a/cibuildwheel/docker_container.py +++ b/cibuildwheel/docker_container.py @@ -1,4 +1,3 @@ -import codecs import io import json import shlex @@ -7,7 +6,7 @@ import sys import uuid from os import PathLike from pathlib import Path, PurePath -from typing import IO, Any, Dict, List, Optional, Sequence, TextIO, Union, cast +from typing import IO, Dict, List, Optional, Sequence, Union class DockerContainer: diff --git a/unit_test/docker_container_test.py b/unit_test/docker_container_test.py index e8ddc7d1..d66155ef 100644 --- a/unit_test/docker_container_test.py +++ b/unit_test/docker_container_test.py @@ -1,29 +1,33 @@ -import pytest -from cibuildwheel.docker_container import DockerContainer import subprocess -import time import textwrap +import pytest + +from cibuildwheel.docker_container import DockerContainer + DEFAULT_IMAGE = 'centos:6' + @pytest.mark.slow def test_simple(): with DockerContainer(DEFAULT_IMAGE) as container: assert container.call(['echo', 'hello'], capture_output=True) == 'hello\n' + @pytest.mark.slow def test_no_lf(): with DockerContainer(DEFAULT_IMAGE) as container: assert container.call(['printf', 'hello'], capture_output=True) == 'hello' + @pytest.mark.slow def test_environment(): with DockerContainer(DEFAULT_IMAGE) as container: assert container.call(['sh', '-c', 'echo $TEST_VAR'], env={'TEST_VAR': '1'}, capture_output=True) == '1\n' + @pytest.mark.slow def test_container_removed(): - start_time = time.time() with DockerContainer(DEFAULT_IMAGE) as container: container.call(['true']) docker_containers_listing = subprocess.run('docker container ls', shell=True, check=True, capture_output=True, universal_newlines=True).stdout @@ -33,6 +37,7 @@ def test_container_removed(): docker_containers_listing = subprocess.run('docker container ls', shell=True, check=True, capture_output=True, universal_newlines=True).stdout assert old_container_name not in docker_containers_listing + @pytest.mark.slow def test_large_environment(): # max environment variable size is 128kB @@ -48,6 +53,7 @@ def test_large_environment(): # check the length of d assert container.call(['sh', '-c', 'echo ${#d}'], env=large_environment, capture_output=True) == f'{long_env_var_length}\n' + @pytest.mark.slow def test_binary_output(): with DockerContainer(DEFAULT_IMAGE) as container: @@ -81,5 +87,3 @@ def test_binary_output(): capture_output=True, ) assert output == binary_data_string - - From 91c3e90017db468e12d04497666202120f0d44c2 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Fri, 26 Jun 2020 21:47:42 +0100 Subject: [PATCH 15/25] Fix Python 3.6 compatibility --- unit_test/docker_container_test.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/unit_test/docker_container_test.py b/unit_test/docker_container_test.py index d66155ef..7e124d38 100644 --- a/unit_test/docker_container_test.py +++ b/unit_test/docker_container_test.py @@ -29,12 +29,13 @@ def test_environment(): @pytest.mark.slow def test_container_removed(): with DockerContainer(DEFAULT_IMAGE) as container: + # call a command to ensure it has started container.call(['true']) - docker_containers_listing = subprocess.run('docker container ls', shell=True, check=True, capture_output=True, universal_newlines=True).stdout + docker_containers_listing = subprocess.run('docker container ls', shell=True, check=True, stdout=subprocess.PIPE, universal_newlines=True).stdout assert container.name in docker_containers_listing old_container_name = container.name - docker_containers_listing = subprocess.run('docker container ls', shell=True, check=True, capture_output=True, universal_newlines=True).stdout + docker_containers_listing = subprocess.run('docker container ls', shell=True, check=True, stdout=subprocess.PIPE, universal_newlines=True).stdout assert old_container_name not in docker_containers_listing From faf6a7637583ff7c383dbff0a2863e155013777f Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Sun, 28 Jun 2020 10:53:03 +0100 Subject: [PATCH 16/25] Change 'slow' tests to 'docker' tests, only run them on Linux --- bin/run_tests.py | 6 +++++- unit_test/conftest.py | 14 +++++++------- unit_test/docker_container_test.py | 12 ++++++------ 3 files changed, 18 insertions(+), 14 deletions(-) diff --git a/bin/run_tests.py b/bin/run_tests.py index 1a1895cf..17f06aa5 100755 --- a/bin/run_tests.py +++ b/bin/run_tests.py @@ -10,7 +10,11 @@ if __name__ == '__main__': os.chdir(Path(__file__).resolve().parents[1]) # run the unit tests - subprocess.check_call([sys.executable, '-m', 'pytest', 'unit_test', '--runslow']) + unit_test_args = [sys.executable, '-m', 'pytest', 'unit_test'] + # run the docker unit tests only on Linux + if sys.platform.startswith('linux'): + unit_test_args += ['--run-docker'] + subprocess.check_call(unit_test_args) # run the integration tests subprocess.check_call([sys.executable, '-m', 'pytest', '-x', '--durations', '0', 'test']) diff --git a/unit_test/conftest.py b/unit_test/conftest.py index e446d0a1..a54658a9 100644 --- a/unit_test/conftest.py +++ b/unit_test/conftest.py @@ -3,19 +3,19 @@ import pytest def pytest_addoption(parser): parser.addoption( - "--runslow", action="store_true", default=False, help="run slow tests" + "--run-docker", action="store_true", default=False, help="run docker tests" ) def pytest_configure(config): - config.addinivalue_line("markers", "slow: mark test as slow to run") + config.addinivalue_line("markers", "docker: mark test requiring docker to run") def pytest_collection_modifyitems(config, items): - if config.getoption("--runslow"): - # --runslow given in cli: do not skip slow tests + if config.getoption("--run-docker"): + # --run-docker given in cli: do not skip docker tests return - skip_slow = pytest.mark.skip(reason="need --runslow option to run") + skip_docker = pytest.mark.skip(reason="need --run-docker option to run") for item in items: - if "slow" in item.keywords: - item.add_marker(skip_slow) + if "docker" in item.keywords: + item.add_marker(skip_docker) diff --git a/unit_test/docker_container_test.py b/unit_test/docker_container_test.py index 7e124d38..cba0fb50 100644 --- a/unit_test/docker_container_test.py +++ b/unit_test/docker_container_test.py @@ -8,25 +8,25 @@ from cibuildwheel.docker_container import DockerContainer DEFAULT_IMAGE = 'centos:6' -@pytest.mark.slow +@pytest.mark.docker def test_simple(): with DockerContainer(DEFAULT_IMAGE) as container: assert container.call(['echo', 'hello'], capture_output=True) == 'hello\n' -@pytest.mark.slow +@pytest.mark.docker def test_no_lf(): with DockerContainer(DEFAULT_IMAGE) as container: assert container.call(['printf', 'hello'], capture_output=True) == 'hello' -@pytest.mark.slow +@pytest.mark.docker def test_environment(): with DockerContainer(DEFAULT_IMAGE) as container: assert container.call(['sh', '-c', 'echo $TEST_VAR'], env={'TEST_VAR': '1'}, capture_output=True) == '1\n' -@pytest.mark.slow +@pytest.mark.docker def test_container_removed(): with DockerContainer(DEFAULT_IMAGE) as container: # call a command to ensure it has started @@ -39,7 +39,7 @@ def test_container_removed(): assert old_container_name not in docker_containers_listing -@pytest.mark.slow +@pytest.mark.docker def test_large_environment(): # max environment variable size is 128kB long_env_var_length = 127*1024 @@ -55,7 +55,7 @@ def test_large_environment(): assert container.call(['sh', '-c', 'echo ${#d}'], env=large_environment, capture_output=True) == f'{long_env_var_length}\n' -@pytest.mark.slow +@pytest.mark.docker def test_binary_output(): with DockerContainer(DEFAULT_IMAGE) as container: # the centos image only has python 2.6, so the below embedded snippets From 27d97daf13b6aa04301640d225845344aad17fac Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Sun, 28 Jun 2020 11:33:22 +0100 Subject: [PATCH 17/25] Don't use redirection syntax in Windows environment. Fix absolute path use for Mac --- test/test_environment.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/test_environment.py b/test/test_environment.py index 7e5f8d10..124867e1 100644 --- a/test/test_environment.py +++ b/test/test_environment.py @@ -55,8 +55,8 @@ def test_overridden_path(tmp_path, capfd): # mess up PATH, somehow with pytest.raises(subprocess.CalledProcessError): utils.cibuildwheel_run(project_dir, output_dir=output_dir, add_env={ - 'CIBW_ENVIRONMENT': '''SOMETHING="$(mkdir -p /new_path ; touch /new_path/python ; chmod +x /new_path/python)" PATH="/new_path:$PATH"''', - 'CIBW_ENVIRONMENT_WINDOWS': '''SOMETHING="$(mkdir new_path && type nul > new_path/python.exe)" PATH="$CD\\new_path;$PATH"''', + 'CIBW_ENVIRONMENT': '''SOMETHING="$(mkdir -p new_path ; touch new_path/python ; chmod +x new_path/python)" PATH="$(pwd)/new_path:$PATH"''', + 'CIBW_ENVIRONMENT_WINDOWS': '''SOMETHING="$(mkdir new_path ; fsutil file createnew new_path/python.exe 0)" PATH="$CD\\new_path;$PATH"''', }) assert len(os.listdir(output_dir)) == 0 From e31c24d3bb4db9dde2be2309e11cf5d5ee07d8b4 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Sun, 28 Jun 2020 21:50:26 +0100 Subject: [PATCH 18/25] Fix overridden PATH test, by ensuring bash command substitution rules --- cibuildwheel/bashlex_eval.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cibuildwheel/bashlex_eval.py b/cibuildwheel/bashlex_eval.py index f4f0641d..b5e478c8 100644 --- a/cibuildwheel/bashlex_eval.py +++ b/cibuildwheel/bashlex_eval.py @@ -41,7 +41,9 @@ def evaluate_node(node: bashlex.ast.node, context: NodeExecutionContext) -> str: if node.kind == 'word': return evaluate_word_node(node, context=context) elif node.kind == 'commandsubstitution': - return evaluate_command_node(node.command, context=context) + node_result = evaluate_command_node(node.command, context=context) + # bash removes training newlines in command substitution + return node_result.rstrip() elif node.kind == 'parameter': return evaluate_parameter_node(node, context=context) else: From b12a560e87863244c7297a1c2d95a703662b062d Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Wed, 1 Jul 2020 13:22:24 +0100 Subject: [PATCH 19/25] Add docker test images for other architectures --- unit_test/docker_container_test.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/unit_test/docker_container_test.py b/unit_test/docker_container_test.py index cba0fb50..6318d8a6 100644 --- a/unit_test/docker_container_test.py +++ b/unit_test/docker_container_test.py @@ -1,3 +1,4 @@ +import platform import subprocess import textwrap @@ -5,7 +6,15 @@ import pytest from cibuildwheel.docker_container import DockerContainer -DEFAULT_IMAGE = 'centos:6' +pm = platform.machine() +if pm == "x86_64": + DEFAULT_IMAGE = 'centos:7' +elif pm == "aarch64": + DEFAULT_IMAGE = 'arm64v8/centos:7' +elif pm == "ppc64le": + DEFAULT_IMAGE = 'ppc64le/centos:7' +elif pm == "s390x": + DEFAULT_IMAGE = 's390x/clefos:7' @pytest.mark.docker From cc913ee7cf0c9cd91f9968a8ad10bc28fb3c2549 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Sun, 5 Jul 2020 17:27:29 +0100 Subject: [PATCH 20/25] Use a different method for the Windows test --- test/test_environment.py | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/test/test_environment.py b/test/test_environment.py index 124867e1..03bf3bd2 100644 --- a/test/test_environment.py +++ b/test/test_environment.py @@ -54,10 +54,19 @@ def test_overridden_path(tmp_path, capfd): # mess up PATH, somehow with pytest.raises(subprocess.CalledProcessError): - utils.cibuildwheel_run(project_dir, output_dir=output_dir, add_env={ - 'CIBW_ENVIRONMENT': '''SOMETHING="$(mkdir -p new_path ; touch new_path/python ; chmod +x new_path/python)" PATH="$(pwd)/new_path:$PATH"''', - 'CIBW_ENVIRONMENT_WINDOWS': '''SOMETHING="$(mkdir new_path ; fsutil file createnew new_path/python.exe 0)" PATH="$CD\\new_path;$PATH"''', - }) + if utils.platform == 'linux': + utils.cibuildwheel_run(project_dir, output_dir=output_dir, add_env={ + 'CIBW_BEFORE_ALL': 'mkdir new_path && touch new_path/python && chmod +x new_path/python', + 'CIBW_ENVIRONMENT': '''PATH="$(pwd)/new_path:$PATH"''', + }) + else: + new_path = tmp_path / 'another_bin' + new_path.mkdir() + (new_path / 'python').touch(mode=0o777) + + utils.cibuildwheel_run(project_dir, output_dir=output_dir, add_env={ + 'CIBW_ENVIRONMENT': f'''PATH="{new_path}{os.pathsep}$PATH"''', + }) assert len(os.listdir(output_dir)) == 0 captured = capfd.readouterr() From 833a0692079aa18ee781403d3ab94122a1ef8fe1 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Sun, 5 Jul 2020 17:39:40 +0100 Subject: [PATCH 21/25] Pass PATH during command substitution test --- unit_test/environment_test.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/unit_test/environment_test.py b/unit_test/environment_test.py index 850233e3..4e10e07c 100644 --- a/unit_test/environment_test.py +++ b/unit_test/environment_test.py @@ -97,8 +97,9 @@ def test_no_vars_pass_through(): def test_operators_inside_eval(): - environment_recipe = parse_environment('SOMETHING="$(echo a ; echo b ; echo c)"') + environment_recipe = parse_environment('SOMETHING="$(echo a; echo b; echo c)"') - environment_dict = environment_recipe.as_dictionary({}) + # pass the existing process env so PATH is available + environment_dict = environment_recipe.as_dictionary(os.environ.copy()) - assert environment_dict == {'SOMETHING': 'a\nb\nc'} + assert environment_dict.get('SOMETHING') == 'a\nb\nc' From 9e35d65de438d0b42e9218a9b7a7ca4633388eb0 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Wed, 8 Jul 2020 22:56:33 +0100 Subject: [PATCH 22/25] Add file-based unit tests --- cibuildwheel/docker_container.py | 4 +++ unit_test/docker_container_test.py | 43 ++++++++++++++++++++++++------ 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/cibuildwheel/docker_container.py b/cibuildwheel/docker_container.py index 837c06c1..8a119136 100644 --- a/cibuildwheel/docker_container.py +++ b/cibuildwheel/docker_container.py @@ -58,6 +58,10 @@ class DockerContainer: assert self.process.stdin and self.process.stdout self.bash_stdin = self.process.stdin self.bash_stdout = self.process.stdout + + # run a noop command to block until the container is responding + self.call(['/bin/true']) + return self def __exit__(self, exc_type, exc_val, exc_tb): diff --git a/unit_test/docker_container_test.py b/unit_test/docker_container_test.py index 6318d8a6..689ff19b 100644 --- a/unit_test/docker_container_test.py +++ b/unit_test/docker_container_test.py @@ -1,20 +1,24 @@ +from pathlib import Path, PurePath import platform import subprocess import textwrap +from uuid import uuid4 import pytest from cibuildwheel.docker_container import DockerContainer +# for these tests we use manylinux2014 images, because they're available on +# multi architectures and include python3.8 pm = platform.machine() if pm == "x86_64": - DEFAULT_IMAGE = 'centos:7' + DEFAULT_IMAGE = 'quay.io/pypa/manylinux2014_x86_64:2020-05-17-2f8ac3b' elif pm == "aarch64": - DEFAULT_IMAGE = 'arm64v8/centos:7' + DEFAULT_IMAGE = 'quay.io/pypa/manylinux2014_aarch64:2020-05-17-2f8ac3b' elif pm == "ppc64le": - DEFAULT_IMAGE = 'ppc64le/centos:7' + DEFAULT_IMAGE = 'quay.io/pypa/manylinux2014_ppc64le:2020-05-17-2f8ac3b' elif pm == "s390x": - DEFAULT_IMAGE = 's390x/clefos:7' + DEFAULT_IMAGE = 'quay.io/pypa/manylinux2014_s390x:2020-05-17-2f8ac3b' @pytest.mark.docker @@ -38,8 +42,6 @@ def test_environment(): @pytest.mark.docker def test_container_removed(): with DockerContainer(DEFAULT_IMAGE) as container: - # call a command to ensure it has started - container.call(['true']) docker_containers_listing = subprocess.run('docker container ls', shell=True, check=True, stdout=subprocess.PIPE, universal_newlines=True).stdout assert container.name in docker_containers_listing old_container_name = container.name @@ -67,8 +69,7 @@ def test_large_environment(): @pytest.mark.docker def test_binary_output(): with DockerContainer(DEFAULT_IMAGE) as container: - # the centos image only has python 2.6, so the below embedded snippets - # are in python2 + # note: the below embedded snippets are in python2 # check that we can pass though arbitrary binary data without erroring container.call(['/usr/bin/python2', '-c', textwrap.dedent(''' @@ -97,3 +98,29 @@ def test_binary_output(): capture_output=True, ) assert output == binary_data_string + + +@pytest.mark.docker +def test_file_operations(tmp_path: Path): + with DockerContainer(DEFAULT_IMAGE) as container: + # test copying a file in + test_binary_data = uuid4().bytes + uuid4().bytes + uuid4().bytes + uuid4().bytes + + original_test_file = tmp_path / 'test.dat' + original_test_file.write_bytes(test_binary_data) + + dst_file = PurePath('/tmp/test.dat') + + container.copy_into(original_test_file, dst_file) + + output = container.call(['cat', dst_file], capture_output=True) + assert test_binary_data == bytes(output, encoding='utf8', errors='surrogateescape') + + # test copying a dir in + test_dir = tmp_path / 'test_dir' + new_test_file = tmp_path / 'test-new.dat' + container.copy_out(dst_file, new_test_file) + + assert original_test_file.read_bytes() == new_test_file.read_bytes() + + assert container.glob(PurePath('/tmp/*.dat')) == [dst_file] From 4c0af9f152df7e53c71a4203631ffa06f4c73e01 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Fri, 10 Jul 2020 12:46:32 +0100 Subject: [PATCH 23/25] Finish file/dir tests --- cibuildwheel/docker_container.py | 10 +++++---- unit_test/docker_container_test.py | 34 ++++++++++++++++++++++++------ 2 files changed, 34 insertions(+), 10 deletions(-) diff --git a/cibuildwheel/docker_container.py b/cibuildwheel/docker_container.py index 8a119136..f5d6e965 100644 --- a/cibuildwheel/docker_container.py +++ b/cibuildwheel/docker_container.py @@ -6,7 +6,7 @@ import sys import uuid from os import PathLike from pathlib import Path, PurePath -from typing import IO, Dict, List, Optional, Sequence, Union +from typing import Any, IO, Dict, List, Optional, Sequence, Union class DockerContainer: @@ -77,16 +77,17 @@ class DockerContainer: # a container is running and the host filesystem is # mounted. https://github.com/moby/moby/issues/38995 # Use `docker exec` instead. + quote = lambda p: shlex.quote(str(p)) if from_path.is_dir(): self.call(['mkdir', '-p', to_path]) subprocess.run( - f'tar cf - . | docker exec -i {self.name} tar -xC {to_path} -f -', + f'tar cf - . | docker exec -i {self.name} tar -xC {quote(to_path)} -f -', shell=True, check=True, cwd=from_path) else: subprocess.run( - f'cat {from_path} | docker exec -i {self.name} sh -c "cat > {to_path}"', + f'cat {quote(from_path)} | docker exec -i {self.name} sh -c "cat > {quote(to_path)}"', shell=True, check=True) @@ -94,8 +95,9 @@ class DockerContainer: # note: we assume from_path is a dir to_path.mkdir(parents=True, exist_ok=True) + quote = lambda p: shlex.quote(str(p)) subprocess.run( - f'docker exec -i {self.name} tar -cC {from_path} -f - . | tar -xf -', + f'docker exec -i {self.name} tar -cC {quote(from_path)} -f - . | tar -xf -', shell=True, check=True, cwd=to_path diff --git a/unit_test/docker_container_test.py b/unit_test/docker_container_test.py index 689ff19b..c3b3c951 100644 --- a/unit_test/docker_container_test.py +++ b/unit_test/docker_container_test.py @@ -1,5 +1,7 @@ from pathlib import Path, PurePath import platform +import random +import shutil import subprocess import textwrap from uuid import uuid4 @@ -104,8 +106,7 @@ def test_binary_output(): def test_file_operations(tmp_path: Path): with DockerContainer(DEFAULT_IMAGE) as container: # test copying a file in - test_binary_data = uuid4().bytes + uuid4().bytes + uuid4().bytes + uuid4().bytes - + test_binary_data = bytes(random.randrange(256) for _ in range(1000)) original_test_file = tmp_path / 'test.dat' original_test_file.write_bytes(test_binary_data) @@ -116,11 +117,32 @@ def test_file_operations(tmp_path: Path): output = container.call(['cat', dst_file], capture_output=True) assert test_binary_data == bytes(output, encoding='utf8', errors='surrogateescape') +@pytest.mark.docker +def test_dir_operations(tmp_path: Path): + with DockerContainer(DEFAULT_IMAGE) as container: + test_binary_data = bytes(random.randrange(256) for _ in range(1000)) + original_test_file = tmp_path / 'test.dat' + original_test_file.write_bytes(test_binary_data) + # test copying a dir in test_dir = tmp_path / 'test_dir' - new_test_file = tmp_path / 'test-new.dat' - container.copy_out(dst_file, new_test_file) + test_dir.mkdir() + test_file = test_dir / 'test.dat' + shutil.copyfile(original_test_file, test_file) - assert original_test_file.read_bytes() == new_test_file.read_bytes() + dst_dir = PurePath('/tmp/test_dir') + dst_file = dst_dir / 'test.dat' + container.copy_into(test_dir, dst_dir) + + output = container.call(['cat', dst_file], capture_output=True) + assert test_binary_data == bytes(output, encoding='utf8', errors='surrogateescape') + + # test glob + assert container.glob(dst_dir / '*.dat') == [dst_file] + + # test copy dir out + new_test_dir = tmp_path / 'test_dir_new' + container.copy_out(dst_dir, new_test_dir) + + assert test_binary_data == (new_test_dir / 'test.dat').read_bytes() - assert container.glob(PurePath('/tmp/*.dat')) == [dst_file] From 44e31988805c30b1e22f43552094940dd2e30a86 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Fri, 10 Jul 2020 12:57:05 +0100 Subject: [PATCH 24/25] Convert glob to take a Path and a str pattern --- cibuildwheel/docker_container.py | 7 +++++-- cibuildwheel/linux.py | 4 ++-- unit_test/docker_container_test.py | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/cibuildwheel/docker_container.py b/cibuildwheel/docker_container.py index f5d6e965..6bd5cdf3 100644 --- a/cibuildwheel/docker_container.py +++ b/cibuildwheel/docker_container.py @@ -1,5 +1,6 @@ import io import json +import os import shlex import subprocess import sys @@ -103,11 +104,13 @@ class DockerContainer: cwd=to_path ) - def glob(self, pattern: PurePath) -> List[PurePath]: + def glob(self, path: PurePath, pattern: str) -> List[PurePath]: + glob_pattern = os.path.join(str(path), pattern) + path_strs = json.loads(self.call([ self.UTILITY_PYTHON, '-c', - f'import sys, json, glob; json.dump(glob.glob({str(pattern)!r}), sys.stdout)' + f'import sys, json, glob; json.dump(glob.glob({glob_pattern!r}), sys.stdout)' ], capture_output=True)) return [PurePath(p) for p in path_strs] diff --git a/cibuildwheel/linux.py b/cibuildwheel/linux.py index 60de4743..e4607f0c 100644 --- a/cibuildwheel/linux.py +++ b/cibuildwheel/linux.py @@ -167,7 +167,7 @@ def build(options: BuildOptions) -> None: *get_build_verbosity_extra_flags(options.build_verbosity) ], env=env) - built_wheel = docker.glob(built_wheel_dir / '*.whl')[0] + built_wheel = docker.glob(built_wheel_dir, '*.whl')[0] repaired_wheel_dir = temp_dir / 'repaired_wheel' docker.call(['rm', '-rf', repaired_wheel_dir]) @@ -179,7 +179,7 @@ def build(options: BuildOptions) -> None: repair_command_prepared = prepare_command(options.repair_command, wheel=built_wheel, dest_dir=repaired_wheel_dir) docker.call(['sh', '-c', repair_command_prepared], env=env) - repaired_wheels = docker.glob(repaired_wheel_dir / '*.whl') + repaired_wheels = docker.glob(repaired_wheel_dir, '*.whl') if options.test_command: # set up a virtual environment to install and test from, to make sure diff --git a/unit_test/docker_container_test.py b/unit_test/docker_container_test.py index c3b3c951..6fe9f957 100644 --- a/unit_test/docker_container_test.py +++ b/unit_test/docker_container_test.py @@ -138,7 +138,7 @@ def test_dir_operations(tmp_path: Path): assert test_binary_data == bytes(output, encoding='utf8', errors='surrogateescape') # test glob - assert container.glob(dst_dir / '*.dat') == [dst_file] + assert container.glob(dst_dir, '*.dat') == [dst_file] # test copy dir out new_test_dir = tmp_path / 'test_dir_new' From 5842a7592727e0b4f58adb563629f2bace786976 Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Fri, 10 Jul 2020 13:04:50 +0100 Subject: [PATCH 25/25] Flake8 fixes --- cibuildwheel/docker_container.py | 15 +++++++++------ unit_test/docker_container_test.py | 5 ++--- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/cibuildwheel/docker_container.py b/cibuildwheel/docker_container.py index 6bd5cdf3..41b2f42f 100644 --- a/cibuildwheel/docker_container.py +++ b/cibuildwheel/docker_container.py @@ -7,7 +7,7 @@ import sys import uuid from os import PathLike from pathlib import Path, PurePath -from typing import Any, IO, Dict, List, Optional, Sequence, Union +from typing import IO, Dict, List, Optional, Sequence, Union class DockerContainer: @@ -78,17 +78,17 @@ class DockerContainer: # a container is running and the host filesystem is # mounted. https://github.com/moby/moby/issues/38995 # Use `docker exec` instead. - quote = lambda p: shlex.quote(str(p)) + if from_path.is_dir(): self.call(['mkdir', '-p', to_path]) subprocess.run( - f'tar cf - . | docker exec -i {self.name} tar -xC {quote(to_path)} -f -', + f'tar cf - . | docker exec -i {self.name} tar -xC {shell_quote(to_path)} -f -', shell=True, check=True, cwd=from_path) else: subprocess.run( - f'cat {quote(from_path)} | docker exec -i {self.name} sh -c "cat > {quote(to_path)}"', + f'cat {shell_quote(from_path)} | docker exec -i {self.name} sh -c "cat > {shell_quote(to_path)}"', shell=True, check=True) @@ -96,9 +96,8 @@ class DockerContainer: # note: we assume from_path is a dir to_path.mkdir(parents=True, exist_ok=True) - quote = lambda p: shlex.quote(str(p)) subprocess.run( - f'docker exec -i {self.name} tar -cC {quote(from_path)} -f - . | tar -xf -', + f'docker exec -i {self.name} tar -cC {shell_quote(from_path)} -f - . | tar -xf -', shell=True, check=True, cwd=to_path @@ -184,3 +183,7 @@ class DockerContainer: def environment_executor(self, command: str, environment: Dict[str, str]) -> str: # used as an EnvironmentExecutor to evaluate commands and capture output return self.call(shlex.split(command), env=environment) + + +def shell_quote(path: PurePath) -> str: + return shlex.quote(str(path)) diff --git a/unit_test/docker_container_test.py b/unit_test/docker_container_test.py index 6fe9f957..fed06174 100644 --- a/unit_test/docker_container_test.py +++ b/unit_test/docker_container_test.py @@ -1,10 +1,9 @@ -from pathlib import Path, PurePath import platform import random import shutil import subprocess import textwrap -from uuid import uuid4 +from pathlib import Path, PurePath import pytest @@ -117,6 +116,7 @@ def test_file_operations(tmp_path: Path): output = container.call(['cat', dst_file], capture_output=True) assert test_binary_data == bytes(output, encoding='utf8', errors='surrogateescape') + @pytest.mark.docker def test_dir_operations(tmp_path: Path): with DockerContainer(DEFAULT_IMAGE) as container: @@ -145,4 +145,3 @@ def test_dir_operations(tmp_path: Path): container.copy_out(dst_dir, new_test_dir) assert test_binary_data == (new_test_dir / 'test.dat').read_bytes() -