From 201e2d4314394e484be7797283ddb8db33fe0bba Mon Sep 17 00:00:00 2001 From: Ethan Steinberg Date: Sun, 27 Apr 2025 08:41:48 -0700 Subject: [PATCH 1/8] fix: Make local runtime repo work for windows In particular, this fixes two problems: First, windows requires interface libraries to be linked in. Second, windows requires an alternative search path since LIBDIR is not filled in. --- python/private/get_local_runtime_info.py | 2 ++ python/private/local_runtime_repo.bzl | 9 +++++++++ python/private/local_runtime_repo_setup.bzl | 19 +++++++++++++++++++ 3 files changed, 30 insertions(+) diff --git a/python/private/get_local_runtime_info.py b/python/private/get_local_runtime_info.py index 19db3a2935..ac05af7d5b 100644 --- a/python/private/get_local_runtime_info.py +++ b/python/private/get_local_runtime_info.py @@ -36,6 +36,8 @@ # https://stackoverflow.com/questions/47423246/get-pythons-lib-path # For now, it seems LIBDIR has what is needed, so just use that. "LIBDIR", + # A backup alternative for deriving LIBDIR in cases that LIBDIR is not available + "LIBDEST", # The versioned libpythonX.Y.so.N file. Usually? # It might be a static archive (.a) file instead. "INSTSONAME", diff --git a/python/private/local_runtime_repo.bzl b/python/private/local_runtime_repo.bzl index ec0643e497..fc5197ce5f 100644 --- a/python/private/local_runtime_repo.bzl +++ b/python/private/local_runtime_repo.bzl @@ -112,6 +112,12 @@ def _local_runtime_repo_impl(rctx): info["INSTSONAME"], ] + if repo_utils.get_platforms_os_name(rctx) == 'windows': + shared_lib_names.append("python{major}{minor}.lib".format(**info)) + shared_lib_names.append("python3.lib") + + interpreter_path = interpreter_path.replace('\\', '/') + # In some cases, the value may be empty. Not clear why. shared_lib_names = [v for v in shared_lib_names if v] @@ -119,6 +125,9 @@ def _local_runtime_repo_impl(rctx): shared_lib_names = {v: None for v in shared_lib_names}.keys() shared_lib_dir = info["LIBDIR"] + if shared_lib_dir == None: + shared_lib_dir = info["LIBDEST"] + "/../libs" + # The specific files are symlinked instead of the whole directory # because it can point to a directory that has more than just # the Python runtime shared libraries, e.g. /usr/lib, or a Python diff --git a/python/private/local_runtime_repo_setup.bzl b/python/private/local_runtime_repo_setup.bzl index 37eab59575..2fe5b8f7aa 100644 --- a/python/private/local_runtime_repo_setup.bzl +++ b/python/private/local_runtime_repo_setup.bzl @@ -16,6 +16,7 @@ load("@bazel_skylib//lib:selects.bzl", "selects") load("@rules_cc//cc:cc_library.bzl", "cc_library") +load("@rules_cc//cc:cc_import.bzl", "cc_import") load("@rules_python//python:py_runtime.bzl", "py_runtime") load("@rules_python//python:py_runtime_pair.bzl", "py_runtime_pair") load("@rules_python//python/cc:py_cc_toolchain.bzl", "py_cc_toolchain") @@ -58,6 +59,20 @@ def define_local_runtime_toolchain_impl( major_minor = "{}.{}".format(major, minor) major_minor_micro = "{}.{}".format(major_minor, micro) + version_dict = {'major': major, 'minor': minor} + + cc_import( + name = "interface", + interface_library = "lib/python{major}{minor}.lib".format(**version_dict), + system_provided = True, + ) + + cc_import( + name = "abi3_interface", + interface_library = "lib/python3.lib", + system_provided = True, + ) + cc_library( name = "_python_headers", # NOTE: Keep in sync with watch_tree() called in local_runtime_repo @@ -67,6 +82,10 @@ def define_local_runtime_toolchain_impl( allow_empty = True, ), includes = ["include"], + deps = select({ + "@bazel_tools//src/conditions:windows": [":interface", ":abi3_interface"], + "//conditions:default": None, + }), ) cc_library( From 43a4f5992243ec1ecd14f219c81e9f4e81c6ecea Mon Sep 17 00:00:00 2001 From: Ethan Steinberg Date: Tue, 6 May 2025 07:52:25 -0700 Subject: [PATCH 2/8] Update get_local_runtime_info.py --- python/private/get_local_runtime_info.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/private/get_local_runtime_info.py b/python/private/get_local_runtime_info.py index ac05af7d5b..ea3f075238 100644 --- a/python/private/get_local_runtime_info.py +++ b/python/private/get_local_runtime_info.py @@ -36,7 +36,7 @@ # https://stackoverflow.com/questions/47423246/get-pythons-lib-path # For now, it seems LIBDIR has what is needed, so just use that. "LIBDIR", - # A backup alternative for deriving LIBDIR in cases that LIBDIR is not available + # A backup alternative for deriving LIBDIR when LIBDIR is not available "LIBDEST", # The versioned libpythonX.Y.so.N file. Usually? # It might be a static archive (.a) file instead. From 4c440a7788198840b2725fe3302f09172fc9ce98 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 6 May 2025 18:57:22 -0700 Subject: [PATCH 3/8] Apply suggestions from code review --- python/private/get_local_runtime_info.py | 3 ++- python/private/local_runtime_repo_setup.bzl | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/python/private/get_local_runtime_info.py b/python/private/get_local_runtime_info.py index ea3f075238..eeca34c3a4 100644 --- a/python/private/get_local_runtime_info.py +++ b/python/private/get_local_runtime_info.py @@ -36,7 +36,8 @@ # https://stackoverflow.com/questions/47423246/get-pythons-lib-path # For now, it seems LIBDIR has what is needed, so just use that. "LIBDIR", - # A backup alternative for deriving LIBDIR when LIBDIR is not available + # LIBDEST is the directory for platform-specific files; it's usually + # a subdirectory of LIBDIR. We use it in case LIBDIR isn't set. "LIBDEST", # The versioned libpythonX.Y.so.N file. Usually? # It might be a static archive (.a) file instead. diff --git a/python/private/local_runtime_repo_setup.bzl b/python/private/local_runtime_repo_setup.bzl index 2fe5b8f7aa..f7d09ef1f2 100644 --- a/python/private/local_runtime_repo_setup.bzl +++ b/python/private/local_runtime_repo_setup.bzl @@ -83,7 +83,7 @@ def define_local_runtime_toolchain_impl( ), includes = ["include"], deps = select({ - "@bazel_tools//src/conditions:windows": [":interface", ":abi3_interface"], + "@platforms//os:windows": [":interface", ":abi3_interface"], "//conditions:default": None, }), ) From 0bab65585d365aff541a6a3b4fb1abbf11b01d39 Mon Sep 17 00:00:00 2001 From: Ethan Steinberg Date: Tue, 6 May 2025 20:54:23 -0700 Subject: [PATCH 4/8] A variety of improvements --- CHANGELOG.md | 1 + python/private/get_local_runtime_info.py | 3 +++ python/private/local_runtime_repo.bzl | 16 +++++++++++----- python/private/local_runtime_repo_setup.bzl | 2 +- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d73613a07..15b3a3ea17 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -78,6 +78,7 @@ END_UNRELEASED_TEMPLATE multiple times. * (tools/wheelmaker.py) Extras are now preserved in Requires-Dist metadata when using requires_file to specify the requirements. +* (toolchains) Local toolchains now supports Windows. {#v0-0-0-added} ### Added diff --git a/python/private/get_local_runtime_info.py b/python/private/get_local_runtime_info.py index eeca34c3a4..57a5a4d33f 100644 --- a/python/private/get_local_runtime_info.py +++ b/python/private/get_local_runtime_info.py @@ -48,6 +48,9 @@ # The platform-specific filename suffix for library files. # Includes the dot, e.g. `.so` "SHLIB_SUFFIX", + # A flag for whether this is a free threaded implementation. + # Set to t when free threading is enabled, '' or None otherwise. + "abi_thread", ] data.update(zip(config_vars, sysconfig.get_config_vars(*config_vars))) print(json.dumps(data)) diff --git a/python/private/local_runtime_repo.bzl b/python/private/local_runtime_repo.bzl index fc5197ce5f..2eb2d8c81e 100644 --- a/python/private/local_runtime_repo.bzl +++ b/python/private/local_runtime_repo.bzl @@ -14,6 +14,7 @@ """Create a repository for a locally installed Python runtime.""" +load("@bazel_skylib//lib:paths.bzl", "paths") load(":enum.bzl", "enum") load(":repo_utils.bzl", "REPO_DEBUG_ENV_VAR", "repo_utils") @@ -112,11 +113,16 @@ def _local_runtime_repo_impl(rctx): info["INSTSONAME"], ] - if repo_utils.get_platforms_os_name(rctx) == 'windows': - shared_lib_names.append("python{major}{minor}.lib".format(**info)) - shared_lib_names.append("python3.lib") + if repo_utils.get_platforms_os_name(rctx) == "windows": + # Fall back to assuming no threading when no information is available + if info.get("abi_thread") == None: + info["abi_thread"] = "" - interpreter_path = interpreter_path.replace('\\', '/') + # Windows requires these library definitions to be linked in with the headers + shared_lib_names.append("python{major}{minor}{abi_thread}.lib".format(**info)) + shared_lib_names.append("python{major}{abi_thread}.lib".format(**info)) + + interpreter_path = interpreter_path.replace("\\", "/") # In some cases, the value may be empty. Not clear why. shared_lib_names = [v for v in shared_lib_names if v] @@ -126,7 +132,7 @@ def _local_runtime_repo_impl(rctx): shared_lib_dir = info["LIBDIR"] if shared_lib_dir == None: - shared_lib_dir = info["LIBDEST"] + "/../libs" + shared_lib_dir = paths.dirname(info["LIBDEST"]) + "/libs" # The specific files are symlinked instead of the whole directory # because it can point to a directory that has more than just diff --git a/python/private/local_runtime_repo_setup.bzl b/python/private/local_runtime_repo_setup.bzl index f7d09ef1f2..c9798fa34e 100644 --- a/python/private/local_runtime_repo_setup.bzl +++ b/python/private/local_runtime_repo_setup.bzl @@ -59,7 +59,7 @@ def define_local_runtime_toolchain_impl( major_minor = "{}.{}".format(major, minor) major_minor_micro = "{}.{}".format(major_minor, micro) - version_dict = {'major': major, 'minor': minor} + version_dict = {"major": major, "minor": minor} cc_import( name = "interface", From 23701d72e6fc8e878ff663cc51fd6f8da9ade729 Mon Sep 17 00:00:00 2001 From: Ethan Steinberg Date: Tue, 6 May 2025 20:56:52 -0700 Subject: [PATCH 5/8] Oops, missing dependency --- .gitignore | 12 ++++---- python/private/BUILD.bazel | 1 + python/private/local_runtime_repo.bzl | 30 +++++++++---------- .../integration/local_toolchains/MODULE.bazel | 2 +- tests/integration/local_toolchains/test.py | 2 +- 5 files changed, 24 insertions(+), 23 deletions(-) diff --git a/.gitignore b/.gitignore index 863b0e9c3f..5a34cd4503 100644 --- a/.gitignore +++ b/.gitignore @@ -32,11 +32,11 @@ *~ # Bazel directories -/bazel-* -/bazel-bin -/bazel-genfiles -/bazel-out -/bazel-testlogs +bazel-* +bazel-bin +bazel-genfiles +bazel-out +bazel-testlogs user.bazelrc # vim swap files @@ -51,4 +51,4 @@ user.bazelrc # MODULE.bazel.lock is ignored for now as per recommendation from upstream. # See https://github.com/bazelbuild/bazel/issues/20369 -MODULE.bazel.lock +MODULE.bazel.lock \ No newline at end of file diff --git a/python/private/BUILD.bazel b/python/private/BUILD.bazel index 9cc8ffc62c..34273898df 100644 --- a/python/private/BUILD.bazel +++ b/python/private/BUILD.bazel @@ -212,6 +212,7 @@ bzl_library( deps = [ ":enum_bzl", ":repo_utils.bzl", + "@bazel_skylib//lib:paths", ], ) diff --git a/python/private/local_runtime_repo.bzl b/python/private/local_runtime_repo.bzl index 2eb2d8c81e..41f0c3b7ed 100644 --- a/python/private/local_runtime_repo.bzl +++ b/python/private/local_runtime_repo.bzl @@ -25,21 +25,21 @@ _OnFailure = enum( FAIL = "fail", ) -_TOOLCHAIN_IMPL_TEMPLATE = """\ -# Generated by python/private/local_runtime_repo.bzl - -load("@rules_python//python/private:local_runtime_repo_setup.bzl", "define_local_runtime_toolchain_impl") - -define_local_runtime_toolchain_impl( - name = "local_runtime", - lib_ext = "{lib_ext}", - major = "{major}", - minor = "{minor}", - micro = "{micro}", - interpreter_path = "{interpreter_path}", - implementation_name = "{implementation_name}", - os = "{os}", -) +_TOOLCHAIN_IMPL_TEMPLATE = """\\\r +# Generated by python/private/local_runtime_repo.bzl\r +\r +load("@rules_python//python/private:local_runtime_repo_setup.bzl", "define_local_runtime_toolchain_impl")\r +\r +define_local_runtime_toolchain_impl(\r + name = "local_runtime",\r + lib_ext = "{lib_ext}",\r + major = "{major}",\r + minor = "{minor}",\r + micro = "{micro}",\r + interpreter_path = "{interpreter_path}",\r + implementation_name = "{implementation_name}",\r + os = "{os}",\r +)\r """ def _local_runtime_repo_impl(rctx): diff --git a/tests/integration/local_toolchains/MODULE.bazel b/tests/integration/local_toolchains/MODULE.bazel index 6c06909cd7..c39a7bfe1d 100644 --- a/tests/integration/local_toolchains/MODULE.bazel +++ b/tests/integration/local_toolchains/MODULE.bazel @@ -28,7 +28,7 @@ local_runtime_toolchains_repo = use_repo_rule("@rules_python//python/local_toolc local_runtime_repo( name = "local_python3", - interpreter_path = "python3", + interpreter_path = "python", on_failure = "fail", ) diff --git a/tests/integration/local_toolchains/test.py b/tests/integration/local_toolchains/test.py index 8e37fff652..7f70eadec2 100644 --- a/tests/integration/local_toolchains/test.py +++ b/tests/integration/local_toolchains/test.py @@ -14,7 +14,7 @@ def test_python_from_path_used(self): # repo-phase and when the test is run are roughly the same. It's # easy to violate this condition if there are shell-local changes # that wouldn't be reflected when sub-shells are run later. - shell_path = shutil.which("python3") + shell_path = shutil.which("python") # We call the interpreter and print its executable because of # things like pyenv: they install a shim that re-execs python. From 009e3de7cf261321bea9ab1ac5d7adc8d6a332f6 Mon Sep 17 00:00:00 2001 From: Ethan Steinberg Date: Tue, 6 May 2025 21:00:15 -0700 Subject: [PATCH 6/8] Revert bad changes --- python/private/local_runtime_repo.bzl | 30 +++++++++---------- .../integration/local_toolchains/MODULE.bazel | 2 +- tests/integration/local_toolchains/test.py | 2 +- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/python/private/local_runtime_repo.bzl b/python/private/local_runtime_repo.bzl index 41f0c3b7ed..7eb4d5f0d1 100644 --- a/python/private/local_runtime_repo.bzl +++ b/python/private/local_runtime_repo.bzl @@ -25,21 +25,21 @@ _OnFailure = enum( FAIL = "fail", ) -_TOOLCHAIN_IMPL_TEMPLATE = """\\\r -# Generated by python/private/local_runtime_repo.bzl\r -\r -load("@rules_python//python/private:local_runtime_repo_setup.bzl", "define_local_runtime_toolchain_impl")\r -\r -define_local_runtime_toolchain_impl(\r - name = "local_runtime",\r - lib_ext = "{lib_ext}",\r - major = "{major}",\r - minor = "{minor}",\r - micro = "{micro}",\r - interpreter_path = "{interpreter_path}",\r - implementation_name = "{implementation_name}",\r - os = "{os}",\r -)\r +_TOOLCHAIN_IMPL_TEMPLATE = """\\ +# Generated by python/private/local_runtime_repo.bzl + +load("@rules_python//python/private:local_runtime_repo_setup.bzl", "define_local_runtime_toolchain_impl") + +define_local_runtime_toolchain_impl( + name = "local_runtime", + lib_ext = "{lib_ext}", + major = "{major}", + minor = "{minor}", + micro = "{micro}", + interpreter_path = "{interpreter_path}", + implementation_name = "{implementation_name}", + os = "{os}", +) """ def _local_runtime_repo_impl(rctx): diff --git a/tests/integration/local_toolchains/MODULE.bazel b/tests/integration/local_toolchains/MODULE.bazel index c39a7bfe1d..6c06909cd7 100644 --- a/tests/integration/local_toolchains/MODULE.bazel +++ b/tests/integration/local_toolchains/MODULE.bazel @@ -28,7 +28,7 @@ local_runtime_toolchains_repo = use_repo_rule("@rules_python//python/local_toolc local_runtime_repo( name = "local_python3", - interpreter_path = "python", + interpreter_path = "python3", on_failure = "fail", ) diff --git a/tests/integration/local_toolchains/test.py b/tests/integration/local_toolchains/test.py index 7f70eadec2..8e37fff652 100644 --- a/tests/integration/local_toolchains/test.py +++ b/tests/integration/local_toolchains/test.py @@ -14,7 +14,7 @@ def test_python_from_path_used(self): # repo-phase and when the test is run are roughly the same. It's # easy to violate this condition if there are shell-local changes # that wouldn't be reflected when sub-shells are run later. - shell_path = shutil.which("python") + shell_path = shutil.which("python3") # We call the interpreter and print its executable because of # things like pyenv: they install a shim that re-execs python. From 1b1960552857bf5763e5afef5adbeb04f86bb95e Mon Sep 17 00:00:00 2001 From: Ethan Steinberg Date: Tue, 6 May 2025 21:00:58 -0700 Subject: [PATCH 7/8] One final bad change --- python/private/local_runtime_repo.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/private/local_runtime_repo.bzl b/python/private/local_runtime_repo.bzl index 7eb4d5f0d1..2eb2d8c81e 100644 --- a/python/private/local_runtime_repo.bzl +++ b/python/private/local_runtime_repo.bzl @@ -25,7 +25,7 @@ _OnFailure = enum( FAIL = "fail", ) -_TOOLCHAIN_IMPL_TEMPLATE = """\\ +_TOOLCHAIN_IMPL_TEMPLATE = """\ # Generated by python/private/local_runtime_repo.bzl load("@rules_python//python/private:local_runtime_repo_setup.bzl", "define_local_runtime_toolchain_impl") From a4430d70d743d3a46c15a0b65f420b306dddc646 Mon Sep 17 00:00:00 2001 From: Ethan Steinberg Date: Tue, 6 May 2025 21:07:44 -0700 Subject: [PATCH 8/8] Fix order --- python/private/local_runtime_repo_setup.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/private/local_runtime_repo_setup.bzl b/python/private/local_runtime_repo_setup.bzl index c9798fa34e..96f8571dff 100644 --- a/python/private/local_runtime_repo_setup.bzl +++ b/python/private/local_runtime_repo_setup.bzl @@ -15,8 +15,8 @@ """Setup code called by the code generated by `local_runtime_repo`.""" load("@bazel_skylib//lib:selects.bzl", "selects") -load("@rules_cc//cc:cc_library.bzl", "cc_library") load("@rules_cc//cc:cc_import.bzl", "cc_import") +load("@rules_cc//cc:cc_library.bzl", "cc_library") load("@rules_python//python:py_runtime.bzl", "py_runtime") load("@rules_python//python:py_runtime_pair.bzl", "py_runtime_pair") load("@rules_python//python/cc:py_cc_toolchain.bzl", "py_cc_toolchain")