From 230a869e9670ca75bd886ab05d74544a0aeaffbe Mon Sep 17 00:00:00 2001 From: Mike Foster Date: Wed, 12 Jun 2024 20:28:38 +0200 Subject: [PATCH 01/10] set VSCMD_ARG_TGT_ARCH based on targetted architecture --- cibuildwheel/windows.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index 83c52ab88..a7eead6f6 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -152,6 +152,12 @@ def setup_setuptools_cross_compile( # set the platform name map_plat = {"32": "win32", "64": "win-amd64", "ARM64": "win-arm64"} plat_name = map_plat[python_configuration.arch] + + # Set environment variable so that setuptools._distutils.get_platform() + # identifies the target, not the host + vscmd_arg_tgt_arch = {"32": "x86", "64": "x64", "ARM64": "arm64"} + env["VSCMD_ARG_TGT_ARCH"] = vscmd_arg_tgt_arch[python_configuration.arch] + # (This file must be default/locale encoding, so we can't pass 'encoding') distutils_cfg.write_text( textwrap.dedent( From ceb885be6c25e78f527e0765072e613f2465ec29 Mon Sep 17 00:00:00 2001 From: Mike Foster Date: Thu, 13 Jun 2024 17:54:07 +0200 Subject: [PATCH 02/10] only set VSCMD_ARG_TGT_ARCH if not already set --- cibuildwheel/windows.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index a7eead6f6..a5b9342a4 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -155,8 +155,9 @@ def setup_setuptools_cross_compile( # Set environment variable so that setuptools._distutils.get_platform() # identifies the target, not the host - vscmd_arg_tgt_arch = {"32": "x86", "64": "x64", "ARM64": "arm64"} - env["VSCMD_ARG_TGT_ARCH"] = vscmd_arg_tgt_arch[python_configuration.arch] + if not env.get("VSCMD_ARG_TGT_ARCH"): + vscmd_arg_tgt_arch = {"32": "x86", "64": "x64", "ARM64": "arm64"} + env["VSCMD_ARG_TGT_ARCH"] = vscmd_arg_tgt_arch[python_configuration.arch] # (This file must be default/locale encoding, so we can't pass 'encoding') distutils_cfg.write_text( From cd9a8800d9066b6b4cf21365744e8f3a2f49ba4f Mon Sep 17 00:00:00 2001 From: Mike Foster Date: Fri, 14 Jun 2024 06:55:14 +0200 Subject: [PATCH 03/10] add unit tests to check setuptools correctly identifies windows arch --- pyproject.toml | 3 ++ unit_test/get_platform_test.py | 88 ++++++++++++++++++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 unit_test/get_platform_test.py diff --git a/pyproject.toml b/pyproject.toml index c8f3ca485..94772e9f3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -75,6 +75,7 @@ test = [ "pytest-timeout", "pytest-xdist", "pytest>=6", + "setuptools", "tomli_w", "validate-pyproject", ] @@ -140,6 +141,8 @@ disallow_untyped_decorators = true [[tool.mypy.overrides]] module = [ "setuptools", + "setuptools._distutils", # needed even if only directly import setuptools._distutils.util + "setuptools._distutils.util", "pytest", # ignored in pre-commit to speed up check "bashlex", "bashlex.*", diff --git a/unit_test/get_platform_test.py b/unit_test/get_platform_test.py new file mode 100644 index 000000000..ccc5273fb --- /dev/null +++ b/unit_test/get_platform_test.py @@ -0,0 +1,88 @@ +# ruff: noqa: ARG001 +import contextlib +from pathlib import Path +from typing import Dict + +import pytest +import setuptools._distutils.util + +from cibuildwheel.windows import PythonConfiguration, setup_setuptools_cross_compile + + +@contextlib.contextmanager +def patched_environment(monkeypatch: pytest.MonkeyPatch, environment: Dict[str, str]): + with monkeypatch.context() as mp: + mp.setattr("os.name", "nt") + for envvar, val in environment.items(): + mp.setenv(name=envvar, value=val) + yield + + +def test_x86(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + arch = "32" + environment: Dict[str, str] = {} + + configuration = PythonConfiguration("irrelevant", arch, "irrelevant", None) + + setup_setuptools_cross_compile(tmp_path, configuration, tmp_path, environment) + with patched_environment(monkeypatch, environment): + target_platform = setuptools._distutils.util.get_platform() + + assert environment["VSCMD_ARG_TGT_ARCH"] == "x86" + assert target_platform == "win32" + + +def test_x64(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + arch = "64" + environment: Dict[str, str] = {} + + configuration = PythonConfiguration("irrelevant", arch, "irrelevant", None) + + setup_setuptools_cross_compile(tmp_path, configuration, tmp_path, environment) + with patched_environment(monkeypatch, environment): + target_platform = setuptools._distutils.util.get_platform() + + assert environment["VSCMD_ARG_TGT_ARCH"] == "x64" + assert target_platform == "win-amd64" + + +def test_arm(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + arch = "ARM64" + environment: Dict[str, str] = {} + + configuration = PythonConfiguration("irrelevant", arch, "irrelevant", None) + + setup_setuptools_cross_compile(tmp_path, configuration, tmp_path, environment) + with patched_environment(monkeypatch, environment): + target_platform = setuptools._distutils.util.get_platform() + + assert environment["VSCMD_ARG_TGT_ARCH"] == "arm64" + assert target_platform == "win-arm64" + + +def test_env_set(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + arch = "32" + environment = {"VSCMD_ARG_TGT_ARCH": "arm64"} + + configuration = PythonConfiguration("irrelevant", arch, "irrelevant", None) + + setup_setuptools_cross_compile(tmp_path, configuration, tmp_path, environment) + with patched_environment(monkeypatch, environment): + target_platform = setuptools._distutils.util.get_platform() + + assert environment["VSCMD_ARG_TGT_ARCH"] == "arm64" + assert target_platform == "win-arm64" + + +def test_env_blank(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + arch = "32" + environment = {"VSCMD_ARG_TGT_ARCH": ""} + + configuration = PythonConfiguration("irrelevant", arch, "irrelevant", None) + + setup_setuptools_cross_compile(tmp_path, configuration, tmp_path, environment) + with patched_environment(monkeypatch, environment): + target_platform = setuptools._distutils.util.get_platform() + + assert environment["VSCMD_ARG_TGT_ARCH"] == "x86" + assert target_platform == "win32" From c1fba360ef65440d3b85dae374cada5856ce045d Mon Sep 17 00:00:00 2001 From: Mike Foster Date: Fri, 14 Jun 2024 07:45:21 +0200 Subject: [PATCH 04/10] only run tests on windows --- unit_test/get_platform_test.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/unit_test/get_platform_test.py b/unit_test/get_platform_test.py index ccc5273fb..c3d00b4a3 100644 --- a/unit_test/get_platform_test.py +++ b/unit_test/get_platform_test.py @@ -1,5 +1,6 @@ # ruff: noqa: ARG001 import contextlib +import sys from pathlib import Path from typing import Dict @@ -8,11 +9,14 @@ from cibuildwheel.windows import PythonConfiguration, setup_setuptools_cross_compile +# monkeypatching os.name is too flaky. E.g. It works on my machine, but fails in pipeline +if not sys.platform.startswith("win"): + pytest.skip("Windows-only tests", allow_module_level=True) + @contextlib.contextmanager def patched_environment(monkeypatch: pytest.MonkeyPatch, environment: Dict[str, str]): with monkeypatch.context() as mp: - mp.setattr("os.name", "nt") for envvar, val in environment.items(): mp.setenv(name=envvar, value=val) yield From cedaf8423f81856955747d6014603afff0f22e0d Mon Sep 17 00:00:00 2001 From: Mike Foster Date: Fri, 14 Jun 2024 08:20:51 +0200 Subject: [PATCH 05/10] arm64 fails on azure pipelines (only) --- unit_test/get_platform_test.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/unit_test/get_platform_test.py b/unit_test/get_platform_test.py index c3d00b4a3..d5e609bcf 100644 --- a/unit_test/get_platform_test.py +++ b/unit_test/get_platform_test.py @@ -7,6 +7,7 @@ import pytest import setuptools._distutils.util +from cibuildwheel.util import CIProvider, detect_ci_provider from cibuildwheel.windows import PythonConfiguration, setup_setuptools_cross_compile # monkeypatching os.name is too flaky. E.g. It works on my machine, but fails in pipeline @@ -50,6 +51,9 @@ def test_x64(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): assert target_platform == "win-amd64" +@pytest.mark.skipif( + detect_ci_provider() == CIProvider.azure_pipelines, reason="arm64 not recognised on azure" +) def test_arm(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): arch = "ARM64" environment: Dict[str, str] = {} @@ -66,7 +70,7 @@ def test_arm(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): def test_env_set(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): arch = "32" - environment = {"VSCMD_ARG_TGT_ARCH": "arm64"} + environment = {"VSCMD_ARG_TGT_ARCH": "x64"} configuration = PythonConfiguration("irrelevant", arch, "irrelevant", None) @@ -74,8 +78,8 @@ def test_env_set(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): with patched_environment(monkeypatch, environment): target_platform = setuptools._distutils.util.get_platform() - assert environment["VSCMD_ARG_TGT_ARCH"] == "arm64" - assert target_platform == "win-arm64" + assert environment["VSCMD_ARG_TGT_ARCH"] == "x64" + assert target_platform == "win-amd64" def test_env_blank(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): From a5c8382ad92a5b0636b822ca0bb24b929b18952b Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Tue, 2 Jul 2024 12:16:39 -0400 Subject: [PATCH 06/10] Update windows.py --- cibuildwheel/windows.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index a5b9342a4..26426157b 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -155,9 +155,12 @@ def setup_setuptools_cross_compile( # Set environment variable so that setuptools._distutils.get_platform() # identifies the target, not the host - if not env.get("VSCMD_ARG_TGT_ARCH"): - vscmd_arg_tgt_arch = {"32": "x86", "64": "x64", "ARM64": "arm64"} - env["VSCMD_ARG_TGT_ARCH"] = vscmd_arg_tgt_arch[python_configuration.arch] + vscmd_arg_tgt_arch = {"32": "x86", "64": "x64", "ARM64": "arm64"} + current_tgt_arch = vscmd_arg_tgt_arch[python_configuration.arch] + if env.get("VSCMD_ARG_TGT_ARCH", current_tgt_arch) != current_tgt_arch: + msg = f"VSCMD_ARG_TGT_ARCH must be set to {current_tgt_arch}, got {env['VSCMD_ARG_TGT_ARCH']}. Make sure you setup MSVC targetting the right architecture." + raise errors.FatalError(msg) + env["VSCMD_ARG_TGT_ARCH"] = current_tgt_arch # (This file must be default/locale encoding, so we can't pass 'encoding') distutils_cfg.write_text( From a3e72e5f6636c11bf22c9f43cda68f011f0c223a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 2 Jul 2024 16:17:06 +0000 Subject: [PATCH 07/10] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- cibuildwheel/windows.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index 26426157b..a8c389886 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -158,8 +158,8 @@ def setup_setuptools_cross_compile( vscmd_arg_tgt_arch = {"32": "x86", "64": "x64", "ARM64": "arm64"} current_tgt_arch = vscmd_arg_tgt_arch[python_configuration.arch] if env.get("VSCMD_ARG_TGT_ARCH", current_tgt_arch) != current_tgt_arch: - msg = f"VSCMD_ARG_TGT_ARCH must be set to {current_tgt_arch}, got {env['VSCMD_ARG_TGT_ARCH']}. Make sure you setup MSVC targetting the right architecture." - raise errors.FatalError(msg) + msg = f"VSCMD_ARG_TGT_ARCH must be set to {current_tgt_arch}, got {env['VSCMD_ARG_TGT_ARCH']}. Make sure you setup MSVC targeting the right architecture." + raise errors.FatalError(msg) env["VSCMD_ARG_TGT_ARCH"] = current_tgt_arch # (This file must be default/locale encoding, so we can't pass 'encoding') From 4d319bf5d74ba5567ec94fa263254891d8a24322 Mon Sep 17 00:00:00 2001 From: Henry Schreiner Date: Tue, 2 Jul 2024 13:54:15 -0400 Subject: [PATCH 08/10] Apply suggestions from code review Co-authored-by: Joe Rickerby --- cibuildwheel/windows.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cibuildwheel/windows.py b/cibuildwheel/windows.py index a8c389886..a1528844e 100644 --- a/cibuildwheel/windows.py +++ b/cibuildwheel/windows.py @@ -157,8 +157,8 @@ def setup_setuptools_cross_compile( # identifies the target, not the host vscmd_arg_tgt_arch = {"32": "x86", "64": "x64", "ARM64": "arm64"} current_tgt_arch = vscmd_arg_tgt_arch[python_configuration.arch] - if env.get("VSCMD_ARG_TGT_ARCH", current_tgt_arch) != current_tgt_arch: - msg = f"VSCMD_ARG_TGT_ARCH must be set to {current_tgt_arch}, got {env['VSCMD_ARG_TGT_ARCH']}. Make sure you setup MSVC targeting the right architecture." + if (env.get("VSCMD_ARG_TGT_ARCH") or current_tgt_arch) != current_tgt_arch: + msg = f"VSCMD_ARG_TGT_ARCH must be set to {current_tgt_arch!r}, got {env['VSCMD_ARG_TGT_ARCH']!r}. If you're setting up MSVC yourself (e.g. using vcvarsall.bat or msvc-dev-cmd), make sure to target the right architecture. Alternatively, run cibuildwheel without configuring MSVC, and let the build backend handle it." raise errors.FatalError(msg) env["VSCMD_ARG_TGT_ARCH"] = current_tgt_arch From 9ee0d28d386bc398a2fd5e059bc676923c753d5b Mon Sep 17 00:00:00 2001 From: Mike Foster Date: Wed, 3 Jul 2024 07:17:21 +0200 Subject: [PATCH 09/10] update test_env_set to validate FatalError is raised on env collision --- unit_test/get_platform_test.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/unit_test/get_platform_test.py b/unit_test/get_platform_test.py index d5e609bcf..194454a0f 100644 --- a/unit_test/get_platform_test.py +++ b/unit_test/get_platform_test.py @@ -7,6 +7,7 @@ import pytest import setuptools._distutils.util +from cibuildwheel.errors import FatalError from cibuildwheel.util import CIProvider, detect_ci_provider from cibuildwheel.windows import PythonConfiguration, setup_setuptools_cross_compile @@ -68,18 +69,14 @@ def test_arm(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): assert target_platform == "win-arm64" -def test_env_set(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): +def test_env_set(tmp_path: Path): arch = "32" environment = {"VSCMD_ARG_TGT_ARCH": "x64"} configuration = PythonConfiguration("irrelevant", arch, "irrelevant", None) - setup_setuptools_cross_compile(tmp_path, configuration, tmp_path, environment) - with patched_environment(monkeypatch, environment): - target_platform = setuptools._distutils.util.get_platform() - - assert environment["VSCMD_ARG_TGT_ARCH"] == "x64" - assert target_platform == "win-amd64" + with pytest.raises(FatalError, match="VSCMD_ARG_TGT_ARCH"): + setup_setuptools_cross_compile(tmp_path, configuration, tmp_path, environment) def test_env_blank(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): From fba21a9aa85785e621b8d05be41a25c068d14679 Mon Sep 17 00:00:00 2001 From: Mike Foster Date: Sun, 7 Jul 2024 15:09:38 +0200 Subject: [PATCH 10/10] remove noqa ARG001 (no longer needed) --- unit_test/get_platform_test.py | 1 - 1 file changed, 1 deletion(-) diff --git a/unit_test/get_platform_test.py b/unit_test/get_platform_test.py index 194454a0f..dfcc7d87b 100644 --- a/unit_test/get_platform_test.py +++ b/unit_test/get_platform_test.py @@ -1,4 +1,3 @@ -# ruff: noqa: ARG001 import contextlib import sys from pathlib import Path