-
Notifications
You must be signed in to change notification settings - Fork 502
Wrap native win-arm64 scripts with a start /machine launcher
#6047
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8875d06
82f54cb
aabf7f7
0f108a8
6f01a09
c9a732d
595c5c6
fd3f4e9
e0c7671
cf481da
ae31081
0a59260
8be946d
da6ec70
026a84a
472a531
3d4127b
c7c01d9
88e257d
0dd56ed
2bca65a
6f1c4c2
affb171
5440d14
c6a8193
8ffa8dc
03c7046
5c6b4e1
3b98c48
79b3f1b
0558ea0
2ce83af
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,12 +1,23 @@ | ||||||
| # Copyright (C) 2014 Anaconda, Inc | ||||||
| # SPDX-License-Identifier: BSD-3-Clause | ||||||
| from __future__ import annotations | ||||||
|
|
||||||
| import os | ||||||
| import platform | ||||||
| import pprint | ||||||
| import sys | ||||||
| import sysconfig | ||||||
| from functools import cache | ||||||
| from itertools import product | ||||||
| from os.path import dirname, isdir, isfile, join | ||||||
| from typing import TYPE_CHECKING | ||||||
|
|
||||||
| if TYPE_CHECKING: | ||||||
| from typing import Literal | ||||||
|
|
||||||
| # importing setuptools patches distutils so that it knows how to find VC for python 2.7 | ||||||
| import setuptools # noqa | ||||||
| from conda.base.context import context | ||||||
|
|
||||||
| # Leverage the hard work done by setuptools/distutils to find vcvarsall using | ||||||
| # either the registry or the VS**COMNTOOLS environment variable | ||||||
|
|
@@ -74,6 +85,38 @@ def fix_staged_scripts(scripts_dir, config): | |||||
| os.remove(join(scripts_dir, fn)) | ||||||
|
|
||||||
|
|
||||||
| @cache | ||||||
| def get_native_windows_architecture() -> Literal["AMD64", "ARM64", "x86"] | str | None: | ||||||
| """ | ||||||
| Python 3.12+ `platform.machine()` on Windows reports the actual machine, | ||||||
| not the running interpreter. So we can just use that. | ||||||
|
|
||||||
| For prior versions, query the registry. | ||||||
|
|
||||||
| Returns None if it value cannot be obtained. | ||||||
| """ | ||||||
| if sys.platform != "win32": | ||||||
| raise OSError("This function is only supported on Windows.") | ||||||
|
|
||||||
| if sys.version_info >= (3, 12): | ||||||
| return platform.machine() or None | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will change if python/cpython#98962 is fixed and I want to push for that. |
||||||
|
|
||||||
| import winreg | ||||||
|
|
||||||
| registry_path = r"SYSTEM\CurrentControlSet\Control\Session Manager\Environment" | ||||||
|
|
||||||
| try: | ||||||
| with winreg.OpenKey( | ||||||
| winreg.HKEY_LOCAL_MACHINE, registry_path, 0, winreg.KEY_READ | ||||||
| ) as key: | ||||||
| native_arch, _ = winreg.QueryValueEx(key, "PROCESSOR_ARCHITECTURE") | ||||||
| return native_arch.upper() | ||||||
| except Exception as e: | ||||||
| log = get_logger(__name__) | ||||||
| log.debug("Could not detect native architecture via registry", exc_info=e) | ||||||
| return None | ||||||
|
|
||||||
|
|
||||||
| def build_vcvarsall_vs_path(version): | ||||||
| """ | ||||||
| Given the Visual Studio version, returns the default path to the | ||||||
|
|
@@ -293,6 +336,13 @@ def write_build_scripts(m, env, bld_bat): | |||||
| env["PYTHONDONTWRITEBYTECODE"] = True | ||||||
| import codecs | ||||||
|
|
||||||
| if m.config.build_subdir != _running_subdir(): | ||||||
| # Can't trust the parent environment under these conditions | ||||||
| build_arch = _build_arch(m) | ||||||
| env["PROCESSOR_ARCHITECTURE"] = build_arch | ||||||
| if build_arch == get_native_windows_architecture(): | ||||||
| env.pop("PROCESSOR_ARCHITEW6432", None) | ||||||
|
Comment on lines
+342
to
+344
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note these are being set to whatever conda-build/conda_build/environ.py Lines 685 to 686 in f2b6617
so we need to undo that here |
||||||
|
|
||||||
| with codecs.getwriter("utf-8")(open(env_script, "wb")) as fo: | ||||||
| # more debuggable with echo on | ||||||
| fo.write("@echo on\n") | ||||||
|
|
@@ -347,6 +397,52 @@ def write_build_scripts(m, env, bld_bat): | |||||
| return work_script, env_script | ||||||
|
|
||||||
|
|
||||||
| def _build_arch(m) -> Literal["AMD64", "ARM64", "x86"]: | ||||||
| """ | ||||||
| If conda-build is run from e.g. a win-64 environment on a win-arm64 machine | ||||||
| users may want to build natively by setting build_platform="win-arm64". | ||||||
| In those cases, we need to ensure that the CMD process is native ARM64 | ||||||
| via this `start` wrapper. Otherwise Windows picks the AMD64 slice! | ||||||
| This gives you the adequate /machine flag value for `start`. | ||||||
| """ | ||||||
| build_arch = m.config.build_subdir.split("-")[1].upper() | ||||||
| return {"64": "AMD64", "32": "x86"}.get(build_arch, build_arch) | ||||||
|
|
||||||
|
|
||||||
| def _running_subdir(): | ||||||
| """ | ||||||
| On Python 3.12+, `platform.machine()` (on which conda.base.context._native_subdir() relies) | ||||||
| may report ARM64 even when running from a win-64 installation. | ||||||
| This is different from what one can observe on macOS/Rosetta, where an emulated osx-64 Python | ||||||
| on Apple Silicon reports x86_64. | ||||||
|
|
||||||
| More context at https://github.com/python/cpython/issues/98962. | ||||||
|
|
||||||
| The most obvious way is to check %PROCESSOR_ARCHITECTURE%, if available, but we need to hope | ||||||
| it was not overridden. `sysconfig.get_platform()` seems to be accurate enough. | ||||||
| """ | ||||||
| if os.name == "nt": | ||||||
| arch = ( | ||||||
| sysconfig.get_platform().lower() | ||||||
| ) # -> Literal['win-amd64', 'win-arm64', 'win32'] | ||||||
| return {"win-amd64": "win-64", "win32": "win-32"}.get(arch, arch) | ||||||
| return context._native_subdir() | ||||||
|
|
||||||
|
|
||||||
| def build_command_arguments(m, script: str) -> list[str]: | ||||||
| if m.config.build_subdir != _running_subdir(): | ||||||
| # See docstring of _cmd_machine_flag() | ||||||
| wrapper = os.path.join(os.path.dirname(script), "_conda_build_wrapper.bat") | ||||||
| with open(wrapper, "w") as f: | ||||||
| f.write( | ||||||
| "@echo off\r\n" | ||||||
| f'start /b /wait /machine {_build_arch(m)} cmd.exe /d /c "{script}"\r\n' | ||||||
| "exit /b %ERRORLEVEL%\r\n" | ||||||
| ) | ||||||
| return ["cmd.exe", "/d", "/c", os.path.basename(wrapper)] | ||||||
| return ["cmd.exe", "/d", "/c", os.path.basename(script)] | ||||||
|
|
||||||
|
|
||||||
| def build(m, bld_bat, stats, provision_only=False): | ||||||
| # TODO: Prepending the prefixes here should probably be guarded by | ||||||
| # if not m.activate_build_script: | ||||||
|
|
@@ -390,7 +486,7 @@ def build(m, bld_bat, stats, provision_only=False): | |||||
| work_script, env_script = write_build_scripts(m, env, bld_bat) | ||||||
|
|
||||||
| if not provision_only and os.path.isfile(work_script): | ||||||
| cmd = ["cmd.exe", "/d", "/c", os.path.basename(work_script)] | ||||||
| cmd = build_command_arguments(m, work_script) | ||||||
| # rewrite long paths in stdout back to their env variables | ||||||
| if m.config.debug or m.config.no_rewrite_stdout_env: | ||||||
| rewrite_env = None | ||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| ### Enhancements | ||
|
|
||
| * <news item> | ||
|
|
||
| ### Bug fixes | ||
|
|
||
| * Make CMD subprocesses match `build_platform` architecture when running emulated Python processes. (#6048 via #6047) | ||
| * `recipe.yaml` build/host platform handling now behaves in the same as it did in `meta.yaml`. Accidentally allowed `{build,host}_platform` settings in `conda_build_config.yaml` are no longer valid; instead, users can export `CONDA_SUBDIR` and define the `target_platform` setting, respectively. (#6047) | ||
|
|
||
| ### Deprecations | ||
|
|
||
| * <news item> | ||
|
|
||
| ### Docs | ||
|
|
||
| * <news item> | ||
|
|
||
| ### Other | ||
|
|
||
| * <news item> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe these lines are sufficiently complex enough to warrant a comment? 😅 I'm struggling to understand exactly what this is doing. Especially because of the doubled
env.PYTEST_MARKERandmatrix.pytest-expressionusage.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These are ternary operators in JavaScript, adapted to Github Actions. Equivalent to
'-m "' if env.PYTEST_MARKER else ''. So if PYTEST_MARKER is set, then we write-m "{{ env.PYTEST_MARKER }}".