From 0997cc1a05a4ef3b3ad72e8d78f907a1c80a4119 Mon Sep 17 00:00:00 2001 From: Simon Mavi Stewart Date: Fri, 6 Dec 2024 16:24:13 +0000 Subject: [PATCH 1/5] Correctly resolve macOS SDK paths XCode has facilities for accurately telling us where SDKs are installed. This is important to use, particularly when there may be multiple SDKs or versions of XCode installed. --- python/private/pypi/whl_library.bzl | 41 +++++++++++++++++++++++------ 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/python/private/pypi/whl_library.bzl b/python/private/pypi/whl_library.bzl index 79a58a81f2..783a1407ad 100644 --- a/python/private/pypi/whl_library.bzl +++ b/python/private/pypi/whl_library.bzl @@ -55,9 +55,31 @@ def _get_xcode_location_cflags(rctx): # This is a full xcode installation somewhere like /Applications/Xcode13.0.app/Contents/Developer # so we need to change the path to to the macos specific tools which are in a different relative # path than xcode installed command line tools. - xcode_root = "{}/Platforms/MacOSX.platform/Developer".format(xcode_root) + xcode_sdks_json = rctx.execute([ + "xcrun", + "xcodebuild", + "-showsdks", + "-json", + ], environment = { + "DEVELOPER_DIR": xcode_root, + }).stdout + xcode_sdks = json.decode(xcode_sdks_json) + potential_sdks = [ + sdk + for sdk in xcode_sdks + if "productName" in sdk and + sdk["productName"] == "macOS" and + "darwinos" not in sdk["canonicalName"] + ] + + # Now we'll get two entries here (one for internal and another one for public) + # It shouldn't matter which one we pick. + xcode_sdk_path = potential_sdks[0]["sdkPath"] + else: + xcode_sdk_path = "{}/SDKs/MacOSX.sdk".format(xcode_root) + return [ - "-isysroot {}/SDKs/MacOSX.sdk".format(xcode_root), + "-isysroot {}".format(xcode_sdk_path), ] def _get_toolchain_unix_cflags(rctx, python_interpreter, logger = None): @@ -158,19 +180,22 @@ def _create_repository_execution_environment(rctx, python_interpreter, logger = Dictionary of environment variable suitable to pass to rctx.execute. """ - # Gather any available CPPFLAGS values - cppflags = [] - cppflags.extend(_get_xcode_location_cflags(rctx)) - cppflags.extend(_get_toolchain_unix_cflags(rctx, python_interpreter, logger = logger)) - env = { "PYTHONPATH": pypi_repo_utils.construct_pythonpath( rctx, entries = rctx.attr._python_path_entries, ), - _CPPFLAGS: " ".join(cppflags), } + # Gather any available CPPFLAGS values + # + # We may want to build in an environment without a cc toolchain. + # In those cases, we're limited to --donwload-only, but we should respect that here. + if not rctx.attr.download_only: + cppflags = [] + cppflags.extend(_get_xcode_location_cflags(rctx)) + cppflags.extend(_get_toolchain_unix_cflags(rctx, python_interpreter)) + env[_CPPFLAGS] = " ".join(cppflags) return env def _whl_library_impl(rctx): From f4c5b611be5db293c4474151bf64e82d4eada108 Mon Sep 17 00:00:00 2001 From: Simon Mavi Stewart Date: Wed, 18 Dec 2024 15:45:43 +0000 Subject: [PATCH 2/5] Respond to review comments --- CHANGELOG.md | 1 + python/private/pypi/whl_library.bzl | 25 +++++++++++++++---------- 2 files changed, 16 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5583399e96..6e22c6e3df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -55,6 +55,7 @@ Unreleased changes template. * Bazel 6 support is dropped and Bazel 7.4.1 is the minimum supported version, per our Bazel support matrix. Earlier versions are not tested by CI, so functionality cannot be guaranteed. +* Use `xcrun xcodebuild --showsdks` to find XCode root {#v0-0-0-fixed} ### Fixed diff --git a/python/private/pypi/whl_library.bzl b/python/private/pypi/whl_library.bzl index 783a1407ad..b1b7fcfc59 100644 --- a/python/private/pypi/whl_library.bzl +++ b/python/private/pypi/whl_library.bzl @@ -55,14 +55,18 @@ def _get_xcode_location_cflags(rctx): # This is a full xcode installation somewhere like /Applications/Xcode13.0.app/Contents/Developer # so we need to change the path to to the macos specific tools which are in a different relative # path than xcode installed command line tools. - xcode_sdks_json = rctx.execute([ - "xcrun", - "xcodebuild", - "-showsdks", - "-json", - ], environment = { - "DEVELOPER_DIR": xcode_root, - }).stdout + xcode_sdks_json = repo_utils.execute_checked( + rctx, + op = "LocateXCodeSDKs", + arguments = [ + repo_utils.which_checked(rctx, "xcrun"), + "xcodebuild", + "-showsdks", + "-json", + ], + environment = { + "DEVELOPER_DIR": xcode_root, + }).stdout xcode_sdks = json.decode(xcode_sdks_json) potential_sdks = [ sdk @@ -190,8 +194,9 @@ def _create_repository_execution_environment(rctx, python_interpreter, logger = # Gather any available CPPFLAGS values # # We may want to build in an environment without a cc toolchain. - # In those cases, we're limited to --donwload-only, but we should respect that here. - if not rctx.attr.download_only: + # In those cases, we're limited to --download-only, but we should respect that here. + is_wheel = rctx.attr.filename and rctx.attr.filename.endswith(".whl") + if not (rctx.attr.download_only or is_wheel): cppflags = [] cppflags.extend(_get_xcode_location_cflags(rctx)) cppflags.extend(_get_toolchain_unix_cflags(rctx, python_interpreter)) From 707ba7df00e410331b74167f3f6edfb5aecce2f3 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 7 Mar 2025 05:44:35 +0000 Subject: [PATCH 3/5] buildifier --- python/private/pypi/whl_library.bzl | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/python/private/pypi/whl_library.bzl b/python/private/pypi/whl_library.bzl index 05e72e9caf..bfa31d6af7 100644 --- a/python/private/pypi/whl_library.bzl +++ b/python/private/pypi/whl_library.bzl @@ -66,7 +66,8 @@ def _get_xcode_location_cflags(rctx): ], environment = { "DEVELOPER_DIR": xcode_root, - }).stdout + }, + ).stdout xcode_sdks = json.decode(xcode_sdks_json) potential_sdks = [ sdk From cc3c9b4d1aec8919a907919b63f763cc1f7110c6 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 7 Mar 2025 05:48:57 +0000 Subject: [PATCH 4/5] pass logger to the newly added code --- python/private/pypi/whl_library.bzl | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/python/private/pypi/whl_library.bzl b/python/private/pypi/whl_library.bzl index bfa31d6af7..7bd85719f9 100644 --- a/python/private/pypi/whl_library.bzl +++ b/python/private/pypi/whl_library.bzl @@ -30,7 +30,7 @@ _CPPFLAGS = "CPPFLAGS" _COMMAND_LINE_TOOLS_PATH_SLUG = "commandlinetools" _WHEEL_ENTRY_POINT_PREFIX = "rules_python_wheel_entry_point" -def _get_xcode_location_cflags(rctx): +def _get_xcode_location_cflags(rctx, logger = None): """Query the xcode sdk location to update cflags Figure out if this interpreter target comes from rules_python, and patch the xcode sdk location if so. @@ -46,6 +46,7 @@ def _get_xcode_location_cflags(rctx): rctx, op = "GetXcodeLocation", arguments = [repo_utils.which_checked(rctx, "xcode-select"), "--print-path"], + logger = logger, ) if xcode_sdk_location.return_code != 0: return [] @@ -67,6 +68,7 @@ def _get_xcode_location_cflags(rctx): environment = { "DEVELOPER_DIR": xcode_root, }, + logger = logger, ).stdout xcode_sdks = json.decode(xcode_sdks_json) potential_sdks = [ @@ -110,6 +112,7 @@ def _get_toolchain_unix_cflags(rctx, python_interpreter, logger = None): "-c", "import sys; print(f'{sys.version_info[0]}.{sys.version_info[1]}', end='')", ], + logger = logger, ) _python_version = stdout include_path = "{}/include/python{}".format( @@ -199,8 +202,8 @@ def _create_repository_execution_environment(rctx, python_interpreter, logger = is_wheel = rctx.attr.filename and rctx.attr.filename.endswith(".whl") if not (rctx.attr.download_only or is_wheel): cppflags = [] - cppflags.extend(_get_xcode_location_cflags(rctx)) - cppflags.extend(_get_toolchain_unix_cflags(rctx, python_interpreter)) + cppflags.extend(_get_xcode_location_cflags(rctx, logger = logger)) + cppflags.extend(_get_toolchain_unix_cflags(rctx, python_interpreter, logger = logger)) env[_CPPFLAGS] = " ".join(cppflags) return env From 44e93556b5403c051a711171f26f3ee4de249c8c Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Tue, 25 Mar 2025 10:44:08 +0900 Subject: [PATCH 5/5] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 43b2e382a7..96bf33dbd5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -59,6 +59,7 @@ Unreleased changes template. * 3.11.11 * 3.12.9 * 3.13.2 +* (pypi) Use `xcrun xcodebuild --showsdks` to find XCode root. [20250317]: https://github.com/astral-sh/python-build-standalone/releases/tag/20250317 @@ -89,7 +90,6 @@ Unreleased changes template. * (deps) platforms 0.0.4 -> 0.0.11 * (py_wheel) Package `py_library.pyi_srcs` (`.pyi` files) in the wheel. * (py_package) Package `py_library.pyi_srcs` (`.pyi` files) in `py_package`. -* (pypi) Use `xcrun xcodebuild --showsdks` to find XCode root. * (gazelle) The generated manifest file (default: `gazelle_python.yaml`) will now include the YAML document start `---` line. Implemented in [#2656](https://github.com/bazel-contrib/rules_python/pull/2656).