From 2ee0e92b9f8cfe327675ae7c7dc60e4be62a921d Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Fri, 22 Mar 2024 15:17:07 +0100 Subject: [PATCH 1/9] Add libs/python3.lib to libpython target for SABI builds on Windows 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, it's 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. --- CHANGELOG.md | 3 +++ python/repositories.bzl | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f8e273cd2..3f9e9c525d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,9 @@ A brief description of the categories of changes: `pip.parse` extension is now possible, see the `examples/pip_parse/MODULE.bazel` for how to do it. See [#1371](https://github.com/bazelbuild/rules_python/issues/1371). +* (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). ### Added diff --git a/python/repositories.bzl b/python/repositories.bzl index aab68eb086..183e17fcfb 100644 --- a/python/repositories.bzl +++ b/python/repositories.bzl @@ -319,7 +319,7 @@ cc_library( name = "libpython", hdrs = [":includes"], srcs = select({{ - "@platforms//os:windows": ["python3.dll", "libs/python{python_version_nodot}.lib"], + "@platforms//os:windows": ["python3.dll", "python{python_version_nodot}.dll", "libs/python3.lib", "libs/python{python_version_nodot}.lib"], "@platforms//os:macos": ["lib/libpython{python_version}.dylib"], "@platforms//os:linux": ["lib/libpython{python_version}.so", "lib/libpython{python_version}.so.1.0"], }}), From 26af5b5a3c8171ab9c3cb6ce775804fef49254a5 Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Thu, 28 Mar 2024 20:23:21 +0100 Subject: [PATCH 2/9] Add Windows libs test to current_py_cc_libs tests Reuses the existing cc_test targeting the stable ABI on Windows. Since this prompts MSVC to search for libs/python3.lib, it serves as a check that the library search path is intact on Windows. --- tests/cc/current_py_cc_libs/BUILD.bazel | 16 ++++++++++++++++ .../python_libs_linking_test.cc | 1 - 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/cc/current_py_cc_libs/BUILD.bazel b/tests/cc/current_py_cc_libs/BUILD.bazel index fb61435d37..8b7e46e121 100644 --- a/tests/cc/current_py_cc_libs/BUILD.bazel +++ b/tests/cc/current_py_cc_libs/BUILD.bazel @@ -31,3 +31,19 @@ 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"], + target_compatible_with = ["@platforms//os:windows"], + deps = [ + "@rules_python//python/cc:current_py_cc_headers", + ], +) 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..e351d6000b 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,6 @@ 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_Finalize(); return 0; } From c567f70ff2c7d0049c40982a05de06c69db7bdb3 Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Fri, 5 Apr 2024 19:31:58 +0200 Subject: [PATCH 3/9] Add cc_import for ABI3-compatible unversioned Windows lib This could potentially fix the header test. --- python/repositories.bzl | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/python/repositories.bzl b/python/repositories.bzl index 183e17fcfb..c4f964233d 100644 --- a/python/repositories.bzl +++ b/python/repositories.bzl @@ -296,6 +296,12 @@ cc_import( system_provided = True, ) +cc_import( + name = "abi3_interface", + interface_library = "libs/python3.lib", + system_provided = True, +) + filegroup( name = "includes", srcs = glob(["include/**/*.h"]), @@ -304,7 +310,7 @@ filegroup( 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"], From 5f4179e9e91661415bf7f5f502d92e5f3f05d872 Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Mon, 8 Apr 2024 20:39:01 +0200 Subject: [PATCH 4/9] Use envvar instead of argc for early return --- tests/cc/current_py_cc_libs/BUILD.bazel | 1 + tests/cc/current_py_cc_libs/python_libs_linking_test.cc | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/cc/current_py_cc_libs/BUILD.bazel b/tests/cc/current_py_cc_libs/BUILD.bazel index 8b7e46e121..995539ba1b 100644 --- a/tests/cc/current_py_cc_libs/BUILD.bazel +++ b/tests/cc/current_py_cc_libs/BUILD.bazel @@ -42,6 +42,7 @@ 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", 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 e351d6000b..f3221ad8be 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 @@ -1,8 +1,10 @@ +#include #include int main(int argc, char** argv) { // Early return to prevent the broken code below from running. - if (argc >= 1) { + char* val = std::getenv("HELLO"); + if (val || argc >= 1) { return 0; } From 1d87f50fba8ae846cad680573e24b60126b8a1fc Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Fri, 29 Nov 2024 15:40:16 +0100 Subject: [PATCH 5/9] Fix free-threaded DLL name, add ABI3 lib to free-threaded libpython The former was probably an oversight, the latter is a consequence of the addition of free-threading, which wasn't there when this PR was first opened. --- python/private/hermetic_runtime_repo_setup.bzl | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/python/private/hermetic_runtime_repo_setup.bzl b/python/private/hermetic_runtime_repo_setup.bzl index 5dd31ddfda..fd58cb3546 100644 --- a/python/private/hermetic_runtime_repo_setup.bzl +++ b/python/private/hermetic_runtime_repo_setup.bzl @@ -91,7 +91,10 @@ def define_hermetic_runtime_toolchain_impl( ) cc_import( name = "abi3_interface", - interface_library = "libs/python3.lib", + interface_library = select({ + _IS_FREETHREADED: "libs/python3t.lib", + "//conditions:default": "libs/python3.lib", + }), system_provided = True, ) @@ -161,8 +164,10 @@ def define_hermetic_runtime_toolchain_impl( "lib/libpython{major}.{minor}t.dylib".format(**version_dict), ], ":is_freethreaded_windows": [ - "python3.dll", + "python3t.dll", + "python{python_version_nodot}t.dll", "libs/python{major}{minor}t.lib".format(**version_dict), + "libs/python3t.lib", ], "@platforms//os:linux": [ "lib/libpython{major}.{minor}.so".format(**version_dict), @@ -174,7 +179,7 @@ def define_hermetic_runtime_toolchain_impl( "python{python_version_nodot}.dll", "libs/python{major}{minor}.lib".format(**version_dict), "libs/python3.lib", - ], + ], }), ) From 3213ae85d1e0d0b3eaa8df1b0192556f04df1c10 Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Fri, 29 Nov 2024 15:54:35 +0100 Subject: [PATCH 6/9] Try again to create a test that actually runs with only SABI functions --- tests/cc/current_py_cc_libs/python_libs_linking_test.cc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) 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 f3221ad8be..64d7e5ae0a 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 @@ -1,10 +1,11 @@ -#include #include int main(int argc, char** argv) { + + PyObject *s = PyUnicode_FromString(argv[0]); // Early return to prevent the broken code below from running. - char* val = std::getenv("HELLO"); - if (val || argc >= 1) { + if (argc >= 1) { + printf("%d\n", (int)s->ob_refcnt); return 0; } From e23a7f9a8d109397b71784bd7c6ff03e48d4f0a2 Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Fri, 29 Nov 2024 16:03:23 +0100 Subject: [PATCH 7/9] Use format() with version dict instead of python_version_nodot --- python/private/hermetic_runtime_repo_setup.bzl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/private/hermetic_runtime_repo_setup.bzl b/python/private/hermetic_runtime_repo_setup.bzl index fd58cb3546..64d721ecad 100644 --- a/python/private/hermetic_runtime_repo_setup.bzl +++ b/python/private/hermetic_runtime_repo_setup.bzl @@ -165,7 +165,7 @@ def define_hermetic_runtime_toolchain_impl( ], ":is_freethreaded_windows": [ "python3t.dll", - "python{python_version_nodot}t.dll", + "python{major}{minor}t.dll".format(**version_dict), "libs/python{major}{minor}t.lib".format(**version_dict), "libs/python3t.lib", ], @@ -176,7 +176,7 @@ def define_hermetic_runtime_toolchain_impl( "@platforms//os:macos": ["lib/libpython{major}.{minor}.dylib".format(**version_dict)], "@platforms//os:windows": [ "python3.dll", - "python{python_version_nodot}.dll", + "python{major}{minor}.dll".format(**version_dict), "libs/python{major}{minor}.lib".format(**version_dict), "libs/python3.lib", ], From 8c5825341c0e5f036da499d5fe7118f2fffe9363 Mon Sep 17 00:00:00 2001 From: Nicholas Junge Date: Fri, 29 Nov 2024 16:53:32 +0100 Subject: [PATCH 8/9] Explicitly pass the libs, refactor cc file --- tests/cc/current_py_cc_libs/BUILD.bazel | 1 + tests/cc/current_py_cc_libs/python_libs_linking_test.cc | 5 +++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/cc/current_py_cc_libs/BUILD.bazel b/tests/cc/current_py_cc_libs/BUILD.bazel index 2596637c0c..9f335990e6 100644 --- a/tests/cc/current_py_cc_libs/BUILD.bazel +++ b/tests/cc/current_py_cc_libs/BUILD.bazel @@ -48,5 +48,6 @@ cc_test( 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 64d7e5ae0a..cc09e66651 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 @@ -2,10 +2,10 @@ int main(int argc, char** argv) { - PyObject *s = PyUnicode_FromString(argv[0]); + // PyObject *s = PyUnicode_FromString(argv[0]); // Early return to prevent the broken code below from running. if (argc >= 1) { - printf("%d\n", (int)s->ob_refcnt); + // printf("%d\n", (int)s->ob_refcnt); return 0; } @@ -15,6 +15,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(); + Py_BytesMain(argc, argv); Py_Finalize(); return 0; } From f3c5c887f8a06dc38a073f0c90084c00f5346643 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Sat, 30 Nov 2024 00:34:52 -0800 Subject: [PATCH 9/9] Apply suggestions from code review --- tests/cc/current_py_cc_libs/python_libs_linking_test.cc | 3 --- 1 file changed, 3 deletions(-) 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 cc09e66651..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 @@ -1,11 +1,8 @@ #include int main(int argc, char** argv) { - - // PyObject *s = PyUnicode_FromString(argv[0]); // Early return to prevent the broken code below from running. if (argc >= 1) { - // printf("%d\n", (int)s->ob_refcnt); return 0; }