From 0fefc888937682312f04f7d3219362d98e1fdfdf Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 22 Sep 2025 19:54:42 -0700 Subject: [PATCH 01/35] empty impl --- python/cc/py_extension.bzl | 8 ++++++++ python/private/cc/py_extension_macro.bzl | 13 +++++++++++++ python/private/cc/py_extension_rule.bzl | 8 ++++++++ 3 files changed, 29 insertions(+) create mode 100644 python/cc/py_extension.bzl create mode 100644 python/private/cc/py_extension_macro.bzl create mode 100644 python/private/cc/py_extension_rule.bzl diff --git a/python/cc/py_extension.bzl b/python/cc/py_extension.bzl new file mode 100644 index 0000000000..72a9ce843c --- /dev/null +++ b/python/cc/py_extension.bzl @@ -0,0 +1,8 @@ +"""Public API for py_extension.""" + +load( + "//python/private/cc:py_extension_macro.bzl", + _py_extension = "py_extension", +) + +py_extension = _py_extension diff --git a/python/private/cc/py_extension_macro.bzl b/python/private/cc/py_extension_macro.bzl new file mode 100644 index 0000000000..98932f9fe3 --- /dev/null +++ b/python/private/cc/py_extension_macro.bzl @@ -0,0 +1,13 @@ +"""Wrapper macro for the py_extension rule.""" + +load(":py_extension_rule.bzl", _py_extension = "py_extension") +load("//python/private:util.bzl", "add_tag") + +def py_extension(**kwargs): + """A macro that calls the py_extension rule and adds a tag. + + Args: + **kwargs: Additional arguments to pass to the rule. + """ + add_tag(kwargs, "@rules_python//python/cc:py_extension") + _py_extension(**kwargs) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl new file mode 100644 index 0000000000..4a8baee0b6 --- /dev/null +++ b/python/private/cc/py_extension_rule.bzl @@ -0,0 +1,8 @@ +"""Implementation of the py_extension rule.""" + +def _py_extension_impl(ctx): + pass + +py_extension = rule( + implementation = _py_extension_impl, +) From 02a9dc1f41af2a46597244a2b14e81855cb9d4e4 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 22 Sep 2025 20:38:08 -0700 Subject: [PATCH 02/35] basic attr builder usage --- python/private/cc/py_extension_rule.bzl | 35 +++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 4a8baee0b6..8740296c84 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -1,8 +1,39 @@ """Implementation of the py_extension rule.""" +load("@rules_cc//cc/common:cc_info.bzl", "CcInfo") +load("//python/private:attr_builders.bzl", "attrb") +load("//python/private:attributes.bzl", "COMMON_ATTRS") +load("//python/private:py_info.bzl", "PyInfo") +load("//python/private:reexports.bzl", "BuiltinPyInfo") +load("//python/private:rule_builders.bzl", "ruleb") +load("//python/private:toolchain_types.bzl", "TARGET_TOOLCHAIN_TYPE") + def _py_extension_impl(ctx): pass -py_extension = rule( +_MaybeBuiltinPyInfo = [[BuiltinPyInfo]] if BuiltinPyInfo != None else [] + +PY_EXTENSION_ATTRS = COMMON_ATTRS | { + "srcs": lambda: attrb.LabelList( + allow_files = True, + doc = "The list of source files that are processed to create the target.", + ), +} + +def create_py_extension_rule_builder(implementation, **kwargs): + """Create a rule builder for a py_extension.""" + builder = ruleb.Rule( + implementation = implementation, + attrs = PY_EXTENSION_ATTRS, + provides = [PyInfo, CcInfo], + toolchains = [ + ruleb.ToolchainType(TARGET_TOOLCHAIN_TYPE), + ruleb.ToolchainType("@bazel_tools//tools/cpp:toolchain_type"), + ], + **kwargs + ) + return builder + +py_extension = create_py_extension_rule_builder( implementation = _py_extension_impl, -) +).build() From bd16c3c2bd78580b3427acc1471a395c8a835dee Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 22 Sep 2025 20:46:33 -0700 Subject: [PATCH 03/35] add stub tests --- python/private/cc/py_extension_rule.bzl | 8 ++-- tests/cc/py_extension/BUILD.bazel | 29 ++++++++++++++ tests/cc/py_extension/ext.c | 1 + tests/cc/py_extension/py_extension_test.py | 10 +++++ tests/cc/py_extension/py_extension_tests.bzl | 40 ++++++++++++++++++++ 5 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 tests/cc/py_extension/BUILD.bazel create mode 100644 tests/cc/py_extension/ext.c create mode 100644 tests/cc/py_extension/py_extension_test.py create mode 100644 tests/cc/py_extension/py_extension_tests.bzl diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 8740296c84..d4e92803b1 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -20,10 +20,10 @@ PY_EXTENSION_ATTRS = COMMON_ATTRS | { ), } -def create_py_extension_rule_builder(implementation, **kwargs): +def create_py_extension_rule_builder(**kwargs): """Create a rule builder for a py_extension.""" builder = ruleb.Rule( - implementation = implementation, + implementation = _py_extension_impl, attrs = PY_EXTENSION_ATTRS, provides = [PyInfo, CcInfo], toolchains = [ @@ -34,6 +34,4 @@ def create_py_extension_rule_builder(implementation, **kwargs): ) return builder -py_extension = create_py_extension_rule_builder( - implementation = _py_extension_impl, -).build() +py_extension = create_py_extension_rule_builder().build() diff --git a/tests/cc/py_extension/BUILD.bazel b/tests/cc/py_extension/BUILD.bazel new file mode 100644 index 0000000000..617000de87 --- /dev/null +++ b/tests/cc/py_extension/BUILD.bazel @@ -0,0 +1,29 @@ +# buildifier: disable=bzl-visibility +load("//python/cc:py_extension.bzl", "py_extension") +load( + "//tests/cc:py_extension_test.bzl", + "py_extension_test", +) +load(":py_extension_tests.bzl", "py_extension_analysis_test_suite") + +package( + default_testonly = True, + default_visibility = ["//visibility:private"], +) + +licenses(["notice"]) + +py_extension( + name = "ext", + srcs = ["ext.c"], +) + +py_extension_test( + name = "py_extension_test", + srcs = ["py_extension_test.py"], + deps = [":ext"], +) + +py_extension_analysis_test_suite( + name = "py_extension_analysis_tests", +) diff --git a/tests/cc/py_extension/ext.c b/tests/cc/py_extension/ext.c new file mode 100644 index 0000000000..dcf89334cc --- /dev/null +++ b/tests/cc/py_extension/ext.c @@ -0,0 +1 @@ +"""A no-op C extension.""" \ No newline at end of file diff --git a/tests/cc/py_extension/py_extension_test.py b/tests/cc/py_extension/py_extension_test.py new file mode 100644 index 0000000000..d05774e349 --- /dev/null +++ b/tests/cc/py_extension/py_extension_test.py @@ -0,0 +1,10 @@ +import unittest + + +class PyExtensionTest(unittest.TestCase): + def test_pass(self): + pass + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/cc/py_extension/py_extension_tests.bzl b/tests/cc/py_extension/py_extension_tests.bzl new file mode 100644 index 0000000000..8a87d04ca2 --- /dev/null +++ b/tests/cc/py_extension/py_extension_tests.bzl @@ -0,0 +1,40 @@ +# Copyright 2025 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Tests for py_extension.""" + +load("@rules_cc//cc/common:cc_info.bzl", "CcInfo") +load("@rules_testing//lib:analysis_test.bzl", "analysis_test", "test_suite") +load("//python/private:py_info.bzl", "PyInfo") + +_tests = [] + +def _test_basic_impl(env, target): + env.expect.that_target(target).has_provider(PyInfo) + env.expect.that_target(target).has_provider(CcInfo) + +def _test_basic(name): + analysis_test( + name = name, + impl = _test_basic_impl, + target = "//tests/cc/py_extension:ext", + ) + +_tests.append(_test_basic) + +def py_extension_analysis_test_suite(name): + test_suite( + name = name, + tests = _tests, + ) From c1f5a0329e3fdf1189944609a5baf0cf46d9745e Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 23 Sep 2025 11:22:37 -0700 Subject: [PATCH 04/35] vibe coded py_extension impl. looks roughly valid, but untested --- python/private/cc/py_extension_rule.bzl | 85 +++++++++++++++++++- tests/cc/py_extension/BUILD.bazel | 24 +++++- tests/cc/py_extension/my_lib.c | 5 ++ tests/cc/py_extension/my_lib.h | 1 + tests/cc/py_extension/py_extension_tests.bzl | 46 +++++++++-- 5 files changed, 151 insertions(+), 10 deletions(-) create mode 100644 tests/cc/py_extension/my_lib.c create mode 100644 tests/cc/py_extension/my_lib.h diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index d4e92803b1..42ff3d165b 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -1,5 +1,6 @@ """Implementation of the py_extension rule.""" +load("@rules_cc//cc/common:cc_common.bzl", "cc_common") load("@rules_cc//cc/common:cc_info.bzl", "CcInfo") load("//python/private:attr_builders.bzl", "attrb") load("//python/private:attributes.bzl", "COMMON_ATTRS") @@ -9,7 +10,74 @@ load("//python/private:rule_builders.bzl", "ruleb") load("//python/private:toolchain_types.bzl", "TARGET_TOOLCHAIN_TYPE") def _py_extension_impl(ctx): - pass + cc_toolchain = cc_common.get_toolchain_info(ctx = ctx).cc_toolchain + feature_configuration = cc_common.configure_features( + ctx = ctx, + cc_toolchain = cc_toolchain, + ) + + # Collect CcInfo from all deps for compilation + static_deps_infos = [dep[CcInfo] for dep in ctx.attr.static_deps] + dynamic_deps_infos = [dep[CcInfo] for dep in ctx.attr.dynamic_deps] + external_deps_infos = [dep[CcInfo] for dep in ctx.attr.external_deps] + all_deps_cc_info = cc_common.merge_cc_infos( + cc_infos = static_deps_infos + dynamic_deps_infos + external_deps_infos, + ) + + # Compile sources + compilation_outputs, _ = cc_common.compile( + name = ctx.label.name, + actions = ctx.actions, + feature_configuration = feature_configuration, + cc_toolchain = cc_toolchain, + srcs = ctx.files.srcs, + compilation_context = all_deps_cc_info.compilation_context, + ) + + # Link the extension + output_filename = ctx.label.name + ".so" + output = ctx.actions.declare_file(output_filename) + + # Static deps are linked directly into the .so + static_linking_context = cc_common.merge_cc_infos( + cc_infos = static_deps_infos, + ).linking_context + + # Dynamic deps are linked as shared libraries + dynamic_linking_context = cc_common.merge_cc_infos( + cc_infos = dynamic_deps_infos, + ).linking_context + + # For external deps, we need to allow undefined symbols. + user_link_flags = [] + if ctx.attr.external_deps: + user_link_flags.append("-Wl,--allow-shlib-undefined") + + cc_common.link( + name = ctx.label.name, + actions = ctx.actions, + feature_configuration = feature_configuration, + cc_toolchain = cc_toolchain, + output = output, + linking_contexts = depset([static_linking_context, dynamic_linking_context]), + linker_inputs = depset([compilation_outputs.linker_inputs]), + output_type = "dynamic_library", + user_link_flags = user_link_flags, + neverlink = True, + ) + + # Propagate CcInfo from dynamic and external deps, but not static ones. + propagated_cc_info = cc_common.merge_cc_infos( + cc_infos = dynamic_deps_infos + external_deps_infos, + ) + + return [ + DefaultInfo(files = depset([output])), + PyInfo( + transitive_sources = depset([output]), + ), + propagated_cc_info, + ] _MaybeBuiltinPyInfo = [[BuiltinPyInfo]] if BuiltinPyInfo != None else [] @@ -18,6 +86,21 @@ PY_EXTENSION_ATTRS = COMMON_ATTRS | { allow_files = True, doc = "The list of source files that are processed to create the target.", ), + "static_deps": lambda: attrb.LabelList( + providers = [CcInfo], + doc = "cc_library targets to be statically and privately linked.", + default = [], + ), + "dynamic_deps": lambda: attrb.LabelList( + providers = [CcInfo], + doc = "cc_library targets to be dynamically linked.", + default = [], + ), + "external_deps": lambda: attrb.LabelList( + providers = [CcInfo], + doc = "cc_library targets with external linkage.", + default = [], + ), } def create_py_extension_rule_builder(**kwargs): diff --git a/tests/cc/py_extension/BUILD.bazel b/tests/cc/py_extension/BUILD.bazel index 617000de87..07fe5bf8b6 100644 --- a/tests/cc/py_extension/BUILD.bazel +++ b/tests/cc/py_extension/BUILD.bazel @@ -14,14 +14,34 @@ package( licenses(["notice"]) py_extension( - name = "ext", + name = "ext_static", srcs = ["ext.c"], + static_deps = [":my_lib"], +) + +py_extension( + name = "ext_dynamic", + srcs = ["ext.c"], + dynamic_deps = [":my_lib_so"], +) + +cc_library( + name = "my_lib", + srcs = ["my_lib.c"], + hdrs = ["my_lib.h"], +) + +cc_library( + name = "my_lib_so", + srcs = ["my_lib.c"], + hdrs = ["my_lib.h"], + linkstatic = False, ) py_extension_test( name = "py_extension_test", srcs = ["py_extension_test.py"], - deps = [":ext"], + deps = [":ext_static"], ) py_extension_analysis_test_suite( diff --git a/tests/cc/py_extension/my_lib.c b/tests/cc/py_extension/my_lib.c new file mode 100644 index 0000000000..4d910c1178 --- /dev/null +++ b/tests/cc/py_extension/my_lib.c @@ -0,0 +1,5 @@ +#include "my_lib.h" + +int my_lib_func() { + return 42; +} diff --git a/tests/cc/py_extension/my_lib.h b/tests/cc/py_extension/my_lib.h new file mode 100644 index 0000000000..d0f272abd7 --- /dev/null +++ b/tests/cc/py_extension/my_lib.h @@ -0,0 +1 @@ +int my_lib_func(); diff --git a/tests/cc/py_extension/py_extension_tests.bzl b/tests/cc/py_extension/py_extension_tests.bzl index 8a87d04ca2..a68e59b77a 100644 --- a/tests/cc/py_extension/py_extension_tests.bzl +++ b/tests/cc/py_extension/py_extension_tests.bzl @@ -16,22 +16,54 @@ load("@rules_cc//cc/common:cc_info.bzl", "CcInfo") load("@rules_testing//lib:analysis_test.bzl", "analysis_test", "test_suite") +load("@rules_testing//lib:truth.bzl", "matching") load("//python/private:py_info.bzl", "PyInfo") _tests = [] -def _test_basic_impl(env, target): - env.expect.that_target(target).has_provider(PyInfo) - env.expect.that_target(target).has_provider(CcInfo) +def _test_static_deps_impl(env, target): + py_info = env.expect.that_target(target).has_provider(PyInfo) + cc_info = env.expect.that_target(target).has_provider(CcInfo) -def _test_basic(name): + # The .so should be in PyInfo + env.expect.that_collection(py_info.transitive_sources).has_size(1) + env.expect.that_collection(py_info.transitive_sources).contains_predicate( + matching.str_matches("ext_static.so$"), + ) + + # CcInfo from static_deps should not be propagated. + env.expect.that_collection(cc_info.linking_context.linker_inputs.to_list()).is_empty() + +def _test_static_deps(name): + analysis_test( + name = name, + impl = _test_static_deps_impl, + target = "//tests/cc/py_extension:ext_static", + ) + +_tests.append(_test_static_deps) + +def _test_dynamic_deps_impl(env, target): + py_info = env.expect.that_target(target).has_provider(PyInfo) + cc_info = env.expect.that_target(target).has_provider(CcInfo) + + # The .so should be in PyInfo + env.expect.that_collection(py_info.transitive_sources).has_size(1) + env.expect.that_collection(py_info.transitive_sources).contains_predicate( + matching.str_matches("ext_dynamic.so$"), + ) + + # CcInfo from dynamic_deps should be propagated. + env.expect.that_collection(cc_info.linking_context.linker_inputs.to_list()).is_not_empty() + +def _test_dynamic_deps(name): analysis_test( name = name, - impl = _test_basic_impl, - target = "//tests/cc/py_extension:ext", + impl = _test_dynamic_deps_impl, + target = "//tests/cc/py_extension:ext_dynamic", ) -_tests.append(_test_basic) +_tests.append(_test_dynamic_deps) def py_extension_analysis_test_suite(name): test_suite( From b12c29519a3c639e325074993b012dd8ccf120d3 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Tue, 23 Sep 2025 23:18:43 -0700 Subject: [PATCH 05/35] making more vibe coded progress --- docs/pyproject.toml | 3 +- docs/requirements.txt | 4 ++ python/private/cc/py_extension_macro.bzl | 2 +- python/private/cc/py_extension_rule.bzl | 67 ++++++++++++------- tests/cc/py_extension/BUILD.bazel | 37 +++++----- tests/cc/py_extension/dyn_dep_a.c | 0 tests/cc/py_extension/dyn_dep_a.h | 0 tests/cc/py_extension/ext.c | 1 - tests/cc/py_extension/ext_shared.c | 1 + tests/cc/py_extension/ext_static.c | 1 + tests/cc/py_extension/py_extension_test.py | 36 +++++++++- tests/cc/py_extension/py_extension_tests.bzl | 4 +- .../py_extension/{my_lib.c => static_dep.c} | 0 .../py_extension/{my_lib.h => static_dep.h} | 0 14 files changed, 108 insertions(+), 48 deletions(-) create mode 100644 tests/cc/py_extension/dyn_dep_a.c create mode 100644 tests/cc/py_extension/dyn_dep_a.h delete mode 100644 tests/cc/py_extension/ext.c create mode 100644 tests/cc/py_extension/ext_shared.c create mode 100644 tests/cc/py_extension/ext_static.c rename tests/cc/py_extension/{my_lib.c => static_dep.c} (100%) rename tests/cc/py_extension/{my_lib.h => static_dep.h} (100%) diff --git a/docs/pyproject.toml b/docs/pyproject.toml index 9a089df59c..c928720a22 100644 --- a/docs/pyproject.toml +++ b/docs/pyproject.toml @@ -13,5 +13,6 @@ dependencies = [ "absl-py", "typing-extensions", "sphinx-reredirects", - "pefile" + "pefile", + "pyelftools", ] diff --git a/docs/requirements.txt b/docs/requirements.txt index 5929e73785..5be6c59bde 100644 --- a/docs/requirements.txt +++ b/docs/requirements.txt @@ -236,6 +236,10 @@ pefile==2024.8.26 \ --hash=sha256:3ff6c5d8b43e8c37bb6e6dd5085658d658a7a0bdcd20b6a07b1fcfc1c4e9d632 \ --hash=sha256:76f8b485dcd3b1bb8166f1128d395fa3d87af26360c2358fb75b80019b957c6f # via rules-python-docs (docs/pyproject.toml) +pyelftools==0.32 \ + --hash=sha256:013df952a006db5e138b1edf6d8a68ecc50630adbd0d83a2d41e7f846163d738 \ + --hash=sha256:6de90ee7b8263e740c8715a925382d4099b354f29ac48ea40d840cf7aa14ace5 + # via rules-python-docs (docs/pyproject.toml) pygments==2.19.2 \ --hash=sha256:636cb2477cec7f8952536970bc533bc43743542f70392ae026374600add5b887 \ --hash=sha256:86540386c03d588bb81d44bc3928634ff26449851e99741617ecb9037ee5ec0b diff --git a/python/private/cc/py_extension_macro.bzl b/python/private/cc/py_extension_macro.bzl index 98932f9fe3..04bd1460fe 100644 --- a/python/private/cc/py_extension_macro.bzl +++ b/python/private/cc/py_extension_macro.bzl @@ -1,7 +1,7 @@ """Wrapper macro for the py_extension rule.""" -load(":py_extension_rule.bzl", _py_extension = "py_extension") load("//python/private:util.bzl", "add_tag") +load(":py_extension_rule.bzl", _py_extension = "py_extension") def py_extension(**kwargs): """A macro that calls the py_extension rule and adds a tag. diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 42ff3d165b..3465cebb90 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -10,7 +10,7 @@ load("//python/private:rule_builders.bzl", "ruleb") load("//python/private:toolchain_types.bzl", "TARGET_TOOLCHAIN_TYPE") def _py_extension_impl(ctx): - cc_toolchain = cc_common.get_toolchain_info(ctx = ctx).cc_toolchain + cc_toolchain = ctx.toolchains["@bazel_tools//tools/cpp:toolchain_type"].cc feature_configuration = cc_common.configure_features( ctx = ctx, cc_toolchain = cc_toolchain, @@ -25,19 +25,15 @@ def _py_extension_impl(ctx): ) # Compile sources - compilation_outputs, _ = cc_common.compile( + _, compilation_outputs = cc_common.compile( name = ctx.label.name, actions = ctx.actions, feature_configuration = feature_configuration, cc_toolchain = cc_toolchain, srcs = ctx.files.srcs, - compilation_context = all_deps_cc_info.compilation_context, + compilation_contexts = [all_deps_cc_info.compilation_context], ) - # Link the extension - output_filename = ctx.label.name + ".so" - output = ctx.actions.declare_file(output_filename) - # Static deps are linked directly into the .so static_linking_context = cc_common.merge_cc_infos( cc_infos = static_deps_infos, @@ -53,17 +49,36 @@ def _py_extension_impl(ctx): if ctx.attr.external_deps: user_link_flags.append("-Wl,--allow-shlib-undefined") - cc_common.link( + # This function also does the linking + _, linking_outputs = cc_common.create_linking_context_from_compilation_outputs( name = ctx.label.name, actions = ctx.actions, feature_configuration = feature_configuration, cc_toolchain = cc_toolchain, - output = output, - linking_contexts = depset([static_linking_context, dynamic_linking_context]), - linker_inputs = depset([compilation_outputs.linker_inputs]), - output_type = "dynamic_library", + compilation_outputs = compilation_outputs, user_link_flags = user_link_flags, - neverlink = True, + linking_contexts = [static_linking_context, dynamic_linking_context], + ) + + print(linking_outputs) + ltl = linking_outputs.library_to_link + print(ltl) + print(ltl.dynamic_library) + print(ltl.resolved_symlink_dynamic_library) + lib_dso = ltl.resolved_symlink_dynamic_library + if lib_dso == None: + lib_dso = ltl.dynamic_library + + if lib_dso == None: + fail("No DSO output found in {}".format(ltl)) + + # todo: pick appropriate infix based on py_extension attr settings + py_dso = ctx.actions.declare_file("{}.so".format(ctx.label.name)) + ctx.actions.run_shell( + command = 'cp "$1" "$2"', + arguments = [lib_dso.path, py_dso.path], + inputs = [lib_dso], + outputs = [py_dso], ) # Propagate CcInfo from dynamic and external deps, but not static ones. @@ -72,9 +87,12 @@ def _py_extension_impl(ctx): ) return [ - DefaultInfo(files = depset([output])), + DefaultInfo( + files = depset([py_dso]), + runfiles = ctx.runfiles([py_dso]), + ), PyInfo( - transitive_sources = depset([output]), + transitive_sources = depset([py_dso]), ), propagated_cc_info, ] @@ -82,15 +100,6 @@ def _py_extension_impl(ctx): _MaybeBuiltinPyInfo = [[BuiltinPyInfo]] if BuiltinPyInfo != None else [] PY_EXTENSION_ATTRS = COMMON_ATTRS | { - "srcs": lambda: attrb.LabelList( - allow_files = True, - doc = "The list of source files that are processed to create the target.", - ), - "static_deps": lambda: attrb.LabelList( - providers = [CcInfo], - doc = "cc_library targets to be statically and privately linked.", - default = [], - ), "dynamic_deps": lambda: attrb.LabelList( providers = [CcInfo], doc = "cc_library targets to be dynamically linked.", @@ -101,6 +110,15 @@ PY_EXTENSION_ATTRS = COMMON_ATTRS | { doc = "cc_library targets with external linkage.", default = [], ), + "srcs": lambda: attrb.LabelList( + allow_files = True, + doc = "The list of source files that are processed to create the target.", + ), + "static_deps": lambda: attrb.LabelList( + providers = [CcInfo], + doc = "cc_library targets to be statically and privately linked.", + default = [], + ), } def create_py_extension_rule_builder(**kwargs): @@ -113,6 +131,7 @@ def create_py_extension_rule_builder(**kwargs): ruleb.ToolchainType(TARGET_TOOLCHAIN_TYPE), ruleb.ToolchainType("@bazel_tools//tools/cpp:toolchain_type"), ], + fragments = ["cpp"], **kwargs ) return builder diff --git a/tests/cc/py_extension/BUILD.bazel b/tests/cc/py_extension/BUILD.bazel index 07fe5bf8b6..6b04ca90be 100644 --- a/tests/cc/py_extension/BUILD.bazel +++ b/tests/cc/py_extension/BUILD.bazel @@ -1,9 +1,8 @@ +load("@rules_cc//cc:defs.bzl", "cc_library") +load("//python:py_test.bzl", "py_test") + # buildifier: disable=bzl-visibility load("//python/cc:py_extension.bzl", "py_extension") -load( - "//tests/cc:py_extension_test.bzl", - "py_extension_test", -) load(":py_extension_tests.bzl", "py_extension_analysis_test_suite") package( @@ -15,33 +14,37 @@ licenses(["notice"]) py_extension( name = "ext_static", - srcs = ["ext.c"], - static_deps = [":my_lib"], + srcs = ["ext_static.c"], + static_deps = [":static_dep"], ) py_extension( - name = "ext_dynamic", - srcs = ["ext.c"], - dynamic_deps = [":my_lib_so"], + name = "ext_shared", + srcs = ["ext_shared.c"], + dynamic_deps = [":dyn_dep_a"], ) cc_library( - name = "my_lib", - srcs = ["my_lib.c"], - hdrs = ["my_lib.h"], + name = "static_dep", + srcs = ["static_dep.c"], + hdrs = ["static_dep.h"], ) cc_library( - name = "my_lib_so", - srcs = ["my_lib.c"], - hdrs = ["my_lib.h"], + name = "dyn_dep_a", + srcs = ["dyn_dep_a.c"], + hdrs = ["dyn_dep_a.h"], linkstatic = False, ) -py_extension_test( +py_test( name = "py_extension_test", srcs = ["py_extension_test.py"], - deps = [":ext_static"], + deps = [ + ":ext_shared", + "@dev_pip//pyelftools", + "@rules_python//python/runfiles", + ], ) py_extension_analysis_test_suite( diff --git a/tests/cc/py_extension/dyn_dep_a.c b/tests/cc/py_extension/dyn_dep_a.c new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/cc/py_extension/dyn_dep_a.h b/tests/cc/py_extension/dyn_dep_a.h new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/cc/py_extension/ext.c b/tests/cc/py_extension/ext.c deleted file mode 100644 index dcf89334cc..0000000000 --- a/tests/cc/py_extension/ext.c +++ /dev/null @@ -1 +0,0 @@ -"""A no-op C extension.""" \ No newline at end of file diff --git a/tests/cc/py_extension/ext_shared.c b/tests/cc/py_extension/ext_shared.c new file mode 100644 index 0000000000..0aacba5694 --- /dev/null +++ b/tests/cc/py_extension/ext_shared.c @@ -0,0 +1 @@ +/* A no-op C extension for shared linking tests. */ diff --git a/tests/cc/py_extension/ext_static.c b/tests/cc/py_extension/ext_static.c new file mode 100644 index 0000000000..500c52db00 --- /dev/null +++ b/tests/cc/py_extension/ext_static.c @@ -0,0 +1 @@ +/* A no-op C extension for static linking tests. */ diff --git a/tests/cc/py_extension/py_extension_test.py b/tests/cc/py_extension/py_extension_test.py index d05774e349..9a19e6aa98 100644 --- a/tests/cc/py_extension/py_extension_test.py +++ b/tests/cc/py_extension/py_extension_test.py @@ -1,9 +1,41 @@ +import os import unittest +from elftools.elf.dynamic import DynamicSection +from elftools.elf.elffile import ELFFile + +from python.runfiles import runfiles + class PyExtensionTest(unittest.TestCase): - def test_pass(self): - pass + def test_inspect_elf(self): + r = runfiles.Create() + ext_path = r.Rlocation("rules_python/tests/cc/py_extension/ext_shared.so") + self.assertTrue( + os.path.exists(ext_path), f"Could not find ext_shared.so at {ext_path}" + ) + + with open(ext_path, "rb") as f: + elf = ELFFile(f) + + # Check for DT_NEEDED entry for the dynamic library + dynamic_section = elf.get_section_by_name(".dynamic") + self.assertIsNotNone(dynamic_section) + self.assertTrue(isinstance(dynamic_section, DynamicSection)) + + needed_libs = [ + tag.needed + for tag in dynamic_section.iter_tags() + if tag.entry.d_tag == "DT_NEEDED" + ] + self.assertIn("libdyn_dep_a.so", needed_libs) + + # Check for the PyInit symbol + dynsym_section = elf.get_section_by_name(".dynsym") + self.assertIsNotNone(dynsym_section) + + symbols = [s.name for s in dynsym_section.iter_symbols()] + self.assertIn("PyInit_ext_shared", symbols) if __name__ == "__main__": diff --git a/tests/cc/py_extension/py_extension_tests.bzl b/tests/cc/py_extension/py_extension_tests.bzl index a68e59b77a..9d471cda49 100644 --- a/tests/cc/py_extension/py_extension_tests.bzl +++ b/tests/cc/py_extension/py_extension_tests.bzl @@ -50,7 +50,7 @@ def _test_dynamic_deps_impl(env, target): # The .so should be in PyInfo env.expect.that_collection(py_info.transitive_sources).has_size(1) env.expect.that_collection(py_info.transitive_sources).contains_predicate( - matching.str_matches("ext_dynamic.so$"), + matching.str_matches("ext_shared.so$"), ) # CcInfo from dynamic_deps should be propagated. @@ -60,7 +60,7 @@ def _test_dynamic_deps(name): analysis_test( name = name, impl = _test_dynamic_deps_impl, - target = "//tests/cc/py_extension:ext_dynamic", + target = "//tests/cc/py_extension:ext_shared", ) _tests.append(_test_dynamic_deps) diff --git a/tests/cc/py_extension/my_lib.c b/tests/cc/py_extension/static_dep.c similarity index 100% rename from tests/cc/py_extension/my_lib.c rename to tests/cc/py_extension/static_dep.c diff --git a/tests/cc/py_extension/my_lib.h b/tests/cc/py_extension/static_dep.h similarity index 100% rename from tests/cc/py_extension/my_lib.h rename to tests/cc/py_extension/static_dep.h From e7d21b102198f0a9ebdf6292adf9a6f16ef26221 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Wed, 24 Sep 2025 01:12:36 -0700 Subject: [PATCH 06/35] gave up vibe coding. confidence isn't correctness. ...and sycophantic toasters are bad collaborators --- python/private/cc/py_extension_rule.bzl | 105 ++++++++++++--------- tests/cc/py_extension/BUILD.bazel | 36 +++++-- tests/cc/py_extension/add_one.c | 3 + tests/cc/py_extension/add_one.h | 6 ++ tests/cc/py_extension/dyn_dep_a.c | 0 tests/cc/py_extension/dyn_dep_a.h | 0 tests/cc/py_extension/ext_shared.c | 34 ++++++- tests/cc/py_extension/py_extension_test.py | 33 +++---- 8 files changed, 144 insertions(+), 73 deletions(-) create mode 100644 tests/cc/py_extension/add_one.c create mode 100644 tests/cc/py_extension/add_one.h delete mode 100644 tests/cc/py_extension/dyn_dep_a.c delete mode 100644 tests/cc/py_extension/dyn_dep_a.h diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 3465cebb90..8d76cdbf4d 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -5,11 +5,13 @@ load("@rules_cc//cc/common:cc_info.bzl", "CcInfo") load("//python/private:attr_builders.bzl", "attrb") load("//python/private:attributes.bzl", "COMMON_ATTRS") load("//python/private:py_info.bzl", "PyInfo") +load("//python/private:py_internal.bzl", "py_internal") load("//python/private:reexports.bzl", "BuiltinPyInfo") load("//python/private:rule_builders.bzl", "ruleb") load("//python/private:toolchain_types.bzl", "TARGET_TOOLCHAIN_TYPE") def _py_extension_impl(ctx): + module_name = ctx.attr.module_name or ctx.label.name cc_toolchain = ctx.toolchains["@bazel_tools//tools/cpp:toolchain_type"].cc feature_configuration = cc_common.configure_features( ctx = ctx, @@ -24,62 +26,75 @@ def _py_extension_impl(ctx): cc_infos = static_deps_infos + dynamic_deps_infos + external_deps_infos, ) - # Compile sources - _, compilation_outputs = cc_common.compile( - name = ctx.label.name, - actions = ctx.actions, - feature_configuration = feature_configuration, - cc_toolchain = cc_toolchain, - srcs = ctx.files.srcs, - compilation_contexts = [all_deps_cc_info.compilation_context], - ) - # Static deps are linked directly into the .so - static_linking_context = cc_common.merge_cc_infos( + static_cc_info = cc_common.merge_cc_infos( cc_infos = static_deps_infos, - ).linking_context + ) # Dynamic deps are linked as shared libraries dynamic_linking_context = cc_common.merge_cc_infos( cc_infos = dynamic_deps_infos, ).linking_context - # For external deps, we need to allow undefined symbols. user_link_flags = [] + user_link_flags.append("-Wl,--export-dynamic-symbol=PyInit_{module_name}".format( + module_name = module_name, + )) + + # The PyInit symbol looks unused, so the linker optimizes it away. Telling it + # to treat it as undefined causes it to be retained. + user_link_flags.append("-Wl,--undefined=PyInit_{module_name}".format( + module_name = module_name, + )) + if ctx.attr.external_deps: user_link_flags.append("-Wl,--allow-shlib-undefined") - # This function also does the linking - _, linking_outputs = cc_common.create_linking_context_from_compilation_outputs( - name = ctx.label.name, + # todo: use toolchain to determine `abi3.` infix + # todo: use toolchain to determine platform extension (pyd, so, etc) + output_filename = "{module_name}.{ext}".format( + module_name = module_name, + ext = "so", + ) + py_dso = ctx.actions.declare_file(output_filename) + + static_linking_context = static_cc_info.linking_context + linking_contexts = [ + static_linking_context, + dynamic_linking_context, + ] + + # Add target-level linkopts last so users can override. + user_link_flags.extend(ctx.attr.linkopts) + print(( + "===LINK:\n" + + " user_link_flags={user_link_flags}" + ).format( + user_link_flags = user_link_flags, + )) + + # todo: add linker script to hide symbols by default + # py_internal allows using some private apis, which may or may not be needed. + # based upon cc_shared_library.bzl + cc_linking_outputs = py_internal.link( actions = ctx.actions, feature_configuration = feature_configuration, cc_toolchain = cc_toolchain, - compilation_outputs = compilation_outputs, + linking_contexts = linking_contexts, user_link_flags = user_link_flags, - linking_contexts = [static_linking_context, dynamic_linking_context], - ) - - print(linking_outputs) - ltl = linking_outputs.library_to_link - print(ltl) - print(ltl.dynamic_library) - print(ltl.resolved_symlink_dynamic_library) - lib_dso = ltl.resolved_symlink_dynamic_library - if lib_dso == None: - lib_dso = ltl.dynamic_library - - if lib_dso == None: - fail("No DSO output found in {}".format(ltl)) - - # todo: pick appropriate infix based on py_extension attr settings - py_dso = ctx.actions.declare_file("{}.so".format(ctx.label.name)) - ctx.actions.run_shell( - command = 'cp "$1" "$2"', - arguments = [lib_dso.path, py_dso.path], - inputs = [lib_dso], - outputs = [py_dso], + # todo: add additional_inputs + name = ctx.label.name, + output_type = "dynamic_library", + main_output = py_dso, + # todo: maybe variables_extension + # todo: maybe additional_outputs ) + print(( + "===LINK OUTPUT:\n" + + " {}" + ).format( + cc_linking_outputs, + )) # Propagate CcInfo from dynamic and external deps, but not static ones. propagated_cc_info = cc_common.merge_cc_infos( @@ -87,10 +102,7 @@ def _py_extension_impl(ctx): ) return [ - DefaultInfo( - files = depset([py_dso]), - runfiles = ctx.runfiles([py_dso]), - ), + DefaultInfo(files = depset([py_dso])), PyInfo( transitive_sources = depset([py_dso]), ), @@ -110,15 +122,14 @@ PY_EXTENSION_ATTRS = COMMON_ATTRS | { doc = "cc_library targets with external linkage.", default = [], ), - "srcs": lambda: attrb.LabelList( - allow_files = True, - doc = "The list of source files that are processed to create the target.", - ), "static_deps": lambda: attrb.LabelList( providers = [CcInfo], doc = "cc_library targets to be statically and privately linked.", default = [], ), + "copts": lambda: attrb.StringList(), + "linkopts": lambda: attrb.StringList(), + "module_name": lambda: attrb.String(), } def create_py_extension_rule_builder(**kwargs): diff --git a/tests/cc/py_extension/BUILD.bazel b/tests/cc/py_extension/BUILD.bazel index 6b04ca90be..b68dbefb25 100644 --- a/tests/cc/py_extension/BUILD.bazel +++ b/tests/cc/py_extension/BUILD.bazel @@ -14,14 +14,35 @@ licenses(["notice"]) py_extension( name = "ext_static", - srcs = ["ext_static.c"], + ##srcs = ["ext_static.c"], static_deps = [":static_dep"], ) py_extension( name = "ext_shared", + dynamic_deps = [ + ":add_one", + ], + static_deps = [ + ":ext_shared_impl", + ], +) + +cc_library( + name = "ext_shared_impl", srcs = ["ext_shared.c"], - dynamic_deps = [":dyn_dep_a"], + copts = [ + # Gemini says PIC is needed + "-fPIC", + "-fvisibility=hidden", + ], + deps = [ + ":add_one_headers", + # todo: if we put this here, we statically link add_one into the + # extension. + #":add_one_impl", + "@rules_python//python/cc:current_py_cc_headers", + ], ) cc_library( @@ -31,10 +52,13 @@ cc_library( ) cc_library( - name = "dyn_dep_a", - srcs = ["dyn_dep_a.c"], - hdrs = ["dyn_dep_a.h"], - linkstatic = False, + name = "add_one_headers", + hdrs = ["add_one.h"], +) + +cc_library( + name = "add_one", + srcs = ["add_one.c"], ) py_test( diff --git a/tests/cc/py_extension/add_one.c b/tests/cc/py_extension/add_one.c new file mode 100644 index 0000000000..c83da4e747 --- /dev/null +++ b/tests/cc/py_extension/add_one.c @@ -0,0 +1,3 @@ +int add_one(int x) { + return x + 1; +} diff --git a/tests/cc/py_extension/add_one.h b/tests/cc/py_extension/add_one.h new file mode 100644 index 0000000000..eda0abf7e5 --- /dev/null +++ b/tests/cc/py_extension/add_one.h @@ -0,0 +1,6 @@ +#ifndef TESTS_CC_PY_EXTENSION_DYN_DEP_A_H_ +#define TESTS_CC_PY_EXTENSION_DYN_DEP_A_H_ + +int add_one(int x); + +#endif // TESTS_CC_PY_EXTENSION_DYN_DEP_A_H_ diff --git a/tests/cc/py_extension/dyn_dep_a.c b/tests/cc/py_extension/dyn_dep_a.c deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/tests/cc/py_extension/dyn_dep_a.h b/tests/cc/py_extension/dyn_dep_a.h deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/tests/cc/py_extension/ext_shared.c b/tests/cc/py_extension/ext_shared.c index 0aacba5694..4b79a6da2f 100644 --- a/tests/cc/py_extension/ext_shared.c +++ b/tests/cc/py_extension/ext_shared.c @@ -1 +1,33 @@ -/* A no-op C extension for shared linking tests. */ +#include + +#include "tests/cc/py_extension/add_one.h" + +// A simple function that returns a Python integer. +static PyObject* do_alpha(PyObject* self, PyObject* args) { + return PyLong_FromLong(add_one(41)); +} + +// Method definition object for this extension, these are the functions +// that will be available in the module. +static PyMethodDef ModuleMethods[] = { + {"do_alpha", do_alpha, METH_NOARGS, "A simple C function."}, + {NULL, NULL, 0, NULL} /* Sentinel */ +}; + +// Module definition +// The arguments of this structure tell Python what to call your extension, +// what its methods are and where to look for its method definitions. +static struct PyModuleDef ext_shared_module = { + PyModuleDef_HEAD_INIT, + "ext_shared", /* name of module */ + NULL, /* module documentation, may be NULL */ + -1, /* size of per-interpreter state of the module, + or -1 if the module keeps state in global variables. */ + ModuleMethods +}; + +// The module init function. This must be exported and retained in the +// shared library output. +PyMODINIT_FUNC PyInit_ext_shared(void) { + return PyModule_Create(&ext_shared_module); +} diff --git a/tests/cc/py_extension/py_extension_test.py b/tests/cc/py_extension/py_extension_test.py index 9a19e6aa98..81f277f021 100644 --- a/tests/cc/py_extension/py_extension_test.py +++ b/tests/cc/py_extension/py_extension_test.py @@ -1,41 +1,36 @@ -import os import unittest - -from elftools.elf.dynamic import DynamicSection -from elftools.elf.elffile import ELFFile - +import os from python.runfiles import runfiles - +from elftools.elf.elffile import ELFFile +from elftools.elf.dynamic import DynamicSection +import ext_shared class PyExtensionTest(unittest.TestCase): def test_inspect_elf(self): r = runfiles.Create() ext_path = r.Rlocation("rules_python/tests/cc/py_extension/ext_shared.so") - self.assertTrue( - os.path.exists(ext_path), f"Could not find ext_shared.so at {ext_path}" - ) + self.assertTrue(os.path.exists(ext_path), f"Could not find ext_shared.so at {ext_path}") - with open(ext_path, "rb") as f: + with open(ext_path, 'rb') as f: elf = ELFFile(f) # Check for DT_NEEDED entry for the dynamic library - dynamic_section = elf.get_section_by_name(".dynamic") + dynamic_section = elf.get_section_by_name('.dynamic') self.assertIsNotNone(dynamic_section) self.assertTrue(isinstance(dynamic_section, DynamicSection)) - needed_libs = [ - tag.needed - for tag in dynamic_section.iter_tags() - if tag.entry.d_tag == "DT_NEEDED" - ] - self.assertIn("libdyn_dep_a.so", needed_libs) + needed_libs = [tag.needed for tag in dynamic_section.iter_tags() if tag.entry.d_tag == 'DT_NEEDED'] + self.assertIn('libdyn_dep_a.so', needed_libs) # Check for the PyInit symbol - dynsym_section = elf.get_section_by_name(".dynsym") + dynsym_section = elf.get_section_by_name('.dynsym') self.assertIsNotNone(dynsym_section) symbols = [s.name for s in dynsym_section.iter_symbols()] - self.assertIn("PyInit_ext_shared", symbols) + self.assertIn('PyInit_ext_shared', symbols) + + def test_import_and_call(self): + self.assertEqual(ext_shared.my_c_function(), 42) if __name__ == "__main__": From 4494b288f2e2b56710b5ce51f72adc205d0c380a Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Wed, 15 Oct 2025 17:40:13 -0700 Subject: [PATCH 07/35] try cc_shared_library impl --- python/private/cc/py_extension_macro.bzl | 24 +++++++++++++-- tests/cc/py_extension/BUILD.bazel | 39 +++++++++++++++++++++--- tests/cc/py_extension/add_one.c | 4 +++ tests/cc/py_extension/add_one_helper.c | 7 +++++ tests/cc/py_extension/add_one_helper.h | 2 ++ 5 files changed, 70 insertions(+), 6 deletions(-) create mode 100644 tests/cc/py_extension/add_one_helper.c create mode 100644 tests/cc/py_extension/add_one_helper.h diff --git a/python/private/cc/py_extension_macro.bzl b/python/private/cc/py_extension_macro.bzl index 04bd1460fe..8c4cc36f58 100644 --- a/python/private/cc/py_extension_macro.bzl +++ b/python/private/cc/py_extension_macro.bzl @@ -1,7 +1,12 @@ """Wrapper macro for the py_extension rule.""" +load("@rules_cc//cc:cc_shared_library.bzl", "cc_shared_library") load("//python/private:util.bzl", "add_tag") -load(":py_extension_rule.bzl", _py_extension = "py_extension") +load( + ":py_extension_rule.bzl", + _py_extension = "py_extension", + ##_py_extension_csl_rule = "py_extension_csl", +) def py_extension(**kwargs): """A macro that calls the py_extension rule and adds a tag. @@ -10,4 +15,19 @@ def py_extension(**kwargs): **kwargs: Additional arguments to pass to the rule. """ add_tag(kwargs, "@rules_python//python/cc:py_extension") - _py_extension(**kwargs) + + use_csl = kwargs.pop("use_csl", False) + if use_csl: + _py_extension_csl(**kwargs) + else: + _py_extension(**kwargs) + +def _py_extension_csl(*, name, module_name = None, **kwargs): + if not module_name: + module_name = name + + cc_shared_library( + name = name, + shared_lib_name = module_name + ".so", + **kwargs + ) diff --git a/tests/cc/py_extension/BUILD.bazel b/tests/cc/py_extension/BUILD.bazel index b68dbefb25..e88690b8c1 100644 --- a/tests/cc/py_extension/BUILD.bazel +++ b/tests/cc/py_extension/BUILD.bazel @@ -1,4 +1,4 @@ -load("@rules_cc//cc:defs.bzl", "cc_library") +load("@rules_cc//cc:cc_library.bzl", "cc_library") load("//python:py_test.bzl", "py_test") # buildifier: disable=bzl-visibility @@ -28,6 +28,22 @@ py_extension( ], ) +py_extension( + name = "ext_csl_shared", + dynamic_deps = [ + ":add_one_shared", + ], + exports_filter = [ + "//tests/cc/py_extension:ext_shared_impl", + ":ext_shared_impl", + ], + module_name = "ext_shared", + use_csl = True, + deps = [ + ":ext_shared_impl", + ], +) + cc_library( name = "ext_shared_impl", srcs = ["ext_shared.c"], @@ -37,10 +53,10 @@ cc_library( "-fvisibility=hidden", ], deps = [ - ":add_one_headers", + #":add_one_headers", # todo: if we put this here, we statically link add_one into the # extension. - #":add_one_impl", + ":add_one_impl", "@rules_python//python/cc:current_py_cc_headers", ], ) @@ -51,14 +67,29 @@ cc_library( hdrs = ["static_dep.h"], ) +cc_shared_library( + name = "add_one_shared", + deps = [":add_one_impl"], +) + cc_library( name = "add_one_headers", hdrs = ["add_one.h"], ) cc_library( - name = "add_one", + name = "add_one_impl", srcs = ["add_one.c"], + deps = [ + ":add_one_headers", + ":add_one_helper", + ], +) + +cc_library( + name = "add_one_helper", + srcs = ["add_one_helper.c"], + hdrs = ["add_one_helper.h"], ) py_test( diff --git a/tests/cc/py_extension/add_one.c b/tests/cc/py_extension/add_one.c index c83da4e747..7da8f6796e 100644 --- a/tests/cc/py_extension/add_one.c +++ b/tests/cc/py_extension/add_one.c @@ -1,3 +1,7 @@ + +#include "add_one_helper.h" + int add_one(int x) { + x = add_one_helper(x); return x + 1; } diff --git a/tests/cc/py_extension/add_one_helper.c b/tests/cc/py_extension/add_one_helper.c new file mode 100644 index 0000000000..21d19a5823 --- /dev/null +++ b/tests/cc/py_extension/add_one_helper.c @@ -0,0 +1,7 @@ + + +#include "add_one_helper.h" + +int add_one_helper(int i) { + return i + 1; +} diff --git a/tests/cc/py_extension/add_one_helper.h b/tests/cc/py_extension/add_one_helper.h new file mode 100644 index 0000000000..5524d3077f --- /dev/null +++ b/tests/cc/py_extension/add_one_helper.h @@ -0,0 +1,2 @@ + +int add_one_helper(int i); From e8f4792a89bebc28fc26ddff3f9a12d1b9a03b03 Mon Sep 17 00:00:00 2001 From: rsartor-cmd Date: Fri, 26 Jun 2026 13:21:41 -0500 Subject: [PATCH 08/35] feat: updates to the py_extension rule for building C extensions (#3851) This builds on the existing `py-extension` branch. It adds support for platform and abi tags in the resulting library filename. The `:py_extension_test`, `:py_extension_analysis_tests`, and `:py_limited_api_tests` test targets in `//tests/cc/py_extension` now build and pass. Relates to https://github.com/bazel-contrib/rules_python/issues/3283 --- python/private/cc/py_extension_rule.bzl | 279 ++++++++++++++++-- tests/cc/py_extension/BUILD.bazel | 27 +- tests/cc/py_extension/ext_limited.c | 22 ++ tests/cc/py_extension/py_extension_test.py | 33 ++- tests/cc/py_extension/py_extension_tests.bzl | 29 +- .../cc/py_extension/py_limited_api_tests.bzl | 77 +++++ 6 files changed, 425 insertions(+), 42 deletions(-) create mode 100644 tests/cc/py_extension/ext_limited.c create mode 100644 tests/cc/py_extension/py_limited_api_tests.bzl diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 8d76cdbf4d..19a0281288 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -12,6 +12,8 @@ load("//python/private:toolchain_types.bzl", "TARGET_TOOLCHAIN_TYPE") def _py_extension_impl(ctx): module_name = ctx.attr.module_name or ctx.label.name + repo_name = ctx.label.workspace_name or ctx.workspace_name + import_path = repo_name + "/" + ctx.label.package cc_toolchain = ctx.toolchains["@bazel_tools//tools/cpp:toolchain_type"].cc feature_configuration = cc_common.configure_features( ctx = ctx, @@ -20,11 +22,8 @@ def _py_extension_impl(ctx): # Collect CcInfo from all deps for compilation static_deps_infos = [dep[CcInfo] for dep in ctx.attr.static_deps] - dynamic_deps_infos = [dep[CcInfo] for dep in ctx.attr.dynamic_deps] + dynamic_deps_infos = [dep[CcSharedLibraryInfo] for dep in ctx.attr.dynamic_deps] external_deps_infos = [dep[CcInfo] for dep in ctx.attr.external_deps] - all_deps_cc_info = cc_common.merge_cc_infos( - cc_infos = static_deps_infos + dynamic_deps_infos + external_deps_infos, - ) # Static deps are linked directly into the .so static_cc_info = cc_common.merge_cc_infos( @@ -32,9 +31,10 @@ def _py_extension_impl(ctx): ) # Dynamic deps are linked as shared libraries - dynamic_linking_context = cc_common.merge_cc_infos( - cc_infos = dynamic_deps_infos, - ).linking_context + linker_inputs = [dep.linker_input for dep in dynamic_deps_infos] + dynamic_linking_context = cc_common.create_linking_context( + linker_inputs = depset(linker_inputs), + ) user_link_flags = [] user_link_flags.append("-Wl,--export-dynamic-symbol=PyInit_{module_name}".format( @@ -50,12 +50,28 @@ def _py_extension_impl(ctx): if ctx.attr.external_deps: user_link_flags.append("-Wl,--allow-shlib-undefined") - # todo: use toolchain to determine `abi3.` infix - # todo: use toolchain to determine platform extension (pyd, so, etc) - output_filename = "{module_name}.{ext}".format( - module_name = module_name, - ext = "so", - ) + ext = _get_extension(cc_toolchain) + use_py_limited_api = ctx.attr.py_limited_api and ctx.attr.py_limited_api != "none" + if use_py_limited_api: + # check that all dependencies have compatible API versions, if defined + _check_limited_api_compatibility(ctx, ctx.attr.py_limited_api) + + output_filename = "{module_name}.abi3.{ext}".format( + module_name=module_name, + ext=ext, + ) + else: + py_toolchain = ctx.toolchains[TARGET_TOOLCHAIN_TYPE] + py_runtime = py_toolchain.py3_runtime + cc_toolchain = ctx.toolchains["@bazel_tools//tools/cpp:toolchain_type"].cc + platform_tag = _get_platform(cc_toolchain) + output_filename = "{module_name}.{pyc_tag}{abi_flags}-{platform}.{ext}".format( + module_name = module_name, + pyc_tag = py_runtime.pyc_tag, # e.g. "cpython-311" + abi_flags = py_runtime.abi_flags, # e.g. "" or "d" + platform = platform_tag, # e.g. "x86_64-linux-gnu" + ext = "so", + ) py_dso = ctx.actions.declare_file(output_filename) static_linking_context = static_cc_info.linking_context @@ -97,14 +113,26 @@ def _py_extension_impl(ctx): )) # Propagate CcInfo from dynamic and external deps, but not static ones. + dynamic_cc_info = CcInfo(linking_context = dynamic_linking_context) propagated_cc_info = cc_common.merge_cc_infos( - cc_infos = dynamic_deps_infos + external_deps_infos, + cc_infos = [dynamic_cc_info] + external_deps_infos, ) + runfiles = ctx.runfiles(files = [py_dso]) + transitive_runfiles = [] + for dep in ctx.attr.static_deps + ctx.attr.dynamic_deps + ctx.attr.external_deps: + if DefaultInfo in dep: + transitive_runfiles.append(dep[DefaultInfo].default_runfiles) + runfiles = runfiles.merge_all(transitive_runfiles) + return [ - DefaultInfo(files = depset([py_dso])), + DefaultInfo( + files = depset([py_dso]), + runfiles = runfiles, + ), PyInfo( transitive_sources = depset([py_dso]), + imports = depset([import_path]), ), propagated_cc_info, ] @@ -113,8 +141,8 @@ _MaybeBuiltinPyInfo = [[BuiltinPyInfo]] if BuiltinPyInfo != None else [] PY_EXTENSION_ATTRS = COMMON_ATTRS | { "dynamic_deps": lambda: attrb.LabelList( - providers = [CcInfo], - doc = "cc_library targets to be dynamically linked.", + providers = [CcSharedLibraryInfo], + doc = "cc_shared_library targets to be dynamically linked.", default = [], ), "external_deps": lambda: attrb.LabelList( @@ -130,6 +158,32 @@ PY_EXTENSION_ATTRS = COMMON_ATTRS | { "copts": lambda: attrb.StringList(), "linkopts": lambda: attrb.StringList(), "module_name": lambda: attrb.String(), + "py_limited_api": lambda: attrb.String( + doc = """\ + The minimum Python version to target for the Limited API (e.g., '3.8'). + + If set to a version string (e.g., '3.8') instead of 'none': + - Configures the output filename to use the simple '.abi3' suffix (e.g., + 'ext.abi3.so'). + - Strictly validates that all linked C++ dependencies (static_deps, + dynamic_deps, etc.) are binary-compatible with this target version, + failing the build if a dependency is missing the 'Py_LIMITED_API' define + or targets a newer version. + + Note: Since the py_extension rule only links pre-compiled libraries, you must + manually add the preprocessor macro to the cc_library targets that compile your + C/C++ sources, for example: + cc_library( + name = "my_impl", + srcs = ["my_code.c"], + defines = ["Py_LIMITED_API=0x03080000"], + ... + ) + + Set to 'none' (the default) to build a standard, version-specific extension. + """, + default = "none" + ), } def create_py_extension_rule_builder(**kwargs): @@ -148,3 +202,194 @@ def create_py_extension_rule_builder(**kwargs): return builder py_extension = create_py_extension_rule_builder().build() + +# Map Bazel's internal CPU names to PEP 3149 standard architecture names +_BAZEL_CPU_TO_PEP_ARCH = { + "k8": "x86_64", + "amd64": "x86_64", + "x86_64": "x86_64", + "aarch64": "aarch64", + "arm64": "arm64", + "darwin": "x86_64", # Historical Bazel Mac CPU + "darwin_x86_64": "x86_64", + "darwin_arm64": "arm64", + "x64_windows": "x86_64", + "arm64_windows": "arm64", +} + +def _get_extension(cc_toolchain): + """ + Derives the appropriate file extension from the C++ toolchain. + + Args: + cc_toolchain: The CcToolchainInfo provider (usually obtained via + ctx.toolchains["@bazel_tools//tools/cpp:toolchain_type"].cc) + + Returns: + The extension, e.g. "so" or "pyd" + """ + + # Windows uses .pyd; Unix (Linux/macOS) uses .so for Python modules + target_name = cc_toolchain.target_gnu_system_name + is_windows = "windows" in target_name or "mingw" in target_name or "msvc" in target_name + ext = "pyd" if is_windows else "so" + return ext + +def _get_platform(cc_toolchain): + """Derives the PEP 3149 platform tag from the C++ toolchain. + + Args: + cc_toolchain: The CcToolchainInfo provider (usually obtained via + ctx.toolchains["@bazel_tools//tools/cpp:toolchain_type"].cc) + + Returns: + The platform tag, e.g. "x86_64-linux-gnu" or "win_amd64" + """ + # Get the GNU target name (e.g., "local-linux-gnu" or "x86_64-unknown-linux-gnu") + target_name = cc_toolchain.target_gnu_system_name + + # Detect the OS family + is_windows = "windows" in target_name or "mingw" in target_name or "msvc" in target_name + is_mac = "apple" in target_name or "darwin" in target_name + + # Parse the architecture from the target_name + # e.g., "x86_64-unknown-linux-gnu" -> "x86_64" + target_parts = target_name.split("-") + arch = target_parts[0] + + # Handle the "local" placeholder by falling back to cc_toolchain.cpu + if arch == "local": + cpu = cc_toolchain.cpu + # Resolve the Bazel CPU name to a standard PEP architecture + arch = _BAZEL_CPU_TO_PEP_ARCH.get(cpu, cpu) + # Normalize standard names if they came from a full target_name + elif arch == "amd64": + arch = "x86_64" + elif arch == "aarch64": + arch = "arm64" if is_mac else "aarch64" + + # Derive the PEP 3149 / PEP 425 platform tag + if is_windows: + platform_tag = "win_amd64" if arch == "x86_64" else "win32" + elif is_mac: + platform_tag = "darwin" + else: + # Linux/Unix: Reconstruct the triplet, dropping the vendor if present + os_part = "linux" + abi_part = "gnu" + + if len(target_parts) == 4: + # [arch, vendor, os, abi] + os_part = target_parts[2] + abi_part = target_parts[3] + elif len(target_parts) == 3: + # [arch, os, abi] + os_part = target_parts[1] + abi_part = target_parts[2] + + platform_tag = "{}-{}-{}".format(arch, os_part, abi_part) + + return platform_tag + + +def _version_to_hex(version_str): + """Converts a version string like '3.10' to Python's version hex '0x030a0000'.""" + parts = version_str.split(".") + if len(parts) != 2: + fail("Invalid py_limited_api version '{}', expected 'major.minor' format (e.g., '3.8')".format(version_str)) + + major = int(parts[0]) + minor = int(parts[1]) + + if major != 3: + fail("Python Limited API is only supported for Python 3.2+ (got Python {})".format(major)) + if minor < 2: + fail("Python Limited API is only supported for Python 3.2+ (got 3.{})".format(minor)) + + # Format the minor version as a 2-digit hex (e.g., 10 -> "0a") + # Starlark doesn't seem to support %02x formatting + + return "0x03%x%x0000" % (int(minor/16), minor%16) + + +def _check_limited_api_compatibility(ctx, ext_version_str): + """Validates that all C++ dependencies are binary-compatible with the extension's Limited API target.""" + if ext_version_str == "none": + return + + ext_version_hex = _version_to_hex(ext_version_str) + ext_version_val = int(ext_version_hex, 16) + + # Collect all dependencies that might propagate CcInfo + deps = [] + deps.extend(ctx.attr.static_deps) + deps.extend(ctx.attr.dynamic_deps) + deps.extend(ctx.attr.external_deps) + + for dep in deps: + if CcInfo not in dep: + continue + + comp_ctx = dep[CcInfo].compilation_context + + # Detect if the dependency has access to Python headers + has_python_headers = False + for header in comp_ctx.headers.to_list(): + if header.basename == "Python.h": + has_python_headers = True + break + + # Inspect the propagated defines + has_limited_api_define = False + limited_api_define_value = None + + for define in comp_ctx.defines.to_list(): + if define.startswith("Py_LIMITED_API="): + has_limited_api_define = True + limited_api_define_value = define.split("=")[1] + elif define == "Py_LIMITED_API": + has_limited_api_define = True + limited_api_define_value = "unspecified" + + # Enforce the compatibility contract + + # Contract Rule A: If the library uses Python, it MUST use the Limited API + if has_python_headers and not has_limited_api_define: + fail(( + "\nERROR: Unsafe Python C API usage in dependency:\n" + + " Dependency '{dep}' includes Python headers (contains 'Python.h')\n" + + " but does NOT define 'Py_LIMITED_API'.\n" + + " This will link unstable Python symbols into your Stable ABI extension.\n" + + " Please add: defines = [\"Py_LIMITED_API={ext_hex}\"] to '{dep}'." + ).format( + dep = dep.label, + ext_hex = ext_version_hex, + )) + + # Contract Rule B: If the Limited API is defined, it must be version-safe + if has_limited_api_define: + if limited_api_define_value == "unspecified": + fail(( + "\nERROR: Unsafe Python Limited API definition in dependency\n" + + " Dependency '{dep}' defines 'Py_LIMITED_API' without a version hex.\n" + + " Please change it to specify the target version explicitly, " + + "for example: defines = [\"Py_LIMITED_API={ext_hex}\"]" + ).format( + dep = dep.label, + ext_hex = ext_version_hex, + )) + else: + dep_version_val = int(limited_api_define_value, 16) + if dep_version_val > ext_version_val: + fail(( + "\nERROR: Incompatible Python Limited API targets detected\n" + + " Extension '{self}' targets version '{ext_ver}' ({ext_hex}).\n" + + " Dependency '{dep}' targets a NEWER version ({dep_hex}).\n" + + " You cannot link a newer Limited API library into an older extension." + ).format( + self = ctx.label, + ext_ver = ext_version_str, + ext_hex = ext_version_hex, + dep = dep.label, + dep_hex = limited_api_define_value, + )) diff --git a/tests/cc/py_extension/BUILD.bazel b/tests/cc/py_extension/BUILD.bazel index e88690b8c1..e139798b24 100644 --- a/tests/cc/py_extension/BUILD.bazel +++ b/tests/cc/py_extension/BUILD.bazel @@ -1,9 +1,11 @@ load("@rules_cc//cc:cc_library.bzl", "cc_library") +load("@rules_cc//cc:cc_shared_library.bzl", "cc_shared_library") load("//python:py_test.bzl", "py_test") # buildifier: disable=bzl-visibility load("//python/cc:py_extension.bzl", "py_extension") load(":py_extension_tests.bzl", "py_extension_analysis_test_suite") +load(":py_limited_api_tests.bzl", "py_limited_api_test_suite") package( default_testonly = True, @@ -21,7 +23,7 @@ py_extension( py_extension( name = "ext_shared", dynamic_deps = [ - ":add_one", + ":add_one_shared", ], static_deps = [ ":ext_shared_impl", @@ -92,6 +94,25 @@ cc_library( hdrs = ["add_one_helper.h"], ) +py_extension( + name = "ext_limited", + static_deps = [":ext_limited_impl"], + py_limited_api = '3.8' +) + +cc_library( + name = "ext_limited_impl", + srcs = ["ext_limited.c"], + defines = ["Py_LIMITED_API=0x3080000"], + copts = [ + "-fPIC", + "-fvisibility=hidden", + ], + deps = [ + "@rules_python//python/cc:current_py_cc_headers", + ], +) + py_test( name = "py_extension_test", srcs = ["py_extension_test.py"], @@ -105,3 +126,7 @@ py_test( py_extension_analysis_test_suite( name = "py_extension_analysis_tests", ) + +py_limited_api_test_suite( + name = "py_limited_api_tests", +) diff --git a/tests/cc/py_extension/ext_limited.c b/tests/cc/py_extension/ext_limited.c new file mode 100644 index 0000000000..f3622c5824 --- /dev/null +++ b/tests/cc/py_extension/ext_limited.c @@ -0,0 +1,22 @@ +#include + +static PyObject* get_limited_api_version(PyObject* self, PyObject* args) { + return PyUnicode_FromFormat("0x%08x", Py_LIMITED_API); +} + +static PyMethodDef ModuleMethods[] = { + {"get_limited_api_version", get_limited_api_version, METH_NOARGS, "Get the version of the limited API this extension was compiled against."}, + {NULL, NULL, 0, NULL} +}; + +static struct PyModuleDef ext_limited_module = { + PyModuleDef_HEAD_INIT, + "ext_limited", + NULL, + -1, + ModuleMethods +}; + +PyMODINIT_FUNC PyInit_ext_limited(void) { + return PyModule_Create(&ext_limited_module); +} diff --git a/tests/cc/py_extension/py_extension_test.py b/tests/cc/py_extension/py_extension_test.py index 81f277f021..252098a46a 100644 --- a/tests/cc/py_extension/py_extension_test.py +++ b/tests/cc/py_extension/py_extension_test.py @@ -1,36 +1,45 @@ -import unittest import os -from python.runfiles import runfiles -from elftools.elf.elffile import ELFFile -from elftools.elf.dynamic import DynamicSection +import unittest + import ext_shared +from elftools.elf.dynamic import DynamicSection +from elftools.elf.elffile import ELFFile + +from python.runfiles import runfiles + class PyExtensionTest(unittest.TestCase): def test_inspect_elf(self): r = runfiles.Create() ext_path = r.Rlocation("rules_python/tests/cc/py_extension/ext_shared.so") - self.assertTrue(os.path.exists(ext_path), f"Could not find ext_shared.so at {ext_path}") + self.assertTrue( + os.path.exists(ext_path), f"Could not find ext_shared.so at {ext_path}" + ) - with open(ext_path, 'rb') as f: + with open(ext_path, "rb") as f: elf = ELFFile(f) # Check for DT_NEEDED entry for the dynamic library - dynamic_section = elf.get_section_by_name('.dynamic') + dynamic_section = elf.get_section_by_name(".dynamic") self.assertIsNotNone(dynamic_section) self.assertTrue(isinstance(dynamic_section, DynamicSection)) - needed_libs = [tag.needed for tag in dynamic_section.iter_tags() if tag.entry.d_tag == 'DT_NEEDED'] - self.assertIn('libdyn_dep_a.so', needed_libs) + needed_libs = [ + tag.needed + for tag in dynamic_section.iter_tags() + if tag.entry.d_tag == "DT_NEEDED" + ] + self.assertIn("libadd_one_shared.so", needed_libs) # Check for the PyInit symbol - dynsym_section = elf.get_section_by_name('.dynsym') + dynsym_section = elf.get_section_by_name(".dynsym") self.assertIsNotNone(dynsym_section) symbols = [s.name for s in dynsym_section.iter_symbols()] - self.assertIn('PyInit_ext_shared', symbols) + self.assertIn("PyInit_ext_shared", symbols) def test_import_and_call(self): - self.assertEqual(ext_shared.my_c_function(), 42) + self.assertEqual(ext_shared.do_alpha(), 43) if __name__ == "__main__": diff --git a/tests/cc/py_extension/py_extension_tests.bzl b/tests/cc/py_extension/py_extension_tests.bzl index 9d471cda49..514bb22ac4 100644 --- a/tests/cc/py_extension/py_extension_tests.bzl +++ b/tests/cc/py_extension/py_extension_tests.bzl @@ -22,17 +22,19 @@ load("//python/private:py_info.bzl", "PyInfo") _tests = [] def _test_static_deps_impl(env, target): - py_info = env.expect.that_target(target).has_provider(PyInfo) - cc_info = env.expect.that_target(target).has_provider(CcInfo) + env.expect.that_target(target).has_provider(PyInfo) + py_info = target[PyInfo] + env.expect.that_target(target).has_provider(CcInfo) + cc_info = target[CcInfo] # The .so should be in PyInfo - env.expect.that_collection(py_info.transitive_sources).has_size(1) - env.expect.that_collection(py_info.transitive_sources).contains_predicate( - matching.str_matches("ext_static.so$"), + env.expect.that_collection(py_info.transitive_sources.to_list()).has_size(1) + env.expect.that_depset_of_files(py_info.transitive_sources).contains_predicate( + matching.file_basename_equals("ext_static.cpython-311-x86_64-linux-gnu.so"), ) # CcInfo from static_deps should not be propagated. - env.expect.that_collection(cc_info.linking_context.linker_inputs.to_list()).is_empty() + env.expect.that_depset_of_files(cc_info.linking_context.linker_inputs).contains_exactly([]) def _test_static_deps(name): analysis_test( @@ -44,17 +46,20 @@ def _test_static_deps(name): _tests.append(_test_static_deps) def _test_dynamic_deps_impl(env, target): - py_info = env.expect.that_target(target).has_provider(PyInfo) - cc_info = env.expect.that_target(target).has_provider(CcInfo) + env.expect.that_target(target).has_provider(PyInfo) + py_info = target[PyInfo] + env.expect.that_target(target).has_provider(CcInfo) + cc_info = target[CcInfo] # The .so should be in PyInfo - env.expect.that_collection(py_info.transitive_sources).has_size(1) - env.expect.that_collection(py_info.transitive_sources).contains_predicate( - matching.str_matches("ext_shared.so$"), + env.expect.that_collection(py_info.transitive_sources.to_list()).has_size(1) + env.expect.that_depset_of_files(py_info.transitive_sources).contains_predicate( + matching.file_basename_equals("ext_shared.cpython-311-x86_64-linux-gnu.so"), ) # CcInfo from dynamic_deps should be propagated. - env.expect.that_collection(cc_info.linking_context.linker_inputs.to_list()).is_not_empty() + print(cc_info.linking_context.linker_inputs.to_list()) + env.expect.that_collection(cc_info.linking_context.linker_inputs.to_list()).has_size(1) def _test_dynamic_deps(name): analysis_test( diff --git a/tests/cc/py_extension/py_limited_api_tests.bzl b/tests/cc/py_extension/py_limited_api_tests.bzl new file mode 100644 index 0000000000..5df7892535 --- /dev/null +++ b/tests/cc/py_extension/py_limited_api_tests.bzl @@ -0,0 +1,77 @@ +# Copyright 2025 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Tests for the py_limited_api attribute for py_extension.""" + +load("@rules_testing//lib:analysis_test.bzl", "analysis_test", "test_suite") +load("@rules_testing//lib:truth.bzl", "matching") +load("@rules_testing//lib:util.bzl", "util") +load("//python/cc:py_extension.bzl", "py_extension") +load("@rules_cc//cc:cc_library.bzl", "cc_library") + + +def _test_limited_same_version(name): + # given + util.helper_target( + cc_library, + name = name + '_csl', + defines = ["Py_LIMITED_API=0x3080000"], + deps = [ + "@rules_python//python/cc:current_py_cc_headers", + ], + ) + py_extension( + name = name + '_pyext', + static_deps = [':' + name + '_csl'], + py_limited_api = '3.8', + ) + + # when + analysis_test( + name = name, + target = name + "_pyext", + impl=_test_limited_same_version_impl) + +def _test_limited_same_version_impl(env, target): + # then + env.expect.that_target(target).default_outputs().contains( + "tests/cc/py_extension/test_limited_same_version_pyext.abi3.so" + ) + +# test cases: +# py_limited_api +# - 3.8 -> 3.9 +# - 3.9 -> 3.8 +# - 3.9 -> 3.9 ok +# - none -> 3.8 +# - 3.8 -> none fail +# - 3.8 -> nopy ok +# - none -> none ok? +# - none -> nopy ok? +# invalid values for version string +# - 2.x +# - 3.0 and 3.1 +# - 4.x +# - not version string, e.g. "asdf" +# - patch versions? 3.8.4 ? +# - empty string or null? + + +def py_limited_api_test_suite(name): + test_suite( + name = name, + tests = [ + _test_limited_same_version, + ], + ) From 82144224d5b5e6738e8b4ea962d1a7bfcf6f3000 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Fri, 26 Jun 2026 19:28:55 +0000 Subject: [PATCH 09/35] Don't leave a trailing slash for the root package. --- python/private/cc/py_extension_rule.bzl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 19a0281288..23a387b140 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -13,7 +13,9 @@ load("//python/private:toolchain_types.bzl", "TARGET_TOOLCHAIN_TYPE") def _py_extension_impl(ctx): module_name = ctx.attr.module_name or ctx.label.name repo_name = ctx.label.workspace_name or ctx.workspace_name - import_path = repo_name + "/" + ctx.label.package + import_path = repo_name + if ctx.label.package: + import_path = repo_name + "/" + ctx.label.package cc_toolchain = ctx.toolchains["@bazel_tools//tools/cpp:toolchain_type"].cc feature_configuration = cc_common.configure_features( ctx = ctx, From 1e89b9972ba66c47865f66d20a1282ccd39aa6c2 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Fri, 26 Jun 2026 19:32:08 +0000 Subject: [PATCH 10/35] Remove duplicate assignment. --- python/private/cc/py_extension_rule.bzl | 1 - 1 file changed, 1 deletion(-) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 23a387b140..a72a3a5f80 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -65,7 +65,6 @@ def _py_extension_impl(ctx): else: py_toolchain = ctx.toolchains[TARGET_TOOLCHAIN_TYPE] py_runtime = py_toolchain.py3_runtime - cc_toolchain = ctx.toolchains["@bazel_tools//tools/cpp:toolchain_type"].cc platform_tag = _get_platform(cc_toolchain) output_filename = "{module_name}.{pyc_tag}{abi_flags}-{platform}.{ext}".format( module_name = module_name, From d921d20912b79eb32abef5d471da1f5c3e61ffff Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Fri, 26 Jun 2026 19:36:22 +0000 Subject: [PATCH 11/35] Remove typical C-style integer suffixes before parsing. --- python/private/cc/py_extension_rule.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index a72a3a5f80..3f5124b4ed 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -380,7 +380,7 @@ def _check_limited_api_compatibility(ctx, ext_version_str): ext_hex = ext_version_hex, )) else: - dep_version_val = int(limited_api_define_value, 16) + dep_version_val = int(limited_api_define_value.rstrip("ULul"), 16) if dep_version_val > ext_version_val: fail(( "\nERROR: Incompatible Python Limited API targets detected\n" + From f318173e62cde2149f0f4b93e88ef9ef95f04b78 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Fri, 26 Jun 2026 19:42:26 +0000 Subject: [PATCH 12/35] Fix check for filename. --- tests/cc/py_extension/py_extension_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cc/py_extension/py_extension_test.py b/tests/cc/py_extension/py_extension_test.py index 252098a46a..3d71e453fd 100644 --- a/tests/cc/py_extension/py_extension_test.py +++ b/tests/cc/py_extension/py_extension_test.py @@ -11,7 +11,7 @@ class PyExtensionTest(unittest.TestCase): def test_inspect_elf(self): r = runfiles.Create() - ext_path = r.Rlocation("rules_python/tests/cc/py_extension/ext_shared.so") + ext_path = r.Rlocation("rules_python/tests/cc/py_extension/ext_shared.cpython-311-x86_64-linux-gnu.so") self.assertTrue( os.path.exists(ext_path), f"Could not find ext_shared.so at {ext_path}" ) From 80a9aae8f326bee4cf56a34c7a609ee27fa68224 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Fri, 26 Jun 2026 20:34:25 +0000 Subject: [PATCH 13/35] Remove unnecessary guard. --- python/private/cc/py_extension_rule.bzl | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 3f5124b4ed..12373fdc06 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -122,8 +122,7 @@ def _py_extension_impl(ctx): runfiles = ctx.runfiles(files = [py_dso]) transitive_runfiles = [] for dep in ctx.attr.static_deps + ctx.attr.dynamic_deps + ctx.attr.external_deps: - if DefaultInfo in dep: - transitive_runfiles.append(dep[DefaultInfo].default_runfiles) + transitive_runfiles.append(dep[DefaultInfo].default_runfiles) runfiles = runfiles.merge_all(transitive_runfiles) return [ From 9bfc33112271567993133bb8da4b3b72229da4ca Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Fri, 26 Jun 2026 20:37:28 +0000 Subject: [PATCH 14/35] Re-format docstring. --- python/private/cc/py_extension_rule.bzl | 47 +++++++++++++------------ 1 file changed, 24 insertions(+), 23 deletions(-) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 12373fdc06..3ce2798a70 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -159,29 +159,30 @@ PY_EXTENSION_ATTRS = COMMON_ATTRS | { "linkopts": lambda: attrb.StringList(), "module_name": lambda: attrb.String(), "py_limited_api": lambda: attrb.String( - doc = """\ - The minimum Python version to target for the Limited API (e.g., '3.8'). - - If set to a version string (e.g., '3.8') instead of 'none': - - Configures the output filename to use the simple '.abi3' suffix (e.g., - 'ext.abi3.so'). - - Strictly validates that all linked C++ dependencies (static_deps, - dynamic_deps, etc.) are binary-compatible with this target version, - failing the build if a dependency is missing the 'Py_LIMITED_API' define - or targets a newer version. - - Note: Since the py_extension rule only links pre-compiled libraries, you must - manually add the preprocessor macro to the cc_library targets that compile your - C/C++ sources, for example: - cc_library( - name = "my_impl", - srcs = ["my_code.c"], - defines = ["Py_LIMITED_API=0x03080000"], - ... - ) - - Set to 'none' (the default) to build a standard, version-specific extension. - """, + doc = """ +The minimum Python version to target for the Limited API (e.g., '3.8'). + +If set to a version string (e.g., '3.8') instead of 'none': + - Configures the output filename to use the simple '.abi3' suffix + (e.g., 'ext.abi3.so'). + - Strictly validates that all linked C++ dependencies (static_deps, + dynamic_deps, etc.) are binary-compatible with this target version, + failing the build if a dependency is missing the 'Py_LIMITED_API' + define or targets a newer version. + +Note: Since the py_extension rule only links pre-compiled libraries, +you must manually add the preprocessor macro to the cc_library targets +that compile your C/C++ sources, for example: + cc_library( + name = "my_impl", + srcs = ["my_code.c"], + defines = ["Py_LIMITED_API=0x03080000"], + ... + ) + +Set to 'none' (the default) to build a standard, version-specific +extension. +""", default = "none" ), } From e85e06b1fd71c999910370877363e9fc02333f80 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Mon, 29 Jun 2026 20:34:30 +0000 Subject: [PATCH 15/35] Get the platform from the constraints. --- python/private/cc/py_extension_rule.bzl | 75 ++++++++++++++++++++++++- 1 file changed, 72 insertions(+), 3 deletions(-) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 3ce2798a70..b1910a6959 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -2,6 +2,7 @@ load("@rules_cc//cc/common:cc_common.bzl", "cc_common") load("@rules_cc//cc/common:cc_info.bzl", "CcInfo") +load("//python:versions.bzl", "PLATFORMS") load("//python/private:attr_builders.bzl", "attrb") load("//python/private:attributes.bzl", "COMMON_ATTRS") load("//python/private:py_info.bzl", "PyInfo") @@ -65,7 +66,7 @@ def _py_extension_impl(ctx): else: py_toolchain = ctx.toolchains[TARGET_TOOLCHAIN_TYPE] py_runtime = py_toolchain.py3_runtime - platform_tag = _get_platform(cc_toolchain) + platform_tag = _get_platform(ctx, cc_toolchain) output_filename = "{module_name}.{pyc_tag}{abi_flags}-{platform}.{ext}".format( module_name = module_name, pyc_tag = py_runtime.pyc_tag, # e.g. "cpython-311" @@ -185,6 +186,20 @@ extension. """, default = "none" ), + "_constraints": lambda: attrb.LabelList( + default = [ + "@platforms//os:linux", + "@platforms//os:macos", + "@platforms//os:windows", + "@platforms//cpu:x86_64", + "@platforms//cpu:aarch64", + "@platforms//cpu:armv7", + "@platforms//cpu:i386", + "@platforms//cpu:ppc", + "@platforms//cpu:riscv64", + "@platforms//cpu:s390x", + ], + ), } def create_py_extension_rule_builder(**kwargs): @@ -236,16 +251,70 @@ def _get_extension(cc_toolchain): ext = "pyd" if is_windows else "so" return ext -def _get_platform(cc_toolchain): - """Derives the PEP 3149 platform tag from the C++ toolchain. +def _derive_pep3149_tag(platform, info): + # platform is the triplet, e.g. "x86_64-unknown-linux-gnu" + p, _, _ = platform.partition("-freethreaded") + parts = p.split("-") + triplet_arch = parts[0] + + if info.os_name == "windows": + if triplet_arch == "x86_64": + return "win_amd64" + elif triplet_arch == "aarch64": + return "win_arm64" + else: + return "win32" + elif info.os_name == "osx": + return "darwin" + elif info.os_name == "linux": + abi = "musl" if p.endswith("-musl") else "gnu" + return "{}-linux-{}".format(triplet_arch, abi) + else: + return triplet_arch + +def _get_platform_from_constraints(ctx): + # Build a map of Label to ConstraintValueInfo from _constraints + constraints_map = {} + for c in ctx.attr._constraints: + if platform_common.ConstraintValueInfo in c: + constraints_map[c.label] = c[platform_common.ConstraintValueInfo] + + # Find the matching platform in PLATFORMS + for platform, info in PLATFORMS.items(): + # Check if all compatible_with constraints are satisfied + match = True + for c_str in info.compatible_with: + c_label = Label(c_str) + if c_label in constraints_map: + c_val = constraints_map[c_label] + if not ctx.target_platform_has_constraint(c_val): + match = False + break + else: + match = False + break + if match: + return _derive_pep3149_tag(platform, info) + + return None + +def _get_platform(ctx, cc_toolchain): + """Derives the PEP 3149 platform tag from the C++ toolchain or target constraints. Args: + ctx: The rule context. cc_toolchain: The CcToolchainInfo provider (usually obtained via ctx.toolchains["@bazel_tools//tools/cpp:toolchain_type"].cc) Returns: The platform tag, e.g. "x86_64-linux-gnu" or "win_amd64" """ + # Try to resolve using modern platform constraints and PLATFORMS + platform_tag = _get_platform_from_constraints(ctx) + if platform_tag: + return platform_tag + + # Fallback to legacy cc_toolchain parsing # Get the GNU target name (e.g., "local-linux-gnu" or "x86_64-unknown-linux-gnu") target_name = cc_toolchain.target_gnu_system_name From 590b66716f1d9477e9dfd6a65e6a56e3f4af20a1 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Mon, 29 Jun 2026 21:14:43 +0000 Subject: [PATCH 16/35] Adjust formatting syntax. --- python/private/cc/py_extension_rule.bzl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index b1910a6959..f31904354f 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -379,7 +379,7 @@ def _version_to_hex(version_str): # Format the minor version as a 2-digit hex (e.g., 10 -> "0a") # Starlark doesn't seem to support %02x formatting - return "0x03%x%x0000" % (int(minor/16), minor%16) + return "0x03%x%x0000" % (minor//16, minor%16) def _check_limited_api_compatibility(ctx, ext_version_str): From 15c95f868a4baba5c14ca06e53d002e21801bc2e Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Mon, 29 Jun 2026 21:22:43 +0000 Subject: [PATCH 17/35] Change the default value. --- python/private/cc/py_extension_rule.bzl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index f31904354f..a4c695831a 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -54,7 +54,7 @@ def _py_extension_impl(ctx): user_link_flags.append("-Wl,--allow-shlib-undefined") ext = _get_extension(cc_toolchain) - use_py_limited_api = ctx.attr.py_limited_api and ctx.attr.py_limited_api != "none" + use_py_limited_api = bool(ctx.attr.py_limited_api) if use_py_limited_api: # check that all dependencies have compatible API versions, if defined _check_limited_api_compatibility(ctx, ctx.attr.py_limited_api) @@ -163,7 +163,7 @@ PY_EXTENSION_ATTRS = COMMON_ATTRS | { doc = """ The minimum Python version to target for the Limited API (e.g., '3.8'). -If set to a version string (e.g., '3.8') instead of 'none': +If set to a version string (e.g., '3.8') instead of '' (empty string): - Configures the output filename to use the simple '.abi3' suffix (e.g., 'ext.abi3.so'). - Strictly validates that all linked C++ dependencies (static_deps, @@ -181,10 +181,10 @@ that compile your C/C++ sources, for example: ... ) -Set to 'none' (the default) to build a standard, version-specific +Set to '' (the default) or None to build a standard, version-specific extension. """, - default = "none" + default = "" ), "_constraints": lambda: attrb.LabelList( default = [ @@ -384,7 +384,7 @@ def _version_to_hex(version_str): def _check_limited_api_compatibility(ctx, ext_version_str): """Validates that all C++ dependencies are binary-compatible with the extension's Limited API target.""" - if ext_version_str == "none": + if not ext_version_str: return ext_version_hex = _version_to_hex(ext_version_str) From daa24558227bf86edf3b8c4f3138324de60cf334 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Mon, 29 Jun 2026 21:25:35 +0000 Subject: [PATCH 18/35] Add more test cases for py_limited_api. --- .../cc/py_extension/py_limited_api_tests.bzl | 222 +++++++++++++++--- 1 file changed, 195 insertions(+), 27 deletions(-) diff --git a/tests/cc/py_extension/py_limited_api_tests.bzl b/tests/cc/py_extension/py_limited_api_tests.bzl index 5df7892535..895a38a318 100644 --- a/tests/cc/py_extension/py_limited_api_tests.bzl +++ b/tests/cc/py_extension/py_limited_api_tests.bzl @@ -20,13 +20,16 @@ load("@rules_testing//lib:util.bzl", "util") load("//python/cc:py_extension.bzl", "py_extension") load("@rules_cc//cc:cc_library.bzl", "cc_library") +def _test_limited_pass_impl(env, target): + env.expect.that_target(target).default_outputs().contains( + "tests/cc/py_extension/{}.abi3.so".format(target.label.name) + ) def _test_limited_same_version(name): - # given util.helper_target( cc_library, name = name + '_csl', - defines = ["Py_LIMITED_API=0x3080000"], + defines = ["Py_LIMITED_API=0x03080000"], deps = [ "@rules_python//python/cc:current_py_cc_headers", ], @@ -36,42 +39,207 @@ def _test_limited_same_version(name): static_deps = [':' + name + '_csl'], py_limited_api = '3.8', ) + analysis_test( + name = name, + target = name + "_pyext", + impl = _test_limited_pass_impl, + ) - # when +def _test_limited_older_dep(name): + util.helper_target( + cc_library, + name = name + '_csl', + defines = ["Py_LIMITED_API=0x03080000"], # 3.8 + deps = [ + "@rules_python//python/cc:current_py_cc_headers", + ], + ) + py_extension( + name = name + '_pyext', + static_deps = [':' + name + '_csl'], + py_limited_api = '3.9', # 3.9 + ) analysis_test( name = name, target = name + "_pyext", - impl=_test_limited_same_version_impl) + impl = _test_limited_pass_impl, + ) -def _test_limited_same_version_impl(env, target): - # then - env.expect.that_target(target).default_outputs().contains( - "tests/cc/py_extension/test_limited_same_version_pyext.abi3.so" - ) - -# test cases: -# py_limited_api -# - 3.8 -> 3.9 -# - 3.9 -> 3.8 -# - 3.9 -> 3.9 ok -# - none -> 3.8 -# - 3.8 -> none fail -# - 3.8 -> nopy ok -# - none -> none ok? -# - none -> nopy ok? -# invalid values for version string -# - 2.x -# - 3.0 and 3.1 -# - 4.x -# - not version string, e.g. "asdf" -# - patch versions? 3.8.4 ? -# - empty string or null? +def _test_limited_newer_dep(name): + util.helper_target( + cc_library, + name = name + '_csl', + defines = ["Py_LIMITED_API=0x03090000"], # 3.9 + deps = [ + "@rules_python//python/cc:current_py_cc_headers", + ], + ) + py_extension( + name = name + '_pyext', + static_deps = [':' + name + '_csl'], + py_limited_api = '3.8', # 3.8 + ) + analysis_test( + name = name, + target = name + "_pyext", + impl = _test_limited_newer_dep_impl, + expect_failure = True, + ) + +def _test_limited_newer_dep_impl(env, target): + env.expect.that_target(target).failures().contains_predicate( + matching.str_matches("*Incompatible Python Limited API targets detected*"), + ) + +def _test_limited_dep_missing_define(name): + util.helper_target( + cc_library, + name = name + '_csl', + deps = [ + "@rules_python//python/cc:current_py_cc_headers", + ], + ) + py_extension( + name = name + '_pyext', + static_deps = [':' + name + '_csl'], + py_limited_api = '3.8', + ) + analysis_test( + name = name, + target = name + "_pyext", + impl = _test_limited_dep_missing_define_impl, + expect_failure = True, + ) +def _test_limited_dep_missing_define_impl(env, target): + env.expect.that_target(target).failures().contains_predicate( + matching.str_matches("*Unsafe Python C API usage in dependency*"), + ) + +def _test_limited_dep_unspecified_define(name): + util.helper_target( + cc_library, + name = name + '_csl', + defines = ["Py_LIMITED_API"], + deps = [ + "@rules_python//python/cc:current_py_cc_headers", + ], + ) + py_extension( + name = name + '_pyext', + static_deps = [':' + name + '_csl'], + py_limited_api = '3.8', + ) + analysis_test( + name = name, + target = name + "_pyext", + impl = _test_limited_dep_unspecified_define_impl, + expect_failure = True, + ) + +def _test_limited_dep_unspecified_define_impl(env, target): + env.expect.that_target(target).failures().contains_predicate( + matching.str_matches("*Unsafe Python Limited API definition in dependency*"), + ) + +def _test_no_limited_api(name): + util.helper_target( + cc_library, + name = name + '_csl', + deps = [ + "@rules_python//python/cc:current_py_cc_headers", + ], + ) + py_extension( + name = name + '_pyext', + static_deps = [':' + name + '_csl'], + ) + analysis_test( + name = name, + target = name + "_pyext", + impl = _test_no_limited_api_impl, + ) + +def _test_no_limited_api_impl(env, target): + # Should pass, nothing to assert on filename since it is platform-specific + pass + +def _test_no_limited_api_dep_has_limited(name): + util.helper_target( + cc_library, + name = name + '_csl', + defines = ["Py_LIMITED_API=0x03080000"], + deps = [ + "@rules_python//python/cc:current_py_cc_headers", + ], + ) + py_extension( + name = name + '_pyext', + static_deps = [':' + name + '_csl'], + ) + analysis_test( + name = name, + target = name + "_pyext", + impl = _test_no_limited_api_dep_has_limited_impl, + ) + +def _test_no_limited_api_dep_has_limited_impl(env, target): + pass + +def _test_limited_api_dep_has_no_python(name): + util.helper_target( + cc_library, + name = name + '_csl', + ) + py_extension( + name = name + '_pyext', + static_deps = [':' + name + '_csl'], + py_limited_api = '3.8', + ) + analysis_test( + name = name, + target = name + "_pyext", + impl = _test_limited_pass_impl, + ) + +def _test_invalid_version_format(name): + util.helper_target( + cc_library, + name = name + '_csl', + defines = ["Py_LIMITED_API=0x03080000"], + deps = [ + "@rules_python//python/cc:current_py_cc_headers", + ], + ) + py_extension( + name = name + '_pyext', + static_deps = [':' + name + '_csl'], + py_limited_api = '3.8.1', + ) + analysis_test( + name = name, + target = name + "_pyext", + impl = _test_invalid_version_format_impl, + expect_failure = True, + ) + +def _test_invalid_version_format_impl(env, target): + env.expect.that_target(target).failures().contains_predicate( + matching.str_matches("*Invalid py_limited_api version*"), + ) def py_limited_api_test_suite(name): test_suite( name = name, tests = [ _test_limited_same_version, + _test_limited_older_dep, + _test_limited_newer_dep, + _test_limited_dep_missing_define, + _test_limited_dep_unspecified_define, + _test_no_limited_api, + _test_no_limited_api_dep_has_limited, + _test_limited_api_dep_has_no_python, + _test_invalid_version_format, ], ) From 030c87952d1dd26d4f7f64e129f94d9cea4f75f7 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Mon, 29 Jun 2026 22:18:29 +0000 Subject: [PATCH 19/35] Use RunfilesBuilder instead of manual. --- python/private/cc/py_extension_rule.bzl | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index a4c695831a..674d994b6b 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -5,6 +5,7 @@ load("@rules_cc//cc/common:cc_info.bzl", "CcInfo") load("//python:versions.bzl", "PLATFORMS") load("//python/private:attr_builders.bzl", "attrb") load("//python/private:attributes.bzl", "COMMON_ATTRS") +load("//python/private:builders.bzl", "builders") load("//python/private:py_info.bzl", "PyInfo") load("//python/private:py_internal.bzl", "py_internal") load("//python/private:reexports.bzl", "BuiltinPyInfo") @@ -120,11 +121,12 @@ def _py_extension_impl(ctx): cc_infos = [dynamic_cc_info] + external_deps_infos, ) - runfiles = ctx.runfiles(files = [py_dso]) - transitive_runfiles = [] - for dep in ctx.attr.static_deps + ctx.attr.dynamic_deps + ctx.attr.external_deps: - transitive_runfiles.append(dep[DefaultInfo].default_runfiles) - runfiles = runfiles.merge_all(transitive_runfiles) + runfiles_builder = builders.RunfilesBuilder() + runfiles_builder.add(py_dso) + runfiles_builder.add_targets(ctx.attr.static_deps) + runfiles_builder.add_targets(ctx.attr.dynamic_deps) + runfiles_builder.add_targets(ctx.attr.external_deps) + runfiles = runfiles_builder.build(ctx) return [ DefaultInfo( From 73eb7c0952899d9dbdb603eb2219afd17e933296 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 30 Jun 2026 01:15:25 +0000 Subject: [PATCH 20/35] Add abi_tag to PyCcToolchainInfo. --- python/private/py_cc_toolchain_info.bzl | 5 +++++ python/private/py_cc_toolchain_rule.bzl | 11 +++++++++++ 2 files changed, 16 insertions(+) diff --git a/python/private/py_cc_toolchain_info.bzl b/python/private/py_cc_toolchain_info.bzl index 8cb3680b59..da7938503f 100644 --- a/python/private/py_cc_toolchain_info.bzl +++ b/python/private/py_cc_toolchain_info.bzl @@ -17,6 +17,11 @@ PyCcToolchainInfo = provider( doc = "C/C++ information about the Python runtime.", fields = { + "abi_tag": """\ +:type: str + +The ABI tag for extension modules, e.g. 'cpython-311' or 'cpython-313t'. +""", "headers": """\ :type: struct diff --git a/python/private/py_cc_toolchain_rule.bzl b/python/private/py_cc_toolchain_rule.bzl index b5c997ea6e..4067df668d 100644 --- a/python/private/py_cc_toolchain_rule.bzl +++ b/python/private/py_cc_toolchain_rule.bzl @@ -45,7 +45,14 @@ def _py_cc_toolchain_impl(ctx): else: headers_abi3 = None + abi_tag = ctx.attr.abi_tag + if not abi_tag: + # Derive default: cpython-XX + version_parts = ctx.attr.python_version.split(".") + abi_tag = "cpython-{}{}".format(version_parts[0], version_parts[1]) + py_cc_toolchain = PyCcToolchainInfo( + abi_tag = abi_tag, headers = struct( providers_map = { "CcInfo": ctx.attr.headers[CcInfo], @@ -67,6 +74,10 @@ def _py_cc_toolchain_impl(ctx): py_cc_toolchain = rule( implementation = _py_cc_toolchain_impl, attrs = { + "abi_tag": attr.string( + doc = "The ABI tag for extension modules, e.g. 'cpython-311'", + default = "", + ), "headers": attr.label( doc = ("Target that provides the Python headers. Typically this " + "is a cc_library target."), From 5cb7c9072b8a2383cabb63e58b7784d2d5dba2b9 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 30 Jun 2026 01:15:53 +0000 Subject: [PATCH 21/35] Use the PyCcToolchainInfo to derive the filename instead of the runtime toolchain. --- python/private/cc/py_extension_rule.bzl | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 674d994b6b..2f71799e3c 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -10,7 +10,7 @@ load("//python/private:py_info.bzl", "PyInfo") load("//python/private:py_internal.bzl", "py_internal") load("//python/private:reexports.bzl", "BuiltinPyInfo") load("//python/private:rule_builders.bzl", "ruleb") -load("//python/private:toolchain_types.bzl", "TARGET_TOOLCHAIN_TYPE") +load("//python/private:toolchain_types.bzl", "PY_CC_TOOLCHAIN_TYPE") def _py_extension_impl(ctx): module_name = ctx.attr.module_name or ctx.label.name @@ -65,15 +65,14 @@ def _py_extension_impl(ctx): ext=ext, ) else: - py_toolchain = ctx.toolchains[TARGET_TOOLCHAIN_TYPE] - py_runtime = py_toolchain.py3_runtime + py_toolchain = ctx.toolchains[PY_CC_TOOLCHAIN_TYPE] + py_cc_toolchain = py_toolchain.py_cc_toolchain platform_tag = _get_platform(ctx, cc_toolchain) - output_filename = "{module_name}.{pyc_tag}{abi_flags}-{platform}.{ext}".format( + output_filename = "{module_name}.{abi_tag}-{platform}.{ext}".format( module_name = module_name, - pyc_tag = py_runtime.pyc_tag, # e.g. "cpython-311" - abi_flags = py_runtime.abi_flags, # e.g. "" or "d" - platform = platform_tag, # e.g. "x86_64-linux-gnu" - ext = "so", + abi_tag = py_cc_toolchain.abi_tag, + platform = platform_tag, + ext = ext, ) py_dso = ctx.actions.declare_file(output_filename) @@ -211,7 +210,7 @@ def create_py_extension_rule_builder(**kwargs): attrs = PY_EXTENSION_ATTRS, provides = [PyInfo, CcInfo], toolchains = [ - ruleb.ToolchainType(TARGET_TOOLCHAIN_TYPE), + ruleb.ToolchainType(PY_CC_TOOLCHAIN_TYPE), ruleb.ToolchainType("@bazel_tools//tools/cpp:toolchain_type"), ], fragments = ["cpp"], From 797bf98e9ecc045ebbe499d1239acaf9582d50f3 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 30 Jun 2026 03:46:05 +0000 Subject: [PATCH 22/35] Remove the fallback. Determine platform tag solely from constraints. --- python/private/cc/py_extension_rule.bzl | 77 ++++--------------------- 1 file changed, 12 insertions(+), 65 deletions(-) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 2f71799e3c..409640b8ad 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -67,7 +67,7 @@ def _py_extension_impl(ctx): else: py_toolchain = ctx.toolchains[PY_CC_TOOLCHAIN_TYPE] py_cc_toolchain = py_toolchain.py_cc_toolchain - platform_tag = _get_platform(ctx, cc_toolchain) + platform_tag = _get_platform(ctx) output_filename = "{module_name}.{abi_tag}-{platform}.{ext}".format( module_name = module_name, abi_tag = py_cc_toolchain.abi_tag, @@ -220,19 +220,6 @@ def create_py_extension_rule_builder(**kwargs): py_extension = create_py_extension_rule_builder().build() -# Map Bazel's internal CPU names to PEP 3149 standard architecture names -_BAZEL_CPU_TO_PEP_ARCH = { - "k8": "x86_64", - "amd64": "x86_64", - "x86_64": "x86_64", - "aarch64": "aarch64", - "arm64": "arm64", - "darwin": "x86_64", # Historical Bazel Mac CPU - "darwin_x86_64": "x86_64", - "darwin_arm64": "arm64", - "x64_windows": "x86_64", - "arm64_windows": "arm64", -} def _get_extension(cc_toolchain): """ @@ -299,68 +286,28 @@ def _get_platform_from_constraints(ctx): return None -def _get_platform(ctx, cc_toolchain): - """Derives the PEP 3149 platform tag from the C++ toolchain or target constraints. +def _get_platform(ctx): + """Derives the PEP 3149 platform tag from the target constraints. Args: ctx: The rule context. - cc_toolchain: The CcToolchainInfo provider (usually obtained via - ctx.toolchains["@bazel_tools//tools/cpp:toolchain_type"].cc) Returns: The platform tag, e.g. "x86_64-linux-gnu" or "win_amd64" """ - # Try to resolve using modern platform constraints and PLATFORMS platform_tag = _get_platform_from_constraints(ctx) if platform_tag: return platform_tag - # Fallback to legacy cc_toolchain parsing - # Get the GNU target name (e.g., "local-linux-gnu" or "x86_64-unknown-linux-gnu") - target_name = cc_toolchain.target_gnu_system_name - - # Detect the OS family - is_windows = "windows" in target_name or "mingw" in target_name or "msvc" in target_name - is_mac = "apple" in target_name or "darwin" in target_name - - # Parse the architecture from the target_name - # e.g., "x86_64-unknown-linux-gnu" -> "x86_64" - target_parts = target_name.split("-") - arch = target_parts[0] - - # Handle the "local" placeholder by falling back to cc_toolchain.cpu - if arch == "local": - cpu = cc_toolchain.cpu - # Resolve the Bazel CPU name to a standard PEP architecture - arch = _BAZEL_CPU_TO_PEP_ARCH.get(cpu, cpu) - # Normalize standard names if they came from a full target_name - elif arch == "amd64": - arch = "x86_64" - elif arch == "aarch64": - arch = "arm64" if is_mac else "aarch64" - - # Derive the PEP 3149 / PEP 425 platform tag - if is_windows: - platform_tag = "win_amd64" if arch == "x86_64" else "win32" - elif is_mac: - platform_tag = "darwin" - else: - # Linux/Unix: Reconstruct the triplet, dropping the vendor if present - os_part = "linux" - abi_part = "gnu" - - if len(target_parts) == 4: - # [arch, vendor, os, abi] - os_part = target_parts[2] - abi_part = target_parts[3] - elif len(target_parts) == 3: - # [arch, os, abi] - os_part = target_parts[1] - abi_part = target_parts[2] - - platform_tag = "{}-{}-{}".format(arch, os_part, abi_part) - - return platform_tag + fail( + """ +ERROR: Unsupported target platform for {self}. + The target platform's constraints do not match any supported platform + in rules_python's central registry (python/versions.bzl). + Please ensure your target platform is configured correctly.""".format( + self = ctx.label, + ) + ) def _version_to_hex(version_str): From 09a2e0a559b01b9b083d41fd39157bc4afd2077c Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 30 Jun 2026 04:01:51 +0000 Subject: [PATCH 23/35] Remove the version compatibility check, as it has performance implications and may produce false-positives. --- python/private/cc/py_extension_rule.bzl | 106 ----------------- .../cc/py_extension/py_limited_api_tests.bzl | 107 ------------------ 2 files changed, 213 deletions(-) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 409640b8ad..54a8404d67 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -57,9 +57,6 @@ def _py_extension_impl(ctx): ext = _get_extension(cc_toolchain) use_py_limited_api = bool(ctx.attr.py_limited_api) if use_py_limited_api: - # check that all dependencies have compatible API versions, if defined - _check_limited_api_compatibility(ctx, ctx.attr.py_limited_api) - output_filename = "{module_name}.abi3.{ext}".format( module_name=module_name, ext=ext, @@ -308,106 +305,3 @@ ERROR: Unsupported target platform for {self}. self = ctx.label, ) ) - - -def _version_to_hex(version_str): - """Converts a version string like '3.10' to Python's version hex '0x030a0000'.""" - parts = version_str.split(".") - if len(parts) != 2: - fail("Invalid py_limited_api version '{}', expected 'major.minor' format (e.g., '3.8')".format(version_str)) - - major = int(parts[0]) - minor = int(parts[1]) - - if major != 3: - fail("Python Limited API is only supported for Python 3.2+ (got Python {})".format(major)) - if minor < 2: - fail("Python Limited API is only supported for Python 3.2+ (got 3.{})".format(minor)) - - # Format the minor version as a 2-digit hex (e.g., 10 -> "0a") - # Starlark doesn't seem to support %02x formatting - - return "0x03%x%x0000" % (minor//16, minor%16) - - -def _check_limited_api_compatibility(ctx, ext_version_str): - """Validates that all C++ dependencies are binary-compatible with the extension's Limited API target.""" - if not ext_version_str: - return - - ext_version_hex = _version_to_hex(ext_version_str) - ext_version_val = int(ext_version_hex, 16) - - # Collect all dependencies that might propagate CcInfo - deps = [] - deps.extend(ctx.attr.static_deps) - deps.extend(ctx.attr.dynamic_deps) - deps.extend(ctx.attr.external_deps) - - for dep in deps: - if CcInfo not in dep: - continue - - comp_ctx = dep[CcInfo].compilation_context - - # Detect if the dependency has access to Python headers - has_python_headers = False - for header in comp_ctx.headers.to_list(): - if header.basename == "Python.h": - has_python_headers = True - break - - # Inspect the propagated defines - has_limited_api_define = False - limited_api_define_value = None - - for define in comp_ctx.defines.to_list(): - if define.startswith("Py_LIMITED_API="): - has_limited_api_define = True - limited_api_define_value = define.split("=")[1] - elif define == "Py_LIMITED_API": - has_limited_api_define = True - limited_api_define_value = "unspecified" - - # Enforce the compatibility contract - - # Contract Rule A: If the library uses Python, it MUST use the Limited API - if has_python_headers and not has_limited_api_define: - fail(( - "\nERROR: Unsafe Python C API usage in dependency:\n" + - " Dependency '{dep}' includes Python headers (contains 'Python.h')\n" + - " but does NOT define 'Py_LIMITED_API'.\n" + - " This will link unstable Python symbols into your Stable ABI extension.\n" + - " Please add: defines = [\"Py_LIMITED_API={ext_hex}\"] to '{dep}'." - ).format( - dep = dep.label, - ext_hex = ext_version_hex, - )) - - # Contract Rule B: If the Limited API is defined, it must be version-safe - if has_limited_api_define: - if limited_api_define_value == "unspecified": - fail(( - "\nERROR: Unsafe Python Limited API definition in dependency\n" + - " Dependency '{dep}' defines 'Py_LIMITED_API' without a version hex.\n" + - " Please change it to specify the target version explicitly, " + - "for example: defines = [\"Py_LIMITED_API={ext_hex}\"]" - ).format( - dep = dep.label, - ext_hex = ext_version_hex, - )) - else: - dep_version_val = int(limited_api_define_value.rstrip("ULul"), 16) - if dep_version_val > ext_version_val: - fail(( - "\nERROR: Incompatible Python Limited API targets detected\n" + - " Extension '{self}' targets version '{ext_ver}' ({ext_hex}).\n" + - " Dependency '{dep}' targets a NEWER version ({dep_hex}).\n" + - " You cannot link a newer Limited API library into an older extension." - ).format( - self = ctx.label, - ext_ver = ext_version_str, - ext_hex = ext_version_hex, - dep = dep.label, - dep_hex = limited_api_define_value, - )) diff --git a/tests/cc/py_extension/py_limited_api_tests.bzl b/tests/cc/py_extension/py_limited_api_tests.bzl index 895a38a318..5e344dbc31 100644 --- a/tests/cc/py_extension/py_limited_api_tests.bzl +++ b/tests/cc/py_extension/py_limited_api_tests.bzl @@ -65,83 +65,6 @@ def _test_limited_older_dep(name): impl = _test_limited_pass_impl, ) -def _test_limited_newer_dep(name): - util.helper_target( - cc_library, - name = name + '_csl', - defines = ["Py_LIMITED_API=0x03090000"], # 3.9 - deps = [ - "@rules_python//python/cc:current_py_cc_headers", - ], - ) - py_extension( - name = name + '_pyext', - static_deps = [':' + name + '_csl'], - py_limited_api = '3.8', # 3.8 - ) - analysis_test( - name = name, - target = name + "_pyext", - impl = _test_limited_newer_dep_impl, - expect_failure = True, - ) - -def _test_limited_newer_dep_impl(env, target): - env.expect.that_target(target).failures().contains_predicate( - matching.str_matches("*Incompatible Python Limited API targets detected*"), - ) - -def _test_limited_dep_missing_define(name): - util.helper_target( - cc_library, - name = name + '_csl', - deps = [ - "@rules_python//python/cc:current_py_cc_headers", - ], - ) - py_extension( - name = name + '_pyext', - static_deps = [':' + name + '_csl'], - py_limited_api = '3.8', - ) - analysis_test( - name = name, - target = name + "_pyext", - impl = _test_limited_dep_missing_define_impl, - expect_failure = True, - ) - -def _test_limited_dep_missing_define_impl(env, target): - env.expect.that_target(target).failures().contains_predicate( - matching.str_matches("*Unsafe Python C API usage in dependency*"), - ) - -def _test_limited_dep_unspecified_define(name): - util.helper_target( - cc_library, - name = name + '_csl', - defines = ["Py_LIMITED_API"], - deps = [ - "@rules_python//python/cc:current_py_cc_headers", - ], - ) - py_extension( - name = name + '_pyext', - static_deps = [':' + name + '_csl'], - py_limited_api = '3.8', - ) - analysis_test( - name = name, - target = name + "_pyext", - impl = _test_limited_dep_unspecified_define_impl, - expect_failure = True, - ) - -def _test_limited_dep_unspecified_define_impl(env, target): - env.expect.that_target(target).failures().contains_predicate( - matching.str_matches("*Unsafe Python Limited API definition in dependency*"), - ) - def _test_no_limited_api(name): util.helper_target( cc_library, @@ -202,44 +125,14 @@ def _test_limited_api_dep_has_no_python(name): impl = _test_limited_pass_impl, ) -def _test_invalid_version_format(name): - util.helper_target( - cc_library, - name = name + '_csl', - defines = ["Py_LIMITED_API=0x03080000"], - deps = [ - "@rules_python//python/cc:current_py_cc_headers", - ], - ) - py_extension( - name = name + '_pyext', - static_deps = [':' + name + '_csl'], - py_limited_api = '3.8.1', - ) - analysis_test( - name = name, - target = name + "_pyext", - impl = _test_invalid_version_format_impl, - expect_failure = True, - ) - -def _test_invalid_version_format_impl(env, target): - env.expect.that_target(target).failures().contains_predicate( - matching.str_matches("*Invalid py_limited_api version*"), - ) - def py_limited_api_test_suite(name): test_suite( name = name, tests = [ _test_limited_same_version, _test_limited_older_dep, - _test_limited_newer_dep, - _test_limited_dep_missing_define, - _test_limited_dep_unspecified_define, _test_no_limited_api, _test_no_limited_api_dep_has_limited, _test_limited_api_dep_has_no_python, - _test_invalid_version_format, ], ) From 5bbe09ff6e8f1fc957c0cabb01d6afb43b92fca3 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 30 Jun 2026 15:58:50 +0000 Subject: [PATCH 24/35] Fix header filename. --- tests/cc/py_extension/static_dep.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/cc/py_extension/static_dep.c b/tests/cc/py_extension/static_dep.c index 4d910c1178..fa95e4dacc 100644 --- a/tests/cc/py_extension/static_dep.c +++ b/tests/cc/py_extension/static_dep.c @@ -1,4 +1,4 @@ -#include "my_lib.h" +#include "static_dep.h" int my_lib_func() { return 42; From 475ed3d263fa8d3fd6b8cd0352d0c5bbad24ba6a Mon Sep 17 00:00:00 2001 From: rsartor-cmd Date: Tue, 30 Jun 2026 11:29:18 -0500 Subject: [PATCH 25/35] Update python/private/cc/py_extension_rule.bzl Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- python/private/cc/py_extension_rule.bzl | 4 ---- 1 file changed, 4 deletions(-) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 54a8404d67..0f73574132 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -164,10 +164,6 @@ The minimum Python version to target for the Limited API (e.g., '3.8'). If set to a version string (e.g., '3.8') instead of '' (empty string): - Configures the output filename to use the simple '.abi3' suffix (e.g., 'ext.abi3.so'). - - Strictly validates that all linked C++ dependencies (static_deps, - dynamic_deps, etc.) are binary-compatible with this target version, - failing the build if a dependency is missing the 'Py_LIMITED_API' - define or targets a newer version. Note: Since the py_extension rule only links pre-compiled libraries, you must manually add the preprocessor macro to the cc_library targets From 903c96a60aa7e97b0365ed76fc818b72e596893f Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 30 Jun 2026 16:05:10 +0000 Subject: [PATCH 26/35] ruff --- tests/cc/py_extension/py_extension_test.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/cc/py_extension/py_extension_test.py b/tests/cc/py_extension/py_extension_test.py index 3d71e453fd..52d5502ea7 100644 --- a/tests/cc/py_extension/py_extension_test.py +++ b/tests/cc/py_extension/py_extension_test.py @@ -11,7 +11,10 @@ class PyExtensionTest(unittest.TestCase): def test_inspect_elf(self): r = runfiles.Create() - ext_path = r.Rlocation("rules_python/tests/cc/py_extension/ext_shared.cpython-311-x86_64-linux-gnu.so") + ext_path = r.Rlocation( + "rules_python/tests/cc/py_extension/" + + "ext_shared.cpython-311-x86_64-linux-gnu.so" + ) self.assertTrue( os.path.exists(ext_path), f"Could not find ext_shared.so at {ext_path}" ) From 9570a985a02a2515355bbeff3d3695b6314b4be2 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 30 Jun 2026 16:28:44 +0000 Subject: [PATCH 27/35] Derive the values for the _constraints attr programmatically, instead of hard-coding. --- python/private/cc/py_extension_rule.bzl | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 0f73574132..23fd0ce627 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -181,18 +181,11 @@ extension. default = "" ), "_constraints": lambda: attrb.LabelList( - default = [ - "@platforms//os:linux", - "@platforms//os:macos", - "@platforms//os:windows", - "@platforms//cpu:x86_64", - "@platforms//cpu:aarch64", - "@platforms//cpu:armv7", - "@platforms//cpu:i386", - "@platforms//cpu:ppc", - "@platforms//cpu:riscv64", - "@platforms//cpu:s390x", - ], + default = sorted({ + c: None + for info in PLATFORMS.values() + for c in info.compatible_with + }.keys()), ), } From 7179c8a991b871f9b3415a08d79fe0fa0c34f252 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 30 Jun 2026 16:45:47 +0000 Subject: [PATCH 28/35] Take glibc-vs-musl into account, so we get the right platform and name. --- python/private/cc/py_extension_macro.bzl | 6 ++++++ python/private/cc/py_extension_rule.bzl | 15 +++++++++++++++ tests/cc/py_extension/py_extension_tests.bzl | 19 +++++++++++++++++++ 3 files changed, 40 insertions(+) diff --git a/python/private/cc/py_extension_macro.bzl b/python/private/cc/py_extension_macro.bzl index 8c4cc36f58..7b052de0a8 100644 --- a/python/private/cc/py_extension_macro.bzl +++ b/python/private/cc/py_extension_macro.bzl @@ -20,6 +20,12 @@ def py_extension(**kwargs): if use_csl: _py_extension_csl(**kwargs) else: + if "libc" not in kwargs: + kwargs["libc"] = select({ + "@rules_python//python/config_settings:_is_py_linux_libc_musl": "musl", + "@rules_python//python/config_settings:_is_py_linux_libc_glibc": "glibc", + "//conditions:default": "glibc", + }) _py_extension(**kwargs) def _py_extension_csl(*, name, module_name = None, **kwargs): diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 23fd0ce627..7592f28088 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -155,6 +155,7 @@ PY_EXTENSION_ATTRS = COMMON_ATTRS | { default = [], ), "copts": lambda: attrb.StringList(), + "libc": lambda: attrb.String(default = "glibc"), "linkopts": lambda: attrb.StringList(), "module_name": lambda: attrb.String(), "py_limited_api": lambda: attrb.String( @@ -253,6 +254,13 @@ def _get_platform_from_constraints(ctx): if platform_common.ConstraintValueInfo in c: constraints_map[c.label] = c[platform_common.ConstraintValueInfo] + # Resolve the target's libc to its config_setting label string + target_libc_setting = None + if ctx.attr.libc == "musl": + target_libc_setting = str(Label("//python/config_settings:_is_py_linux_libc_musl")) + elif ctx.attr.libc == "glibc": + target_libc_setting = str(Label("//python/config_settings:_is_py_linux_libc_glibc")) + # Find the matching platform in PLATFORMS for platform, info in PLATFORMS.items(): # Check if all compatible_with constraints are satisfied @@ -267,6 +275,13 @@ def _get_platform_from_constraints(ctx): else: match = False break + + if match: + # Additional check for Linux libc consistency using target_settings + if info.os_name == "linux" and target_libc_setting: + if target_libc_setting not in info.target_settings: + match = False + if match: return _derive_pep3149_tag(platform, info) diff --git a/tests/cc/py_extension/py_extension_tests.bzl b/tests/cc/py_extension/py_extension_tests.bzl index 514bb22ac4..8235e8cd0f 100644 --- a/tests/cc/py_extension/py_extension_tests.bzl +++ b/tests/cc/py_extension/py_extension_tests.bzl @@ -70,6 +70,25 @@ def _test_dynamic_deps(name): _tests.append(_test_dynamic_deps) +def _test_musl_platform_impl(env, target): + env.expect.that_target(target).has_provider(PyInfo) + py_info = target[PyInfo] + env.expect.that_depset_of_files(py_info.transitive_sources).contains_predicate( + matching.file_basename_equals("ext_static.cpython-311-x86_64-linux-musl.so"), + ) + +def _test_musl_platform(name): + analysis_test( + name = name, + impl = _test_musl_platform_impl, + target = "//tests/cc/py_extension:ext_static", + config_settings = { + str(Label("//python/config_settings:py_linux_libc")): "musl", + }, + ) + +_tests.append(_test_musl_platform) + def py_extension_analysis_test_suite(name): test_suite( name = name, From b353be69bbe655b18fee9945d36d8831626554f2 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 30 Jun 2026 16:55:17 +0000 Subject: [PATCH 29/35] Clean up as per the linter. --- python/private/cc/py_extension_rule.bzl | 9 ++-- tests/cc/py_extension/BUILD.bazel | 4 +- .../cc/py_extension/py_limited_api_tests.bzl | 42 +++++++++---------- 3 files changed, 27 insertions(+), 28 deletions(-) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 7592f28088..f7ec72fe6f 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -58,8 +58,8 @@ def _py_extension_impl(ctx): use_py_limited_api = bool(ctx.attr.py_limited_api) if use_py_limited_api: output_filename = "{module_name}.abi3.{ext}".format( - module_name=module_name, - ext=ext, + module_name = module_name, + ext = ext, ) else: py_toolchain = ctx.toolchains[PY_CC_TOOLCHAIN_TYPE] @@ -179,7 +179,7 @@ that compile your C/C++ sources, for example: Set to '' (the default) or None to build a standard, version-specific extension. """, - default = "" + default = "", ), "_constraints": lambda: attrb.LabelList( default = sorted({ @@ -207,7 +207,6 @@ def create_py_extension_rule_builder(**kwargs): py_extension = create_py_extension_rule_builder().build() - def _get_extension(cc_toolchain): """ Derives the appropriate file extension from the C++ toolchain. @@ -307,5 +306,5 @@ ERROR: Unsupported target platform for {self}. in rules_python's central registry (python/versions.bzl). Please ensure your target platform is configured correctly.""".format( self = ctx.label, - ) + ), ) diff --git a/tests/cc/py_extension/BUILD.bazel b/tests/cc/py_extension/BUILD.bazel index e139798b24..6a595cf57b 100644 --- a/tests/cc/py_extension/BUILD.bazel +++ b/tests/cc/py_extension/BUILD.bazel @@ -96,18 +96,18 @@ cc_library( py_extension( name = "ext_limited", + py_limited_api = "3.8", static_deps = [":ext_limited_impl"], - py_limited_api = '3.8' ) cc_library( name = "ext_limited_impl", srcs = ["ext_limited.c"], - defines = ["Py_LIMITED_API=0x3080000"], copts = [ "-fPIC", "-fvisibility=hidden", ], + defines = ["Py_LIMITED_API=0x3080000"], deps = [ "@rules_python//python/cc:current_py_cc_headers", ], diff --git a/tests/cc/py_extension/py_limited_api_tests.bzl b/tests/cc/py_extension/py_limited_api_tests.bzl index 5e344dbc31..c1a42eef64 100644 --- a/tests/cc/py_extension/py_limited_api_tests.bzl +++ b/tests/cc/py_extension/py_limited_api_tests.bzl @@ -14,30 +14,30 @@ """Tests for the py_limited_api attribute for py_extension.""" +load("@rules_cc//cc:cc_library.bzl", "cc_library") load("@rules_testing//lib:analysis_test.bzl", "analysis_test", "test_suite") load("@rules_testing//lib:truth.bzl", "matching") load("@rules_testing//lib:util.bzl", "util") load("//python/cc:py_extension.bzl", "py_extension") -load("@rules_cc//cc:cc_library.bzl", "cc_library") def _test_limited_pass_impl(env, target): env.expect.that_target(target).default_outputs().contains( - "tests/cc/py_extension/{}.abi3.so".format(target.label.name) + "tests/cc/py_extension/{}.abi3.so".format(target.label.name), ) def _test_limited_same_version(name): util.helper_target( cc_library, - name = name + '_csl', + name = name + "_csl", defines = ["Py_LIMITED_API=0x03080000"], deps = [ "@rules_python//python/cc:current_py_cc_headers", ], ) py_extension( - name = name + '_pyext', - static_deps = [':' + name + '_csl'], - py_limited_api = '3.8', + name = name + "_pyext", + static_deps = [":" + name + "_csl"], + py_limited_api = "3.8", ) analysis_test( name = name, @@ -48,16 +48,16 @@ def _test_limited_same_version(name): def _test_limited_older_dep(name): util.helper_target( cc_library, - name = name + '_csl', - defines = ["Py_LIMITED_API=0x03080000"], # 3.8 + name = name + "_csl", + defines = ["Py_LIMITED_API=0x03080000"], # 3.8 deps = [ "@rules_python//python/cc:current_py_cc_headers", ], ) py_extension( - name = name + '_pyext', - static_deps = [':' + name + '_csl'], - py_limited_api = '3.9', # 3.9 + name = name + "_pyext", + static_deps = [":" + name + "_csl"], + py_limited_api = "3.9", # 3.9 ) analysis_test( name = name, @@ -68,14 +68,14 @@ def _test_limited_older_dep(name): def _test_no_limited_api(name): util.helper_target( cc_library, - name = name + '_csl', + name = name + "_csl", deps = [ "@rules_python//python/cc:current_py_cc_headers", ], ) py_extension( - name = name + '_pyext', - static_deps = [':' + name + '_csl'], + name = name + "_pyext", + static_deps = [":" + name + "_csl"], ) analysis_test( name = name, @@ -90,15 +90,15 @@ def _test_no_limited_api_impl(env, target): def _test_no_limited_api_dep_has_limited(name): util.helper_target( cc_library, - name = name + '_csl', + name = name + "_csl", defines = ["Py_LIMITED_API=0x03080000"], deps = [ "@rules_python//python/cc:current_py_cc_headers", ], ) py_extension( - name = name + '_pyext', - static_deps = [':' + name + '_csl'], + name = name + "_pyext", + static_deps = [":" + name + "_csl"], ) analysis_test( name = name, @@ -112,12 +112,12 @@ def _test_no_limited_api_dep_has_limited_impl(env, target): def _test_limited_api_dep_has_no_python(name): util.helper_target( cc_library, - name = name + '_csl', + name = name + "_csl", ) py_extension( - name = name + '_pyext', - static_deps = [':' + name + '_csl'], - py_limited_api = '3.8', + name = name + "_pyext", + static_deps = [":" + name + "_csl"], + py_limited_api = "3.8", ) analysis_test( name = name, From ee6395dcd5044ab69a4a00664c6d1b6ac23f62e8 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 30 Jun 2026 17:00:04 +0000 Subject: [PATCH 30/35] Remove print()s per buildifier. --- python/private/cc/py_extension_rule.bzl | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index f7ec72fe6f..a7353cf10a 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -81,12 +81,6 @@ def _py_extension_impl(ctx): # Add target-level linkopts last so users can override. user_link_flags.extend(ctx.attr.linkopts) - print(( - "===LINK:\n" + - " user_link_flags={user_link_flags}" - ).format( - user_link_flags = user_link_flags, - )) # todo: add linker script to hide symbols by default # py_internal allows using some private apis, which may or may not be needed. @@ -104,12 +98,6 @@ def _py_extension_impl(ctx): # todo: maybe variables_extension # todo: maybe additional_outputs ) - print(( - "===LINK OUTPUT:\n" + - " {}" - ).format( - cc_linking_outputs, - )) # Propagate CcInfo from dynamic and external deps, but not static ones. dynamic_cc_info = CcInfo(linking_context = dynamic_linking_context) From 8d807837cc9e0f279bb82fa335d6490ebb6d9e17 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 30 Jun 2026 17:01:21 +0000 Subject: [PATCH 31/35] Remove unused variable (buildifier). --- python/private/cc/py_extension_rule.bzl | 1 - 1 file changed, 1 deletion(-) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index a7353cf10a..d716cd981f 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -124,7 +124,6 @@ def _py_extension_impl(ctx): propagated_cc_info, ] -_MaybeBuiltinPyInfo = [[BuiltinPyInfo]] if BuiltinPyInfo != None else [] PY_EXTENSION_ATTRS = COMMON_ATTRS | { "dynamic_deps": lambda: attrb.LabelList( From 451ea480022dc26b2ef02721150ffb09c336dff9 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 30 Jun 2026 17:02:27 +0000 Subject: [PATCH 32/35] Sort keys [buildifier] --- python/private/cc/py_extension_macro.bzl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/python/private/cc/py_extension_macro.bzl b/python/private/cc/py_extension_macro.bzl index 7b052de0a8..f4f2a27c35 100644 --- a/python/private/cc/py_extension_macro.bzl +++ b/python/private/cc/py_extension_macro.bzl @@ -22,9 +22,9 @@ def py_extension(**kwargs): else: if "libc" not in kwargs: kwargs["libc"] = select({ - "@rules_python//python/config_settings:_is_py_linux_libc_musl": "musl", - "@rules_python//python/config_settings:_is_py_linux_libc_glibc": "glibc", "//conditions:default": "glibc", + "@rules_python//python/config_settings:_is_py_linux_libc_glibc": "glibc", + "@rules_python//python/config_settings:_is_py_linux_libc_musl": "musl", }) _py_extension(**kwargs) From a40ae2b6fff01ec0757d34fb137c18ffc16eace6 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 30 Jun 2026 17:03:10 +0000 Subject: [PATCH 33/35] Remove print()s per buildifier. --- tests/cc/py_extension/py_extension_tests.bzl | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/cc/py_extension/py_extension_tests.bzl b/tests/cc/py_extension/py_extension_tests.bzl index 8235e8cd0f..b84ddb9ecb 100644 --- a/tests/cc/py_extension/py_extension_tests.bzl +++ b/tests/cc/py_extension/py_extension_tests.bzl @@ -58,7 +58,6 @@ def _test_dynamic_deps_impl(env, target): ) # CcInfo from dynamic_deps should be propagated. - print(cc_info.linking_context.linker_inputs.to_list()) env.expect.that_collection(cc_info.linking_context.linker_inputs.to_list()).has_size(1) def _test_dynamic_deps(name): From 90ad765992f91f336c11dc3d7f289db0ed6e67f1 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 30 Jun 2026 17:04:05 +0000 Subject: [PATCH 34/35] Remove unused load(). --- tests/cc/py_extension/py_limited_api_tests.bzl | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/cc/py_extension/py_limited_api_tests.bzl b/tests/cc/py_extension/py_limited_api_tests.bzl index c1a42eef64..46b36ec519 100644 --- a/tests/cc/py_extension/py_limited_api_tests.bzl +++ b/tests/cc/py_extension/py_limited_api_tests.bzl @@ -16,7 +16,6 @@ load("@rules_cc//cc:cc_library.bzl", "cc_library") load("@rules_testing//lib:analysis_test.bzl", "analysis_test", "test_suite") -load("@rules_testing//lib:truth.bzl", "matching") load("@rules_testing//lib:util.bzl", "util") load("//python/cc:py_extension.bzl", "py_extension") From 708dac4b19454f9b8274a3e00d6c8bc2add5cedd Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 30 Jun 2026 17:09:58 +0000 Subject: [PATCH 35/35] Call out unused parameters [buildifier] --- tests/cc/py_extension/py_limited_api_tests.bzl | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/cc/py_extension/py_limited_api_tests.bzl b/tests/cc/py_extension/py_limited_api_tests.bzl index 46b36ec519..bf5b8e457e 100644 --- a/tests/cc/py_extension/py_limited_api_tests.bzl +++ b/tests/cc/py_extension/py_limited_api_tests.bzl @@ -84,7 +84,8 @@ def _test_no_limited_api(name): def _test_no_limited_api_impl(env, target): # Should pass, nothing to assert on filename since it is platform-specific - pass + _ = env # @unused + _ = target # @unused def _test_no_limited_api_dep_has_limited(name): util.helper_target( @@ -106,7 +107,8 @@ def _test_no_limited_api_dep_has_limited(name): ) def _test_no_limited_api_dep_has_limited_impl(env, target): - pass + _ = env # @unused + _ = target # @unused def _test_limited_api_dep_has_no_python(name): util.helper_target(