From 7a27ae941ce8e4e1113cc9e7392b807d7ea4efdf Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Sun, 5 Jul 2020 23:09:02 -0400 Subject: [PATCH 01/11] fix: Support environment markers in PEP 518 workaround --- cibuildwheel/windows.py | 4 +++- test/test_pep518.py | 9 +++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index 7a694797..1996d061 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -8,6 +8,7 @@ from pathlib import Path from typing import Dict, List, NamedTuple, Optional, Sequence, Union from zipfile import ZipFile import toml +import shlex from .environment import ParsedEnvironment from .util import (BuildOptions, BuildSelector, download, @@ -172,7 +173,8 @@ def pep_518_cp35_workaround(package_dir: Path, env: Dict[str, str]) -> None: else [] ) if requirements: - shell(['pip', 'install'] + requirements, env=env) + escaped_requirements = [shlex.quote(s) for s in requirements] + shell(['pip', 'install'] + escaped_requirements, env=env) def build(options: BuildOptions) -> None: diff --git a/test/test_pep518.py b/test/test_pep518.py index 7d5f5081..2054616b 100644 --- a/test/test_pep518.py +++ b/test/test_pep518.py @@ -6,8 +6,12 @@ basic_project = test_projects.new_c_project( setup_py_add=textwrap.dedent( """ # Will fail if PEP 518 does work + import sys import requests - assert requests.__version__ == "2.23.0", "Requests found but wrong version ({0})".format(requests.__version__) + if sys.version_info < (3, 6, 0): + assert requests.__version__ == "2.22.0", "Requests found but wrong version ({0})".format(requests.__version__) + else: + assert requests.__version__ == "2.23.0", "Requests found but wrong version ({0})".format(requests.__version__) # Just making sure environment is still set import os @@ -24,7 +28,8 @@ basic_project.files[ requires = [ "setuptools>=42", "wheel", - "requests==2.23.0" + "requests==2.22.0; python_version<'3.6'", + "requests==2.23.0; python_version>='3.6'" ] build-backend = "setuptools.build_meta" From c36be04ee8285fd25c759fc0dad47109cf673c16 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Sun, 5 Jul 2020 23:51:28 -0400 Subject: [PATCH 02/11] fix: Use repr --- cibuildwheel/windows.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index 1996d061..dfbfc368 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -8,7 +8,6 @@ from pathlib import Path from typing import Dict, List, NamedTuple, Optional, Sequence, Union from zipfile import ZipFile import toml -import shlex from .environment import ParsedEnvironment from .util import (BuildOptions, BuildSelector, download, @@ -173,7 +172,7 @@ def pep_518_cp35_workaround(package_dir: Path, env: Dict[str, str]) -> None: else [] ) if requirements: - escaped_requirements = [shlex.quote(s) for s in requirements] + escaped_requirements = [f'{s!r}' for s in requirements] shell(['pip', 'install'] + escaped_requirements, env=env) From fc24230cb4269c264cbfadaba2692eee9aca5ccd Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 6 Jul 2020 01:31:47 -0400 Subject: [PATCH 03/11] Try original --- cibuildwheel/windows.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index dfbfc368..7a694797 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -172,8 +172,7 @@ def pep_518_cp35_workaround(package_dir: Path, env: Dict[str, str]) -> None: else [] ) if requirements: - escaped_requirements = [f'{s!r}' for s in requirements] - shell(['pip', 'install'] + escaped_requirements, env=env) + shell(['pip', 'install'] + requirements, env=env) def build(options: BuildOptions) -> None: From c68be58e7845b6918726877735f842897f3b4d19 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 6 Jul 2020 08:55:36 -0400 Subject: [PATCH 04/11] fix: Try simple quoting --- cibuildwheel/windows.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index 7a694797..e7dab1c4 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -172,7 +172,8 @@ def pep_518_cp35_workaround(package_dir: Path, env: Dict[str, str]) -> None: else [] ) if requirements: - shell(['pip', 'install'] + requirements, env=env) + escaped_requirements = [f'"{s}"' for s in requirements] + shell(['pip', 'install'] + escaped_requirements, env=env) def build(options: BuildOptions) -> None: From 84661945763ca50c7eb383282cbdccf10a2c7506 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 6 Jul 2020 10:42:51 -0400 Subject: [PATCH 05/11] fix: Try removing shell --- cibuildwheel/windows.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index e7dab1c4..c94515e7 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -22,7 +22,7 @@ def shell(args: Sequence[Union[str, PathLike]], env: Optional[Dict[str, str]] = cwd: Optional[str] = None) -> int: command = ' '.join(str(a) for a in args) print(f'+ {command}') - return subprocess.check_call(command, env=env, cwd=cwd, shell=True) + return subprocess.check_call(list(str(a) for a in args), env=env, cwd=cwd) def get_nuget_args(version: str, arch: str) -> List[str]: @@ -172,8 +172,7 @@ def pep_518_cp35_workaround(package_dir: Path, env: Dict[str, str]) -> None: else [] ) if requirements: - escaped_requirements = [f'"{s}"' for s in requirements] - shell(['pip', 'install'] + escaped_requirements, env=env) + shell(['pip', 'install'] + requirements, env=env) def build(options: BuildOptions) -> None: From 317397b39adfc67e1e3d15d3faaade46f0925127 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 6 Jul 2020 13:18:50 -0400 Subject: [PATCH 06/11] fix: Try resolving --- cibuildwheel/windows.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index c94515e7..ad844844 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -18,11 +18,18 @@ IS_RUNNING_ON_AZURE = Path('C:\\hostedtoolcache').exists() IS_RUNNING_ON_TRAVIS = os.environ.get('TRAVIS_OS_NAME') == 'windows' +def normalize(x: [Union[str, PathLike]], cwd: Optional[PathLike]) -> str: + if hasattr(x, 'resolve'): + return str((cwd / x if cwd is not None else x).resolve()) + else: + return x + + def shell(args: Sequence[Union[str, PathLike]], env: Optional[Dict[str, str]] = None, cwd: Optional[str] = None) -> int: - command = ' '.join(str(a) for a in args) + command = ' '.join(normalize(a, cwd) for a in args) print(f'+ {command}') - return subprocess.check_call(list(str(a) for a in args), env=env, cwd=cwd) + return subprocess.check_call(list(normalize(a, cwd) for a in args), env=env, cwd=cwd) def get_nuget_args(version: str, arch: str) -> List[str]: From 53ba9bd0d08253eb8c804db6cfd947b4696a9e48 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Mon, 6 Jul 2020 18:11:29 -0400 Subject: [PATCH 07/11] Try adding PATH to env --- cibuildwheel/windows.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index ad844844..6c19e4dd 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -1,3 +1,4 @@ +import copy import os import shutil import subprocess @@ -29,6 +30,9 @@ def shell(args: Sequence[Union[str, PathLike]], env: Optional[Dict[str, str]] = cwd: Optional[str] = None) -> int: command = ' '.join(normalize(a, cwd) for a in args) print(f'+ {command}') + if env is not None: + env = copy.copy(env) + env["PATH"] = os.environ["PATH"] return subprocess.check_call(list(normalize(a, cwd) for a in args), env=env, cwd=cwd) From 47fe71efb4b7f154c7385275f6541c3c91c4b76e Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Tue, 7 Jul 2020 23:09:40 -0400 Subject: [PATCH 08/11] Try just converting to list --- cibuildwheel/windows.py | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index 6c19e4dd..4bc5a973 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -1,4 +1,3 @@ -import copy import os import shutil import subprocess @@ -19,21 +18,11 @@ IS_RUNNING_ON_AZURE = Path('C:\\hostedtoolcache').exists() IS_RUNNING_ON_TRAVIS = os.environ.get('TRAVIS_OS_NAME') == 'windows' -def normalize(x: [Union[str, PathLike]], cwd: Optional[PathLike]) -> str: - if hasattr(x, 'resolve'): - return str((cwd / x if cwd is not None else x).resolve()) - else: - return x - - def shell(args: Sequence[Union[str, PathLike]], env: Optional[Dict[str, str]] = None, cwd: Optional[str] = None) -> int: - command = ' '.join(normalize(a, cwd) for a in args) + command = ' '.join(str(a) for a in args) print(f'+ {command}') - if env is not None: - env = copy.copy(env) - env["PATH"] = os.environ["PATH"] - return subprocess.check_call(list(normalize(a, cwd) for a in args), env=env, cwd=cwd) + return subprocess.check_call([str(s) for a in args], env=env, cwd=cwd, shell=True) def get_nuget_args(version: str, arch: str) -> List[str]: From ac2b39179065eef7da7143b890381c9585433e74 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Tue, 7 Jul 2020 23:33:03 -0400 Subject: [PATCH 09/11] Typo --- cibuildwheel/windows.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index 4bc5a973..e1d416eb 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -22,7 +22,7 @@ def shell(args: Sequence[Union[str, PathLike]], env: Optional[Dict[str, str]] = cwd: Optional[str] = None) -> int: command = ' '.join(str(a) for a in args) print(f'+ {command}') - return subprocess.check_call([str(s) for a in args], env=env, cwd=cwd, shell=True) + return subprocess.check_call([str(a) for a in args], env=env, cwd=cwd, shell=True) def get_nuget_args(version: str, arch: str) -> List[str]: From c521dca40bd24a6a17e0de01c7694eadbc3b3d3e Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Wed, 8 Jul 2020 18:17:55 +0100 Subject: [PATCH 10/11] My own unique attempt on the shell/call debacle --- cibuildwheel/windows.py | 52 +++++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index e1d416eb..1baf66f5 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -18,11 +18,17 @@ IS_RUNNING_ON_AZURE = Path('C:\\hostedtoolcache').exists() IS_RUNNING_ON_TRAVIS = os.environ.get('TRAVIS_OS_NAME') == 'windows' -def shell(args: Sequence[Union[str, PathLike]], env: Optional[Dict[str, str]] = None, - cwd: Optional[str] = None) -> int: - command = ' '.join(str(a) for a in args) +def call(args: Sequence[Union[str, PathLike]], env: Optional[Dict[str, str]] = None, + cwd: Optional[str] = None) -> None: + print('+ ' + ' '.join(str(a) for a in args)) + # we use shell=True here, even though we don't need a shell due to a bug + # https://bugs.python.org/issue8557 + subprocess.check_call([str(a) for a in args], env=env, cwd=cwd, shell=True) + + +def shell(command: str, env: Optional[Dict[str, str]] = None, cwd: Optional[str] = None) -> None: print(f'+ {command}') - return subprocess.check_call([str(a) for a in args], env=env, cwd=cwd, shell=True) + subprocess.check_call(command, env=env, cwd=cwd, shell=True) def get_nuget_args(version: str, arch: str) -> List[str]: @@ -76,7 +82,7 @@ def extract_zip(zip_src: Path, dest: Path) -> None: def install_cpython(version: str, arch: str, nuget: Path) -> Path: nuget_args = get_nuget_args(version, arch) installation_path = Path(nuget_args[-1]) / (nuget_args[0] + '.' + version) / 'tools' - shell([nuget, 'install', *nuget_args]) + call([nuget, 'install', *nuget_args]) return installation_path @@ -125,9 +131,9 @@ def setup_python(python_configuration: PythonConfiguration, dependency_constrain env = environment.as_dictionary(prev_environment=env) # for the logs - check we're running the right version of python - shell(['where', 'python'], env=env) - shell(['python', '--version'], env=env) - shell(['python', '-c', '"import struct; print(struct.calcsize(\'P\') * 8)"'], env=env) + call(['where', 'python'], env=env) + call(['python', '--version'], env=env) + call(['python', '-c', '"import struct; print(struct.calcsize(\'P\') * 8)"'], env=env) where_python = subprocess.check_output(['where', 'python'], env=env, universal_newlines=True).splitlines()[0].strip() if where_python != str(installation_path / 'python.exe'): print("cibuildwheel: python available on PATH doesn't match our installed instance. If you have modified PATH, ensure that you don't overwrite cibuildwheel's entry or insert python above it.", file=sys.stderr) @@ -135,7 +141,7 @@ def setup_python(python_configuration: PythonConfiguration, dependency_constrain # make sure pip is installed if not (installation_path / 'Scripts' / 'pip.exe').exists(): - shell(['python', get_pip_script, *dependency_constraint_flags], env=env, cwd="C:\\cibw") + call(['python', get_pip_script, *dependency_constraint_flags], env=env, cwd="C:\\cibw") assert (installation_path / 'Scripts' / 'pip.exe').exists() where_pip = subprocess.check_output(['where', 'pip'], env=env, universal_newlines=True).splitlines()[0].strip() if where_pip.strip() != str(installation_path / 'Scripts' / 'pip.exe'): @@ -143,9 +149,9 @@ def setup_python(python_configuration: PythonConfiguration, dependency_constrain exit(1) # prepare the Python environment - shell(['python', '-m', 'pip', 'install', '--upgrade', 'pip', *dependency_constraint_flags], env=env) - shell(['pip', '--version'], env=env) - shell(['pip', 'install', '--upgrade', 'setuptools', 'wheel', *dependency_constraint_flags], env=env) + call(['python', '-m', 'pip', 'install', '--upgrade', 'pip', *dependency_constraint_flags], env=env) + call(['pip', '--version'], env=env) + call(['pip', 'install', '--upgrade', 'setuptools', 'wheel', *dependency_constraint_flags], env=env) return env @@ -187,7 +193,7 @@ def build(options: BuildOptions) -> None: if options.before_all: env = options.environment.as_dictionary(prev_environment=os.environ) before_all_prepared = prepare_command(options.before_all, project='.', package=options.package_dir) - shell([before_all_prepared], env=env) + shell(before_all_prepared, env=env) python_configurations = get_python_configurations(options.build_selector) for config in python_configurations: @@ -203,7 +209,7 @@ def build(options: BuildOptions) -> None: # run the before_build command if options.before_build: before_build_prepared = prepare_command(options.before_build, project='.', package=options.package_dir) - shell([before_build_prepared], env=env) + shell(before_build_prepared, env=env) # activate the PEP 518 patch if on Windows Python 3.5 # (will only have an effect if PEP 517 builds are used): @@ -216,7 +222,7 @@ def build(options: BuildOptions) -> None: built_wheel_dir.mkdir(parents=True) # Path.resolve() is needed. Without it pip wheel may try to fetch package from pypi.org # see https://github.com/joerick/cibuildwheel/pull/369 - shell([ + call([ 'pip', 'wheel', options.package_dir.resolve(), '-w', built_wheel_dir, @@ -235,18 +241,18 @@ def build(options: BuildOptions) -> None: shutil.move(str(built_wheel), repaired_wheel_dir) else: repair_command_prepared = prepare_command(options.repair_command, wheel=built_wheel, dest_dir=repaired_wheel_dir) - shell([repair_command_prepared], env=env) + shell(repair_command_prepared, env=env) repaired_wheel = next(repaired_wheel_dir.glob('*.whl')) 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. - shell(['pip', 'install', 'virtualenv', *dependency_constraint_flags], env=env) + call(['pip', 'install', 'virtualenv', *dependency_constraint_flags], env=env) venv_dir = Path(tempfile.mkdtemp()) # Use --no-download to ensure determinism by using seed libraries # built into virtualenv - shell(['python', '-m', 'virtualenv', '--no-download', venv_dir], env=env) + call(['python', '-m', 'virtualenv', '--no-download', venv_dir], env=env) virtualenv_env = env.copy() virtualenv_env['PATH'] = os.pathsep.join([ @@ -255,7 +261,7 @@ def build(options: BuildOptions) -> None: ]) # check that we are using the Python from the virtual environment - shell(['which', 'python'], env=virtualenv_env) + call(['which', 'python'], env=virtualenv_env) if options.before_test: before_test_prepared = prepare_command( @@ -263,14 +269,14 @@ def build(options: BuildOptions) -> None: project='.', package=options.package_dir ) - shell([before_test_prepared], env=virtualenv_env) + shell(before_test_prepared, env=virtualenv_env) # install the wheel - shell(['pip', 'install', str(repaired_wheel) + options.test_extras], env=virtualenv_env) + call(['pip', 'install', str(repaired_wheel) + options.test_extras], env=virtualenv_env) # test the wheel if options.test_requires: - shell(['pip', 'install'] + options.test_requires, env=virtualenv_env) + call(['pip', 'install'] + options.test_requires, env=virtualenv_env) # run the tests from c:\, with an absolute path in the command # (this ensures that Python runs the tests against the installed wheel @@ -280,7 +286,7 @@ def build(options: BuildOptions) -> None: project=Path('.').resolve(), package=options.package_dir.resolve() ) - shell([test_command_prepared], cwd='c:\\', env=virtualenv_env) + shell(test_command_prepared, cwd='c:\\', env=virtualenv_env) # clean up shutil.rmtree(venv_dir) From 048ddc4af36e3231091d372bfc0b26d2c90f30be Mon Sep 17 00:00:00 2001 From: Joe Rickerby Date: Wed, 8 Jul 2020 20:12:50 +0100 Subject: [PATCH 11/11] Update test/test_pep518.py Co-authored-by: Henry Schreiner --- test/test_pep518.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test_pep518.py b/test/test_pep518.py index 2054616b..9257f7db 100644 --- a/test/test_pep518.py +++ b/test/test_pep518.py @@ -26,7 +26,7 @@ basic_project.files[ ] = """ [build-system] requires = [ - "setuptools>=42", + "setuptools >= 42", "wheel", "requests==2.22.0; python_version<'3.6'", "requests==2.23.0; python_version>='3.6'"