Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable building for Windows ARM64 on a non-ARM64 device #1144

Merged
merged 48 commits into from Oct 11, 2022
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
ea559af
Enable building for Windows ARM64 on a non-ARM64 device
zooba Jun 17, 2022
310cfda
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jun 17, 2022
6bcca2d
Satisfy MyPy
zooba Jun 17, 2022
e18c21a
Merge branch 'arm64' of https://github.com/zooba/cibuildwheel into arm64
zooba Jun 17, 2022
dc14bd1
Add cross-compiler test
zooba Jun 17, 2022
0704190
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jun 17, 2022
222d56b
Allow more arch values in get_nuget_args
zooba Jun 17, 2022
36442d5
Merge branch 'arm64' of https://github.com/zooba/cibuildwheel into arm64
zooba Jun 17, 2022
b5b8e12
Add a pyproject.toml to the windows arm64 cross compile test
joerick Jul 4, 2022
71df33d
Merge remote-tracking branch 'upstream/main' into arm64
zooba Aug 1, 2022
59febc1
Add setup_rust_cross_compile
zooba Aug 1, 2022
a2386a2
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 1, 2022
94a053e
Apply some PR feedback
zooba Aug 1, 2022
9405d82
Merge branch 'arm64' of https://github.com/zooba/cibuildwheel into arm64
zooba Aug 1, 2022
4c73245
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 1, 2022
c3dcb07
Revert type names
zooba Aug 1, 2022
46cc262
Merge branch 'main' into arm64
zooba Aug 15, 2022
15288be
Merge branch 'main' into arm64
zooba Aug 22, 2022
5f391d6
Merge branch 'main' into arm64
zooba Aug 22, 2022
634a3f7
Expect correct wheel tag
zooba Aug 23, 2022
f5c82fc
Merge branch 'main' into arm64
zooba Aug 23, 2022
09e08f6
Write to ~/pydistutils.cfg instead, preserving any existing file
zooba Aug 23, 2022
b8ac98e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 23, 2022
dcc5c00
Ensure cfg is removed, and add a message
zooba Aug 23, 2022
9e07dae
Skip ARM64 test run when not running on ARM64
zooba Aug 24, 2022
9a4e8f6
Actually call unlink, and add more typing
zooba Aug 25, 2022
f277099
Skip ARM64 tests automatically with warning, and improved cleanup
zooba Aug 31, 2022
7b0ac9e
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Aug 31, 2022
b94aceb
Merge remote-tracking branch 'upstream/main' into arm64
zooba Aug 31, 2022
4242fe9
Merge branch 'arm64' of https://github.com/zooba/cibuildwheel into arm64
zooba Aug 31, 2022
9dc5512
Fix typing
zooba Aug 31, 2022
44ebbc4
Remove unused import
zooba Aug 31, 2022
1d7054c
Switch to potential new distutils environment variable
zooba Sep 1, 2022
ff2d9de
Flow through tmp directory
zooba Sep 1, 2022
5030c2c
Remove unused names
zooba Sep 2, 2022
9815b9c
Merge remote-tracking branch 'upstream/main' into arm64
zooba Sep 9, 2022
10a4e30
Merge remote-tracking branch 'upstream/main' into arm64
zooba Sep 26, 2022
4531370
Rename variable to match one added to setuptools
zooba Sep 26, 2022
063b8e4
Merge branch 'main' into arm64
zooba Sep 28, 2022
b8e4789
Add skip for missing ARM64 tools
zooba Sep 28, 2022
7fd4a22
Merge branch 'arm64' of https://github.com/zooba/cibuildwheel into arm64
zooba Sep 28, 2022
7a9bd57
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Sep 28, 2022
ea1d15b
For mypy
zooba Sep 28, 2022
18ced3c
Update docs and fix default architectures
zooba Oct 3, 2022
ae58080
Merge remote-tracking branch 'upstream/main' into arm64
zooba Oct 3, 2022
aa79b6e
Revert default archs on ARM64 change
zooba Oct 4, 2022
9128465
Merge remote-tracking branch 'upstream/main' into arm64
zooba Oct 4, 2022
afdae3f
AppVeyor supports ARM64 builds
zooba Oct 4, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
76 changes: 72 additions & 4 deletions cibuildwheel/windows.py
@@ -1,4 +1,5 @@
import os
import platform as platform_module
import shutil
import subprocess
import sys
Expand Down Expand Up @@ -33,10 +34,16 @@


def get_nuget_args(version: str, arch: str, output_directory: Path) -> List[str]:
platform_suffix = {"32": "x86", "64": "", "ARM64": "arm64"}
python_name = "python" + platform_suffix[arch]
package_name = {
"32": "pythonx86",
"64": "python",
"ARM64": "pythonarm64",
# Aliases for platform.machine() return values
"x86": "pythonx86",
"AMD64": "python",
}[arch]
return [
python_name,
package_name,
"-Version",
version,
"-FallbackSource",
Expand Down Expand Up @@ -115,6 +122,50 @@ def install_pypy(tmp: Path, arch: str, url: str) -> Path:
return installation_path / "python.exe"


def setup_setuptools_cross_compile(
python_configuration: PythonConfiguration,
python_libs_base: Path,
env: Dict[str, str],
) -> None:
# We write to distutils_cfg for distutils-based builds because we know we
# currently don't have one (unless it's our own from a previous build)
for p in env["PATH"].split(os.pathsep):
distutils_cfg = Path(p) / "Lib/site-packages/setuptools/_distutils/distutils.cfg"
if distutils_cfg.parent.is_dir():
break
else:
log.warning("Did not find setuptools install to configure")
return
joerick marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On this method more broadly, I do worry that it would end up modifying something outside of the build venv if setuptools isn't installed there, but is in a venv higher up.

So I might propose a change to this logic, like

Suggested change
for p in env["PATH"].split(os.pathsep):
distutils_cfg = Path(p) / "Lib/site-packages/setuptools/_distutils/distutils.cfg"
if distutils_cfg.parent.is_dir():
break
else:
log.warning("Did not find setuptools install to configure")
return
venv_python = call(
["python", "-c", "import sys; sys.stdout.write(sys.executable)"],
env=env,
capture_stdout=True,
)
distutils_cfg = Path(venv_python).parent / "Lib/site-packages/setuptools/_distutils/distutils.cfg"
if not distutils_cfg.parent.is_dir():
log.warning("Did not find setuptools install to configure for cross-compilation.")
return

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with this new logic is that call(["python" ... doesn't actually search PATH to resolve python. It first looks in the current executable directory (normal Windows behaviour, assuming that you want the files you installed with your own app before searching other installs). So it only works if we're already running in the correct Python, in which case we may as well go straight to sys.prefix.

Also, if we're going to run Python and import it, I'd rather import distutils; print(distutils.__file__) and work from that to save a few assumptions. Or deliberately keep the venv path around from earlier - I tried that briefly and decided it wasn't worth the changes, but it's going to be the most reliable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, and we alse know that env["PATH"] contains the right venv first because we just updated it ourselves.

Copy link
Contributor

@joerick joerick Jul 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem with this new logic is that call(["python" ... doesn't actually search PATH to resolve python. It first looks in the current executable directory (normal Windows behaviour, assuming that you want the files you installed with your own app before searching other installs).

馃く whoa really? when you say the 'current executable directory', do you mean the working directory, which would be the project dir in this case? We do call(["python" ... all over the place and we've never had an issue with the wrong one being invoked. Maybe because people don't tend to have a python.exe in the root of their project.

Also, if we're going to run Python and import it, I'd rather import distutils; print(distutils.__file__) and work from that to save a few assumptions.

I'd prefer that, I think. We probably won't preinstall setuptools in the build venv forever. Though I think it should be import setuptools._distutils; print(setuptools._distutils.__file__), right? (though, depending on the outcome of the isolated build-backend venv conversation, it might be a moot point...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whoa really? when you say the 'current executable directory', do you mean the working directory, which would be the project dir in this case?

No, it's more like (don't reuse this code) os.path.split(sys.orig_argv[0]). The current working directory lookup is a "feature" of the old cmd shell, but looking in the application directory is a "feature" of CreateProcess (the underlying OS API call). Best way to disable it is to pass a full path.

Though I think it should be import setuptools._distutils; print(setuptools._distutils.__file__), right?

That isn't forward compatible though. setuptools._distutils is an internal name, and I expect setuptools to deprecate their distutils shim almost immediately and get people using their commands directly.

But yeah, isolated backends are going to break this approach anyway. Setuptools is currently broken anyway because of another change, so if they're amenable to having their build* commands pick up default values from environment variables, we can probably get that in (though I'm about to be AFK for at least a week, and then at EuroPython the week after that, so it might have to wait until the sprints).


# Ensure our additional import libraries are made available, and explicitly
# set the platform name
map_plat = {"32": "win32", "64": "win-amd64", "ARM64": "win-arm64"}
plat_name = map_plat[python_configuration.arch]
# (Sorry, this file must be default/locale encoding.)
with distutils_cfg.open("w") as f:
print("[build]", file=f)
print("plat_name=", plat_name, file=f, sep="")
print("[build_ext]", file=f)
print("library_dirs=", python_libs_base, file=f, sep="")
print("plat_name=", plat_name, file=f, sep="")
print("[bdist_wheel]", file=f)
print("plat_name=", plat_name, file=f, sep="")
zooba marked this conversation as resolved.
Show resolved Hide resolved

# setuptools builds require explicit override of PYD extension
# This is because it always gets the extension from the running
# interpreter, and has no logic to construct it. Currently, CPython's
# extensions follow our identifiers, but if they ever diverge in the
# future, we will need to store new data
env["SETUPTOOLS_EXT_SUFFIX"] = f".{python_configuration.identifier}.pyd"

# Cross-compilation requires fixes that only exist in setuptools's copy of
# distutils, so ensure that it is activated
# Since not all projects can handle the newer distutils, display a warning
# to help them figure out what may have gone wrong if this breaks for them
log.warning("Setting SETUPTOOLS_USE_DISTUTILS=local as it is required for cross-compilation")
zooba marked this conversation as resolved.
Show resolved Hide resolved
env["SETUPTOOLS_USE_DISTUTILS"] = "local"


def setup_python(
tmp: Path,
python_configuration: PythonConfiguration,
Expand All @@ -124,9 +175,22 @@ def setup_python(
) -> Dict[str, str]:
tmp.mkdir()
implementation_id = python_configuration.identifier.split("-")[0]
python_libs_base = None
log.step(f"Installing Python {implementation_id}...")
if implementation_id.startswith("cp"):
base_python = install_cpython(python_configuration.version, python_configuration.arch)
native_arch = platform_module.machine()
if python_configuration.arch == "ARM64" != native_arch:
# To cross-compile for ARM64, we need a native CPython to run the
# build, and a copy of the ARM64 import libraries ('.\libs\*.lib')
# for any extension modules.
python_libs_base = install_cpython(
python_configuration.version, python_configuration.arch
)
python_libs_base = python_libs_base.parent / "libs"
log.step(f"Installing native Python {native_arch} for cross-compilation...")
base_python = install_cpython(python_configuration.version, native_arch)
else:
base_python = install_cpython(python_configuration.version, python_configuration.arch)
elif implementation_id.startswith("pp"):
assert python_configuration.url is not None
base_python = install_pypy(tmp, python_configuration.arch, python_configuration.url)
Expand Down Expand Up @@ -227,6 +291,10 @@ def setup_python(
else:
assert_never(build_frontend)

if python_libs_base:
# Set up the environment for various backends to enable cross-compilation
joerick marked this conversation as resolved.
Show resolved Hide resolved
setup_setuptools_cross_compile(python_configuration, python_libs_base, env)

return env


Expand Down
32 changes: 32 additions & 0 deletions test/test_windows.py
@@ -0,0 +1,32 @@
import pytest

from . import test_projects, utils

basic_project = test_projects.new_c_project()
zooba marked this conversation as resolved.
Show resolved Hide resolved


def test_wheel_tag_is_correct_when_using_windows_cross_compile(tmp_path):
if utils.platform != "windows":
pytest.skip("This test is only relevant to Windows")

project_dir = tmp_path / "project"
basic_project.generate(project_dir)

# build the wheels
actual_wheels = utils.cibuildwheel_run(
project_dir,
add_env={
"CIBW_BUILD": "cp310-*",
},
add_args=["--platform", "windows", "--archs", "ARM64"],
zooba marked this conversation as resolved.
Show resolved Hide resolved
)

# check that the expected wheels are produced
expected_wheels = [
"spam-0.1.0-cp310-win_arm64.whl",
]

print("actual_wheels", actual_wheels)
print("expected_wheels", expected_wheels)

assert set(actual_wheels) == set(expected_wheels)