From d5a595c6f2b95b5174b1373ed09b3be2740ce869 Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Sat, 30 Nov 2024 09:50:12 +0100 Subject: [PATCH] fix: Add `libs/python3.lib` to libpython target for SABI builds on Windows (#1820) When targeting the Python Stable ABI on Windows (by setting the Py_LIMITED_API macro to a Python minimum version hex), the unversioned python3.lib needs to be linked instead of the versioned one (e.g. python38.lib for Python 3.8). Python's own config sets the library to link by default in a header called pyconfig.h (https://github.com/python/cpython/blob/9cc9e277254023c0ca08e1a9e379fd89475ca9c2/PC/pyconfig.h#L270), which prompts the linker to search for python3.lib if a stable ABI extension is built using `@rules_python` toolchains. Since this library is not exported on Windows in the `python_repository()` rule, building Python C++ extensions with rules_python toolchains fails in the linking step, because the library is never copied. Consequently, it is added now to allow Python SABI extensions to be built (and linked) on Windows with `@rules_python`. Since Python takes responsibility for linking the correct lib on Windows, and never both at the same time, no other changes are made. --------- Co-authored-by: Richard Levasseur Co-authored-by: Richard Levasseur --- CHANGELOG.md | 3 +++ .../private/hermetic_runtime_repo_setup.bzl | 21 ++++++++++++++++--- tests/cc/current_py_cc_libs/BUILD.bazel | 18 ++++++++++++++++ .../python_libs_linking_test.cc | 2 +- 4 files changed, 40 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cc66afa510..37a9e710a1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -95,6 +95,9 @@ Other changes: ([2169](https://github.com/bazelbuild/rules_python/issues/2169)). * (workspace) Corrected protobuf's name to com_google_protobuf, the name is hardcoded in Bazel, WORKSPACE mode. +* (repositories): Add libs/python3.lib and pythonXY.dll to the `libpython` target + defined by a repository template. This enables stable ABI builds of Python extensions + on Windows (by defining Py_LIMITED_API). {#v0-0-0-added} ### Added diff --git a/python/private/hermetic_runtime_repo_setup.bzl b/python/private/hermetic_runtime_repo_setup.bzl index d041be5964..64d721ecad 100644 --- a/python/private/hermetic_runtime_repo_setup.bzl +++ b/python/private/hermetic_runtime_repo_setup.bzl @@ -89,6 +89,14 @@ def define_hermetic_runtime_toolchain_impl( }), system_provided = True, ) + cc_import( + name = "abi3_interface", + interface_library = select({ + _IS_FREETHREADED: "libs/python3t.lib", + "//conditions:default": "libs/python3.lib", + }), + system_provided = True, + ) native.filegroup( name = "includes", @@ -97,7 +105,7 @@ def define_hermetic_runtime_toolchain_impl( cc_library( name = "python_headers", deps = select({ - "@bazel_tools//src/conditions:windows": [":interface"], + "@bazel_tools//src/conditions:windows": [":interface", ":abi3_interface"], "//conditions:default": None, }), hdrs = [":includes"], @@ -156,15 +164,22 @@ def define_hermetic_runtime_toolchain_impl( "lib/libpython{major}.{minor}t.dylib".format(**version_dict), ], ":is_freethreaded_windows": [ - "python3.dll", + "python3t.dll", + "python{major}{minor}t.dll".format(**version_dict), "libs/python{major}{minor}t.lib".format(**version_dict), + "libs/python3t.lib", ], "@platforms//os:linux": [ "lib/libpython{major}.{minor}.so".format(**version_dict), "lib/libpython{major}.{minor}.so.1.0".format(**version_dict), ], "@platforms//os:macos": ["lib/libpython{major}.{minor}.dylib".format(**version_dict)], - "@platforms//os:windows": ["python3.dll", "libs/python{major}{minor}.lib".format(**version_dict)], + "@platforms//os:windows": [ + "python3.dll", + "python{major}{minor}.dll".format(**version_dict), + "libs/python{major}{minor}.lib".format(**version_dict), + "libs/python3.lib", + ], }), ) diff --git a/tests/cc/current_py_cc_libs/BUILD.bazel b/tests/cc/current_py_cc_libs/BUILD.bazel index 218055532e..9f335990e6 100644 --- a/tests/cc/current_py_cc_libs/BUILD.bazel +++ b/tests/cc/current_py_cc_libs/BUILD.bazel @@ -33,3 +33,21 @@ cc_test( "@rules_python//python/cc:current_py_cc_libs", ], ) + +# This is technically a headers test, but since the pyconfig.h header +# designates the appropriate lib to link on Win+MSVC, this test verifies that +# the expected Windows libraries are all present in the expected location. +# Since we define the Py_LIMITED_API macro, we expect the linker to go search +# for libs/python3.lib. +# buildifier: disable=native-cc +cc_test( + name = "python_libs_linking_windows_test", + srcs = ["python_libs_linking_test.cc"], + defines = ["Py_LIMITED_API=0x030A0000"], + env = {"HELLO": "world"}, + target_compatible_with = ["@platforms//os:windows"], + deps = [ + "@rules_python//python/cc:current_py_cc_headers", + "@rules_python//python/cc:current_py_cc_libs", + ], +) diff --git a/tests/cc/current_py_cc_libs/python_libs_linking_test.cc b/tests/cc/current_py_cc_libs/python_libs_linking_test.cc index 1ecce088b6..2f26a2c597 100644 --- a/tests/cc/current_py_cc_libs/python_libs_linking_test.cc +++ b/tests/cc/current_py_cc_libs/python_libs_linking_test.cc @@ -12,7 +12,7 @@ int main(int argc, char** argv) { // To make it actually run, more custom initialization is necessary. // See https://docs.python.org/3/c-api/intro.html#embedding-python Py_Initialize(); - PyRun_SimpleString("print('Hello, world')\n"); + Py_BytesMain(argc, argv); Py_Finalize(); return 0; }