From 0fefc888937682312f04f7d3219362d98e1fdfdf Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Mon, 22 Sep 2025 19:54:42 -0700 Subject: [PATCH 01/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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/58] 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( From 830723ff45f3a0664b6e78f859b5a8967f81da9f Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Mon, 6 Jul 2026 18:21:04 +0000 Subject: [PATCH 36/58] Add some basic tests of different dependency situations, so we can confirm py_extension behaves similarly to cc_shared_library. --- tests/cc/py_extension/BUILD.bazel | 5 + .../py_extension/dependency_graph_tests.bzl | 227 ++++++++++++++++++ 2 files changed, 232 insertions(+) create mode 100644 tests/cc/py_extension/dependency_graph_tests.bzl diff --git a/tests/cc/py_extension/BUILD.bazel b/tests/cc/py_extension/BUILD.bazel index 6a595cf57b..6b91562875 100644 --- a/tests/cc/py_extension/BUILD.bazel +++ b/tests/cc/py_extension/BUILD.bazel @@ -4,6 +4,7 @@ load("//python:py_test.bzl", "py_test") # buildifier: disable=bzl-visibility load("//python/cc:py_extension.bzl", "py_extension") +load(":dependency_graph_tests.bzl", "dependency_graph_test_suite") load(":py_extension_tests.bzl", "py_extension_analysis_test_suite") load(":py_limited_api_tests.bzl", "py_limited_api_test_suite") @@ -130,3 +131,7 @@ py_extension_analysis_test_suite( py_limited_api_test_suite( name = "py_limited_api_tests", ) + +dependency_graph_test_suite( + name = "dependency_graph_tests", +) diff --git a/tests/cc/py_extension/dependency_graph_tests.bzl b/tests/cc/py_extension/dependency_graph_tests.bzl new file mode 100644 index 0000000000..eab10c776e --- /dev/null +++ b/tests/cc/py_extension/dependency_graph_tests.bzl @@ -0,0 +1,227 @@ +# 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. + +"""Parity tests comparing cc_shared_library and py_extension behavior.""" + +load("@rules_cc//cc:cc_library.bzl", "cc_library") +load("@rules_cc//cc:cc_shared_library.bzl", "cc_shared_library") +load("@rules_testing//lib:analysis_test.bzl", "analysis_test", "test_suite") +load("@rules_testing//lib:util.bzl", "util") +load("//python/cc:py_extension.bzl", "py_extension") +# buildifier: disable=bzl-visibility +load("@rules_cc//cc/common:cc_info.bzl", "CcInfo") + +# Test 1: CSL A -> CSL B -> CSL C (Dynamic deps) +def _test_csl_dynamic_deps(name): + util.helper_target( + cc_library, + name = name + "_libC", + srcs = ["test_lib_c.c"], + hdrs = ["test_symbols.h"], + copts = ["-fPIC"], + ) + util.helper_target( + cc_shared_library, + name = name + "_cslC", + deps = [":" + name + "_libC"], + ) + util.helper_target( + cc_library, + name = name + "_libB", + srcs = ["test_lib_b.c"], + hdrs = ["test_symbols.h"], + copts = ["-fPIC"], + deps = [":" + name + "_libC"], + ) + util.helper_target( + cc_shared_library, + name = name + "_cslB", + deps = [":" + name + "_libB"], + dynamic_deps = [":" + name + "_cslC"], + ) + util.helper_target( + cc_library, + name = name + "_libA", + srcs = ["test_lib_a.c"], + hdrs = ["test_symbols.h"], + copts = ["-fPIC"], + deps = [":" + name + "_libB", ":" + name + "_libC"], + ) + util.helper_target( + cc_shared_library, + name = name + "_cslA", + deps = [":" + name + "_libA"], + dynamic_deps = [":" + name + "_cslB", ":" + name + "_cslC"], + ) + analysis_test( + name = name, + target = name + "_cslA", + impl = _csl_dynamic_deps_test_impl, + ) + +def _csl_dynamic_deps_test_impl(env, target): + env.expect.that_target(target).has_provider(CcSharedLibraryInfo) + +# Test 2: py_extension A -> CSL B -> CSL C (Dynamic deps) +def _test_pyext_dynamic_deps(name): + util.helper_target( + cc_library, + name = name + "_libC", + srcs = ["test_lib_c.c"], + hdrs = ["test_symbols.h"], + copts = ["-fPIC"], + ) + util.helper_target( + cc_shared_library, + name = name + "_cslC", + deps = [":" + name + "_libC"], + ) + util.helper_target( + cc_library, + name = name + "_libB", + srcs = ["test_lib_b.c"], + hdrs = ["test_symbols.h"], + copts = ["-fPIC"], + deps = [":" + name + "_libC"], + ) + util.helper_target( + cc_shared_library, + name = name + "_cslB", + deps = [":" + name + "_libB"], + dynamic_deps = [":" + name + "_cslC"], + ) + util.helper_target( + cc_library, + name = name + "_libA", + srcs = ["test_lib_a.c"], + hdrs = ["test_symbols.h"], + copts = ["-fPIC"], + deps = [":" + name + "_libB", ":" + name + "_libC"], + ) + py_extension( + name = name + "_pyextA", + static_deps = [":" + name + "_libA"], + dynamic_deps = [":" + name + "_cslB", ":" + name + "_cslC"], + ) + analysis_test( + name = name, + target = name + "_pyextA", + impl = _pyext_dynamic_deps_test_impl, + ) + +def _pyext_dynamic_deps_test_impl(env, target): + env.expect.that_target(target).has_provider(CcInfo) + cc_info = target[CcInfo] + # Should propagate CcInfo from dynamic_deps (cslB and cslC) + env.expect.that_collection(cc_info.linking_context.linker_inputs.to_list()).has_size(2) + +# Test 3: CSL A -> CSL B, CL C (Static sharing) +def _test_csl_static_sharing(name): + util.helper_target( + cc_library, + name = name + "_libC", + srcs = ["test_lib_c.c"], + hdrs = ["test_symbols.h"], + copts = ["-fPIC"], + ) + util.helper_target( + cc_library, + name = name + "_libB", + srcs = ["test_lib_b.c"], + hdrs = ["test_symbols.h"], + copts = ["-fPIC"], + deps = [":" + name + "_libC"], + ) + util.helper_target( + cc_shared_library, + name = name + "_cslB", + deps = [":" + name + "_libB", ":" + name + "_libC"], + ) + util.helper_target( + cc_library, + name = name + "_libA", + srcs = ["test_lib_a.c"], + hdrs = ["test_symbols.h"], + copts = ["-fPIC"], + deps = [":" + name + "_libB", ":" + name + "_libC"], + ) + util.helper_target( + cc_shared_library, + name = name + "_cslA", + deps = [":" + name + "_libA"], + dynamic_deps = [":" + name + "_cslB"], + ) + analysis_test( + name = name, + target = name + "_cslA", + impl = _csl_static_sharing_test_impl, + ) + +def _csl_static_sharing_test_impl(env, target): + env.expect.that_target(target).has_provider(CcSharedLibraryInfo) + +# Test 4: Same as 3, but A is py_extension +def _test_pyext_static_sharing(name): + util.helper_target( + cc_library, + name = name + "_libC", + srcs = ["test_lib_c.c"], + hdrs = ["test_symbols.h"], + copts = ["-fPIC"], + ) + util.helper_target( + cc_library, + name = name + "_libB", + srcs = ["test_lib_b.c"], + hdrs = ["test_symbols.h"], + copts = ["-fPIC"], + deps = [":" + name + "_libC"], + ) + util.helper_target( + cc_shared_library, + name = name + "_cslB", + deps = [":" + name + "_libB", ":" + name + "_libC"], + ) + util.helper_target( + cc_library, + name = name + "_libA", + srcs = ["test_lib_a.c"], + hdrs = ["test_symbols.h"], + copts = ["-fPIC"], + deps = [":" + name + "_libB", ":" + name + "_libC"], + ) + py_extension( + name = name + "_pyextA", + static_deps = [":" + name + "_libA", ":" + name + "_libC"], + dynamic_deps = [":" + name + "_cslB"], + ) + analysis_test( + name = name, + target = name + "_pyextA", + impl = _pyext_static_sharing_test_impl, + ) + +def _pyext_static_sharing_test_impl(env, target): + env.expect.that_target(target).has_provider(CcInfo) + +def dependency_graph_test_suite(name): + test_suite( + name = name, + tests = [ + _test_csl_dynamic_deps, + _test_pyext_dynamic_deps, + _test_csl_static_sharing, + _test_pyext_static_sharing, + ], + ) From 37f5eaa6fe5a25ecdeefe26b39fd131a4af23c6c Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Mon, 6 Jul 2026 22:19:40 +0000 Subject: [PATCH 37/58] Replace the custom compiling and linking logic with internal cc_library and cc_shared_library targets. --- python/private/cc/py_extension_macro.bzl | 102 +++++++++--- python/private/cc/py_extension_rule.bzl | 145 +++--------------- tests/cc/py_extension/BUILD.bazel | 29 ++-- .../py_extension/dependency_graph_tests.bzl | 14 +- tests/cc/py_extension/py_extension_tests.bzl | 16 +- .../cc/py_extension/py_limited_api_tests.bzl | 10 +- 6 files changed, 127 insertions(+), 189 deletions(-) diff --git a/python/private/cc/py_extension_macro.bzl b/python/private/cc/py_extension_macro.bzl index f4f2a27c35..c28b6ee1e8 100644 --- a/python/private/cc/py_extension_macro.bzl +++ b/python/private/cc/py_extension_macro.bzl @@ -1,39 +1,91 @@ -"""Wrapper macro for the py_extension rule.""" +"""Macro for creating Python extensions.""" +load("@rules_cc//cc:cc_library.bzl", "cc_library") 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", - ##_py_extension_csl_rule = "py_extension_csl", -) +load(":py_extension_rule.bzl", "py_extension_wrapper") -def py_extension(**kwargs): - """A macro that calls the py_extension rule and adds a tag. +def py_extension( + name, + srcs = None, + hdrs = None, + copts = None, + defines = None, + deps = None, + dynamic_deps = None, + exports_filter = None, + user_link_flags = None, + visibility = None, + **kwargs): + """Creates a Python extension module. Args: - **kwargs: Additional arguments to pass to the rule. + name: Target name. + srcs: Optional C/C++ source files to compile directly for this extension. + hdrs: Optional header files for the srcs. + copts: Optional compiler flags for srcs. + defines: Optional preprocessor defines for srcs. + deps: cc_library targets to statically link into the extension. + dynamic_deps: cc_shared_library targets to dynamically link. + exports_filter: Filter for exported symbols passed to cc_shared_library. + user_link_flags: Additional link flags passed to cc_shared_library. + visibility: Target visibility. + **kwargs: Additional arguments passed to the underlying wrapper rule. """ add_tag(kwargs, "@rules_python//python/cc:py_extension") - use_csl = kwargs.pop("use_csl", False) - if use_csl: - _py_extension_csl(**kwargs) - else: - if "libc" not in kwargs: - kwargs["libc"] = select({ - "//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) - -def _py_extension_csl(*, name, module_name = None, **kwargs): - if not module_name: - module_name = name + csl_deps = [] + + # 1. Handle user-supplied static deps + if deps: + csl_deps.extend(deps) + + # 2. If srcs or hdrs are specified, create an implicit cc_library for them + if srcs or hdrs: + impl_lib_name = "_" + name + "_impl" + cc_library( + name = impl_lib_name, + srcs = srcs, + hdrs = hdrs, + copts = (copts or []) + ["-fPIC"], + defines = defines, + deps = ["@rules_python//python/cc:current_py_cc_headers"], + visibility = ["//visibility:private"], + ) + csl_deps.append(":" + impl_lib_name) + + # 3. If no static deps or sources were specified, use empty target for CSL requirement + if not csl_deps: + csl_deps.append("//python/private/cc:empty") + + # 4. Create the underlying cc_shared_library + csl_name = "_" + name + "_csl" + csl_kwargs = {} + if exports_filter: + csl_kwargs["exports_filter"] = exports_filter + if user_link_flags: + csl_kwargs["user_link_flags"] = user_link_flags cc_shared_library( + name = csl_name, + deps = csl_deps, + dynamic_deps = dynamic_deps, + visibility = ["//visibility:private"], + **csl_kwargs + ) + + # 5. Select default libc constraint if not provided + if "libc" not in kwargs: + kwargs["libc"] = select({ + "//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", + }) + + # 6. Wrap with py_extension_wrapper for PEP 3149 naming & PyInfo + py_extension_wrapper( name = name, - shared_lib_name = module_name + ".so", + src = ":" + csl_name, + visibility = visibility, **kwargs ) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index d716cd981f..395e661e4d 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -1,59 +1,21 @@ -"""Implementation of the py_extension rule.""" +"""Implementation of the _py_extension_wrapper rule.""" -load("@rules_cc//cc/common:cc_common.bzl", "cc_common") -load("@rules_cc//cc/common:cc_info.bzl", "CcInfo") +load("@rules_cc//cc/common:cc_shared_library_info.bzl", "CcSharedLibraryInfo") 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") load("//python/private:rule_builders.bzl", "ruleb") load("//python/private:toolchain_types.bzl", "PY_CC_TOOLCHAIN_TYPE") -def _py_extension_impl(ctx): +def _py_extension_wrapper_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 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, - 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[CcSharedLibraryInfo] for dep in ctx.attr.dynamic_deps] - external_deps_infos = [dep[CcInfo] for dep in ctx.attr.external_deps] - - # Static deps are linked directly into the .so - static_cc_info = cc_common.merge_cc_infos( - cc_infos = static_deps_infos, - ) - - # Dynamic deps are linked as shared libraries - 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( - 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") + cc_toolchain = ctx.toolchains["@bazel_tools//tools/cpp:toolchain_type"].cc ext = _get_extension(cc_toolchain) use_py_limited_api = bool(ctx.attr.py_limited_api) if use_py_limited_api: @@ -71,46 +33,18 @@ def _py_extension_impl(ctx): platform = platform_tag, ext = ext, ) - 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) - - # 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, - linking_contexts = linking_contexts, - user_link_flags = user_link_flags, - # todo: add additional_inputs - name = ctx.label.name, - output_type = "dynamic_library", - main_output = py_dso, - # todo: maybe variables_extension - # todo: maybe additional_outputs - ) + py_dso = ctx.actions.declare_file(output_filename) - # 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_cc_info] + external_deps_infos, + # Symlink the cc_shared_library output to the PEP 3149 / abi3 filename + csl_target = ctx.attr.src + csl_file = csl_target[DefaultInfo].files.to_list()[0] + ctx.actions.symlink( + output = py_dso, + target_file = csl_file, ) - 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) + runfiles = ctx.runfiles(files = [py_dso]).merge(csl_target[DefaultInfo].default_runfiles) return [ DefaultInfo( @@ -121,51 +55,18 @@ def _py_extension_impl(ctx): transitive_sources = depset([py_dso]), imports = depset([import_path]), ), - propagated_cc_info, + csl_target[CcSharedLibraryInfo], ] - -PY_EXTENSION_ATTRS = COMMON_ATTRS | { - "dynamic_deps": lambda: attrb.LabelList( +PY_EXTENSION_WRAPPER_ATTRS = COMMON_ATTRS | { + "src": lambda: attrb.Label( + mandatory = True, providers = [CcSharedLibraryInfo], - doc = "cc_shared_library targets to be dynamically linked.", - default = [], + doc = "The cc_shared_library target to wrap.", ), - "external_deps": lambda: attrb.LabelList( - providers = [CcInfo], - doc = "cc_library targets with external linkage.", - default = [], - ), - "static_deps": lambda: attrb.LabelList( - providers = [CcInfo], - doc = "cc_library targets to be statically and privately linked.", - 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( - 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 '' (empty string): - - Configures the output filename to use the simple '.abi3' suffix - (e.g., 'ext.abi3.so'). - -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 '' (the default) or None to build a standard, version-specific -extension. -""", default = "", ), "_constraints": lambda: attrb.LabelList( @@ -177,12 +78,12 @@ extension. ), } -def create_py_extension_rule_builder(**kwargs): - """Create a rule builder for a py_extension.""" +def create_py_extension_wrapper_rule_builder(**kwargs): + """Create a rule builder for the wrapper.""" builder = ruleb.Rule( - implementation = _py_extension_impl, - attrs = PY_EXTENSION_ATTRS, - provides = [PyInfo, CcInfo], + implementation = _py_extension_wrapper_impl, + attrs = PY_EXTENSION_WRAPPER_ATTRS, + provides = [PyInfo, CcSharedLibraryInfo], toolchains = [ ruleb.ToolchainType(PY_CC_TOOLCHAIN_TYPE), ruleb.ToolchainType("@bazel_tools//tools/cpp:toolchain_type"), @@ -192,7 +93,7 @@ def create_py_extension_rule_builder(**kwargs): ) return builder -py_extension = create_py_extension_rule_builder().build() +py_extension_wrapper = create_py_extension_wrapper_rule_builder().build() def _get_extension(cc_toolchain): """ diff --git a/tests/cc/py_extension/BUILD.bazel b/tests/cc/py_extension/BUILD.bazel index 6b91562875..5ee8a15b6e 100644 --- a/tests/cc/py_extension/BUILD.bazel +++ b/tests/cc/py_extension/BUILD.bazel @@ -16,33 +16,25 @@ package( licenses(["notice"]) py_extension( - name = "ext_static", - ##srcs = ["ext_static.c"], - static_deps = [":static_dep"], + # An extension defined solely by source files, with no deps + name = "ext_source", + srcs = ["ext_source.c"], ) py_extension( - name = "ext_shared", - dynamic_deps = [ - ":add_one_shared", - ], - static_deps = [ - ":ext_shared_impl", - ], + # A python extension that gets its code from a statically-linked library + name = "ext_static", + deps = [":static_dep"], ) py_extension( - name = "ext_csl_shared", + # A python extension that dynamically links to another shared library + name = "ext_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", ], ) @@ -96,9 +88,10 @@ cc_library( ) py_extension( + # An extension that uses the Python limited API name = "ext_limited", py_limited_api = "3.8", - static_deps = [":ext_limited_impl"], + deps = [":ext_limited_impl"], ) cc_library( diff --git a/tests/cc/py_extension/dependency_graph_tests.bzl b/tests/cc/py_extension/dependency_graph_tests.bzl index eab10c776e..95b32560bc 100644 --- a/tests/cc/py_extension/dependency_graph_tests.bzl +++ b/tests/cc/py_extension/dependency_graph_tests.bzl @@ -16,11 +16,10 @@ load("@rules_cc//cc:cc_library.bzl", "cc_library") load("@rules_cc//cc:cc_shared_library.bzl", "cc_shared_library") +load("@rules_cc//cc/common:cc_shared_library_info.bzl", "CcSharedLibraryInfo") load("@rules_testing//lib:analysis_test.bzl", "analysis_test", "test_suite") load("@rules_testing//lib:util.bzl", "util") load("//python/cc:py_extension.bzl", "py_extension") -# buildifier: disable=bzl-visibility -load("@rules_cc//cc/common:cc_info.bzl", "CcInfo") # Test 1: CSL A -> CSL B -> CSL C (Dynamic deps) def _test_csl_dynamic_deps(name): @@ -111,7 +110,7 @@ def _test_pyext_dynamic_deps(name): ) py_extension( name = name + "_pyextA", - static_deps = [":" + name + "_libA"], + deps = [":" + name + "_libA"], dynamic_deps = [":" + name + "_cslB", ":" + name + "_cslC"], ) analysis_test( @@ -121,10 +120,7 @@ def _test_pyext_dynamic_deps(name): ) def _pyext_dynamic_deps_test_impl(env, target): - env.expect.that_target(target).has_provider(CcInfo) - cc_info = target[CcInfo] - # Should propagate CcInfo from dynamic_deps (cslB and cslC) - env.expect.that_collection(cc_info.linking_context.linker_inputs.to_list()).has_size(2) + env.expect.that_target(target).has_provider(CcSharedLibraryInfo) # Test 3: CSL A -> CSL B, CL C (Static sharing) def _test_csl_static_sharing(name): @@ -203,7 +199,7 @@ def _test_pyext_static_sharing(name): ) py_extension( name = name + "_pyextA", - static_deps = [":" + name + "_libA", ":" + name + "_libC"], + deps = [":" + name + "_libA"], dynamic_deps = [":" + name + "_cslB"], ) analysis_test( @@ -213,7 +209,7 @@ def _test_pyext_static_sharing(name): ) def _pyext_static_sharing_test_impl(env, target): - env.expect.that_target(target).has_provider(CcInfo) + env.expect.that_target(target).has_provider(CcSharedLibraryInfo) def dependency_graph_test_suite(name): test_suite( diff --git a/tests/cc/py_extension/py_extension_tests.bzl b/tests/cc/py_extension/py_extension_tests.bzl index b84ddb9ecb..3305d8d8d9 100644 --- a/tests/cc/py_extension/py_extension_tests.bzl +++ b/tests/cc/py_extension/py_extension_tests.bzl @@ -14,7 +14,7 @@ """Tests for py_extension.""" -load("@rules_cc//cc/common:cc_info.bzl", "CcInfo") +load("@rules_cc//cc/common:cc_shared_library_info.bzl", "CcSharedLibraryInfo") 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") @@ -24,8 +24,7 @@ _tests = [] def _test_static_deps_impl(env, target): env.expect.that_target(target).has_provider(PyInfo) py_info = target[PyInfo] - env.expect.that_target(target).has_provider(CcInfo) - cc_info = target[CcInfo] + env.expect.that_target(target).has_provider(CcSharedLibraryInfo) # The .so should be in PyInfo env.expect.that_collection(py_info.transitive_sources.to_list()).has_size(1) @@ -33,9 +32,6 @@ def _test_static_deps_impl(env, target): matching.file_basename_equals("ext_static.cpython-311-x86_64-linux-gnu.so"), ) - # CcInfo from static_deps should not be propagated. - env.expect.that_depset_of_files(cc_info.linking_context.linker_inputs).contains_exactly([]) - def _test_static_deps(name): analysis_test( name = name, @@ -48,8 +44,8 @@ _tests.append(_test_static_deps) def _test_dynamic_deps_impl(env, target): env.expect.that_target(target).has_provider(PyInfo) py_info = target[PyInfo] - env.expect.that_target(target).has_provider(CcInfo) - cc_info = target[CcInfo] + env.expect.that_target(target).has_provider(CcSharedLibraryInfo) + csl_info = target[CcSharedLibraryInfo] # The .so should be in PyInfo env.expect.that_collection(py_info.transitive_sources.to_list()).has_size(1) @@ -57,8 +53,8 @@ def _test_dynamic_deps_impl(env, target): 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()).has_size(1) + # CcSharedLibraryInfo provider should be present and non-empty + env.expect.that_target(target).has_provider(CcSharedLibraryInfo) 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 index bf5b8e457e..a59628c19c 100644 --- a/tests/cc/py_extension/py_limited_api_tests.bzl +++ b/tests/cc/py_extension/py_limited_api_tests.bzl @@ -35,7 +35,7 @@ def _test_limited_same_version(name): ) py_extension( name = name + "_pyext", - static_deps = [":" + name + "_csl"], + deps = [":" + name + "_csl"], py_limited_api = "3.8", ) analysis_test( @@ -55,7 +55,7 @@ def _test_limited_older_dep(name): ) py_extension( name = name + "_pyext", - static_deps = [":" + name + "_csl"], + deps = [":" + name + "_csl"], py_limited_api = "3.9", # 3.9 ) analysis_test( @@ -74,7 +74,7 @@ def _test_no_limited_api(name): ) py_extension( name = name + "_pyext", - static_deps = [":" + name + "_csl"], + deps = [":" + name + "_csl"], ) analysis_test( name = name, @@ -98,7 +98,7 @@ def _test_no_limited_api_dep_has_limited(name): ) py_extension( name = name + "_pyext", - static_deps = [":" + name + "_csl"], + deps = [":" + name + "_csl"], ) analysis_test( name = name, @@ -117,7 +117,7 @@ def _test_limited_api_dep_has_no_python(name): ) py_extension( name = name + "_pyext", - static_deps = [":" + name + "_csl"], + deps = [":" + name + "_csl"], py_limited_api = "3.8", ) analysis_test( From 4596ae291ccdbbaf4fd54f779093e7a8fa127456 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Mon, 6 Jul 2026 22:36:05 +0000 Subject: [PATCH 38/58] Add missing files. --- tests/cc/py_extension/ext_source.c | 33 ++++++++++++++++++++++++++++++ tests/cc/py_extension/test_lib_a.c | 6 ++++++ tests/cc/py_extension/test_lib_b.c | 5 +++++ tests/cc/py_extension/test_lib_c.c | 6 ++++++ 4 files changed, 50 insertions(+) create mode 100644 tests/cc/py_extension/ext_source.c create mode 100644 tests/cc/py_extension/test_lib_a.c create mode 100644 tests/cc/py_extension/test_lib_b.c create mode 100644 tests/cc/py_extension/test_lib_c.c diff --git a/tests/cc/py_extension/ext_source.c b/tests/cc/py_extension/ext_source.c new file mode 100644 index 0000000000..df0239df3a --- /dev/null +++ b/tests/cc/py_extension/ext_source.c @@ -0,0 +1,33 @@ + +#include + + +static PyObject* calc_one_plus_two(PyObject* self, PyObject* args) { + return PyLong_FromLong(1 + 2); +} + + +// Method definition object for this extension, these are the functions +// that will be available in the module. +static PyMethodDef ModuleMethods[] = { + {"calc_one_plus_two", calc_one_plus_two, 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_source_module = { + PyModuleDef_HEAD_INIT, + "ext_source", /* 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_source(void) { + return PyModule_Create(&ext_source_module); +} diff --git a/tests/cc/py_extension/test_lib_a.c b/tests/cc/py_extension/test_lib_a.c new file mode 100644 index 0000000000..19e2f489bf --- /dev/null +++ b/tests/cc/py_extension/test_lib_a.c @@ -0,0 +1,6 @@ +#include "test_symbols.h" + +void fnA() { + fnB(); + fnC(); +} diff --git a/tests/cc/py_extension/test_lib_b.c b/tests/cc/py_extension/test_lib_b.c new file mode 100644 index 0000000000..3621587c1c --- /dev/null +++ b/tests/cc/py_extension/test_lib_b.c @@ -0,0 +1,5 @@ +#include "test_symbols.h" + +void fnB() { + fnC(); +} diff --git a/tests/cc/py_extension/test_lib_c.c b/tests/cc/py_extension/test_lib_c.c new file mode 100644 index 0000000000..99941576dd --- /dev/null +++ b/tests/cc/py_extension/test_lib_c.c @@ -0,0 +1,6 @@ +#include "test_symbols.h" +#include + +void fnC() { + printf("fnC\n"); +} From 933f69fe43e14d79a20e66544398fa6c5e70409e Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Mon, 6 Jul 2026 22:38:31 +0000 Subject: [PATCH 39/58] Add a data argument to the macro. --- python/private/cc/py_extension_macro.bzl | 1 + python/private/cc/py_extension_rule.bzl | 8 +++++++- tests/cc/py_extension/BUILD.bazel | 6 ++++++ tests/cc/py_extension/py_extension_tests.bzl | 20 ++++++++++++++++++++ 4 files changed, 34 insertions(+), 1 deletion(-) diff --git a/python/private/cc/py_extension_macro.bzl b/python/private/cc/py_extension_macro.bzl index c28b6ee1e8..5bf5e6c8e3 100644 --- a/python/private/cc/py_extension_macro.bzl +++ b/python/private/cc/py_extension_macro.bzl @@ -30,6 +30,7 @@ def py_extension( exports_filter: Filter for exported symbols passed to cc_shared_library. user_link_flags: Additional link flags passed to cc_shared_library. visibility: Target visibility. + data: Optional list of files or targets needed by this extension at runtime. **kwargs: Additional arguments passed to the underlying wrapper rule. """ add_tag(kwargs, "@rules_python//python/cc:py_extension") diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 395e661e4d..a504a49b4d 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -4,6 +4,7 @@ load("@rules_cc//cc/common:cc_shared_library_info.bzl", "CcSharedLibraryInfo") 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:rule_builders.bzl", "ruleb") load("//python/private:toolchain_types.bzl", "PY_CC_TOOLCHAIN_TYPE") @@ -44,7 +45,12 @@ def _py_extension_wrapper_impl(ctx): target_file = csl_file, ) - runfiles = ctx.runfiles(files = [py_dso]).merge(csl_target[DefaultInfo].default_runfiles) + runfiles_builder = builders.RunfilesBuilder() + runfiles_builder.add(py_dso) + runfiles_builder.add(ctx.files.data) + runfiles_builder.add_targets(ctx.attr.data) + runfiles_builder.add(csl_target[DefaultInfo].default_runfiles) + runfiles = runfiles_builder.build(ctx) return [ DefaultInfo( diff --git a/tests/cc/py_extension/BUILD.bazel b/tests/cc/py_extension/BUILD.bazel index 5ee8a15b6e..c687872ec6 100644 --- a/tests/cc/py_extension/BUILD.bazel +++ b/tests/cc/py_extension/BUILD.bazel @@ -27,6 +27,12 @@ py_extension( deps = [":static_dep"], ) +py_extension( + name = "ext_with_data", + deps = [":static_dep"], + data = ["test_symbols.h"], +) + py_extension( # A python extension that dynamically links to another shared library name = "ext_shared", diff --git a/tests/cc/py_extension/py_extension_tests.bzl b/tests/cc/py_extension/py_extension_tests.bzl index 3305d8d8d9..b21880f83f 100644 --- a/tests/cc/py_extension/py_extension_tests.bzl +++ b/tests/cc/py_extension/py_extension_tests.bzl @@ -41,6 +41,26 @@ def _test_static_deps(name): _tests.append(_test_static_deps) +def _test_data_deps_impl(env, target): + env.expect.that_target(target).has_provider(PyInfo) + py_info = target[PyInfo] + env.expect.that_target(target).has_provider(CcSharedLibraryInfo) + + # Check that data file is in runfiles + default_info = target[DefaultInfo] + env.expect.that_depset_of_files(default_info.default_runfiles.files).contains_predicate( + matching.file_basename_equals("test_symbols.h"), + ) + +def _test_data_deps(name): + analysis_test( + name = name, + impl = _test_data_deps_impl, + target = "//tests/cc/py_extension:ext_with_data", + ) + +_tests.append(_test_data_deps) + def _test_dynamic_deps_impl(env, target): env.expect.that_target(target).has_provider(PyInfo) py_info = target[PyInfo] From 6f05ae0ee7474d4d15063b380effca4820fbd012 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 7 Jul 2026 00:06:54 +0000 Subject: [PATCH 40/58] Add missing file --- tests/cc/py_extension/test_symbols.h | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 tests/cc/py_extension/test_symbols.h diff --git a/tests/cc/py_extension/test_symbols.h b/tests/cc/py_extension/test_symbols.h new file mode 100644 index 0000000000..59ab3b02e7 --- /dev/null +++ b/tests/cc/py_extension/test_symbols.h @@ -0,0 +1,8 @@ +#ifndef TEST_SYMBOLS_H +#define TEST_SYMBOLS_H + +void fnC(); +void fnB(); +void fnA(); + +#endif // TEST_SYMBOLS_H From 8f2fa549d6c3294560bd751fe393d6bf051a73da Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 7 Jul 2026 16:36:32 +0000 Subject: [PATCH 41/58] Add a couple more examples for PyInit_* defined in dependency targets. --- tests/cc/py_extension/BUILD.bazel | 64 +++++++++++++++++++++++++++---- 1 file changed, 57 insertions(+), 7 deletions(-) diff --git a/tests/cc/py_extension/BUILD.bazel b/tests/cc/py_extension/BUILD.bazel index c687872ec6..65d37b73da 100644 --- a/tests/cc/py_extension/BUILD.bazel +++ b/tests/cc/py_extension/BUILD.bazel @@ -15,24 +15,39 @@ package( licenses(["notice"]) +##### + py_extension( # An extension defined solely by source files, with no deps name = "ext_source", srcs = ["ext_source.c"], ) +##### + py_extension( # A python extension that gets its code from a statically-linked library name = "ext_static", deps = [":static_dep"], ) +cc_library( + name = "static_dep", + srcs = ["static_dep.c"], + hdrs = ["static_dep.h"], +) + +##### + py_extension( + # An extension that also depends on a data file name = "ext_with_data", - deps = [":static_dep"], data = ["test_symbols.h"], + deps = [":static_dep"], ) +##### + py_extension( # A python extension that dynamically links to another shared library name = "ext_shared", @@ -62,12 +77,6 @@ cc_library( ], ) -cc_library( - name = "static_dep", - srcs = ["static_dep.c"], - hdrs = ["static_dep.h"], -) - cc_shared_library( name = "add_one_shared", deps = [":add_one_impl"], @@ -93,6 +102,8 @@ cc_library( hdrs = ["add_one_helper.h"], ) +##### + py_extension( # An extension that uses the Python limited API name = "ext_limited", @@ -113,6 +124,45 @@ cc_library( ], ) +##### + +py_extension( + # An extension with its PyInit_* function in a static dependency + name = "ext_init_in_dep", + deps = [":ext_init_in_dep_impl"], +) + +cc_library( + name = "ext_init_in_dep_impl", + srcs = ["ext_init_in_dep.c"], + deps = [ + "@rules_python//python/cc:current_py_cc_headers", + ], +) + +##### + +py_extension( + # An extension with its PyInit_* function in a dynamic dependency + name = "ext_init_in_dynamic_dep", + dynamic_deps = [":ext_init_in_dynamic_dep_2"], +) + +cc_shared_library( + name = "ext_init_in_dynamic_dep_2", + deps = [":ext_init_in_dynamic_dep_impl"], +) + +cc_library( + name = "ext_init_in_dynamic_dep_impl", + srcs = ["ext_init_in_dynamic_dep.c"], + deps = [ + "@rules_python//python/cc:current_py_cc_headers", + ], +) + +##### + py_test( name = "py_extension_test", srcs = ["py_extension_test.py"], From 908ccd5bc63b8fa52041528f6f32611e8b29cc77 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 7 Jul 2026 16:55:20 +0000 Subject: [PATCH 42/58] Add missing files --- tests/cc/py_extension/ext_init_in_dep.c | 20 +++++++++++++++++++ .../cc/py_extension/ext_init_in_dynamic_dep.c | 20 +++++++++++++++++++ 2 files changed, 40 insertions(+) create mode 100644 tests/cc/py_extension/ext_init_in_dep.c create mode 100644 tests/cc/py_extension/ext_init_in_dynamic_dep.c diff --git a/tests/cc/py_extension/ext_init_in_dep.c b/tests/cc/py_extension/ext_init_in_dep.c new file mode 100644 index 0000000000..a6d32ba0b2 --- /dev/null +++ b/tests/cc/py_extension/ext_init_in_dep.c @@ -0,0 +1,20 @@ + +#include + +// No methods defined; we're just testing the init function. +static PyMethodDef ModuleMethods[] = { + {NULL, NULL, 0, NULL} /* Sentinel */ +}; + +static struct PyModuleDef ext_init_in_dep_module = { + PyModuleDef_HEAD_INIT, + "ext_init_in_dep", /* 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 +}; + +PyMODINIT_FUNC PyInit_ext_init_in_dep(void) { + return PyModule_Create(&ext_init_in_dep_module); +} diff --git a/tests/cc/py_extension/ext_init_in_dynamic_dep.c b/tests/cc/py_extension/ext_init_in_dynamic_dep.c new file mode 100644 index 0000000000..ad196adb06 --- /dev/null +++ b/tests/cc/py_extension/ext_init_in_dynamic_dep.c @@ -0,0 +1,20 @@ + +#include + +// No methods defined; we're just testing the init function. +static PyMethodDef ModuleMethods[] = { + {NULL, NULL, 0, NULL} /* Sentinel */ +}; + +static struct PyModuleDef ext_init_in_dynamic_dep_module = { + PyModuleDef_HEAD_INIT, + "ext_init_in_dynamic_dep", /* 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 +}; + +PyMODINIT_FUNC PyInit_ext_init_in_dynamic_dep(void) { + return PyModule_Create(&ext_init_in_dynamic_dep_module); +} From eb4b8a8981125b77ac7df1a859933fa8b5d38357 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 7 Jul 2026 17:54:12 +0000 Subject: [PATCH 43/58] Expand the analysis tests with more details assertions. --- .../py_extension/dependency_graph_tests.bzl | 217 ++++++++++++------ 1 file changed, 145 insertions(+), 72 deletions(-) diff --git a/tests/cc/py_extension/dependency_graph_tests.bzl b/tests/cc/py_extension/dependency_graph_tests.bzl index 95b32560bc..be82931c98 100644 --- a/tests/cc/py_extension/dependency_graph_tests.bzl +++ b/tests/cc/py_extension/dependency_graph_tests.bzl @@ -21,8 +21,8 @@ load("@rules_testing//lib:analysis_test.bzl", "analysis_test", "test_suite") load("@rules_testing//lib:util.bzl", "util") load("//python/cc:py_extension.bzl", "py_extension") -# Test 1: CSL A -> CSL B -> CSL C (Dynamic deps) -def _test_csl_dynamic_deps(name): +# For tests 1 and 2 +def _create_dynamic_deps_helpers(name): util.helper_target( cc_library, name = name + "_libC", @@ -57,6 +57,10 @@ def _test_csl_dynamic_deps(name): copts = ["-fPIC"], deps = [":" + name + "_libB", ":" + name + "_libC"], ) + +# Test 1: CSL A -> CSL B -> CSL C (Dynamic deps) +def _test_csl_dynamic_deps_top(name): + _create_dynamic_deps_helpers(name) util.helper_target( cc_shared_library, name = name + "_cslA", @@ -71,43 +75,23 @@ def _test_csl_dynamic_deps(name): def _csl_dynamic_deps_test_impl(env, target): env.expect.that_target(target).has_provider(CcSharedLibraryInfo) + csl_info = target[CcSharedLibraryInfo] + + # Derive labels + test_name = target.label.name[:-5] # remove "_cslA" + libA_label = target.label.same_package_label(test_name + "_libA") + libB_label = target.label.same_package_label(test_name + "_libB") + libC_label = target.label.same_package_label(test_name + "_libC") + + env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(libA_label)]) + if hasattr(csl_info, "link_once_static_libs"): + static_libs = [str(l) for l in csl_info.link_once_static_libs] + env.expect.that_collection(static_libs).contains(str(libA_label)) + env.expect.that_collection(static_libs).contains_none_of([str(libB_label), str(libC_label)]) # Test 2: py_extension A -> CSL B -> CSL C (Dynamic deps) -def _test_pyext_dynamic_deps(name): - util.helper_target( - cc_library, - name = name + "_libC", - srcs = ["test_lib_c.c"], - hdrs = ["test_symbols.h"], - copts = ["-fPIC"], - ) - util.helper_target( - cc_shared_library, - name = name + "_cslC", - deps = [":" + name + "_libC"], - ) - util.helper_target( - cc_library, - name = name + "_libB", - srcs = ["test_lib_b.c"], - hdrs = ["test_symbols.h"], - copts = ["-fPIC"], - deps = [":" + name + "_libC"], - ) - util.helper_target( - cc_shared_library, - name = name + "_cslB", - deps = [":" + name + "_libB"], - dynamic_deps = [":" + name + "_cslC"], - ) - util.helper_target( - cc_library, - name = name + "_libA", - srcs = ["test_lib_a.c"], - hdrs = ["test_symbols.h"], - copts = ["-fPIC"], - deps = [":" + name + "_libB", ":" + name + "_libC"], - ) +def _test_pyext_dynamic_deps_top(name): + _create_dynamic_deps_helpers(name) py_extension( name = name + "_pyextA", deps = [":" + name + "_libA"], @@ -119,11 +103,72 @@ def _test_pyext_dynamic_deps(name): impl = _pyext_dynamic_deps_test_impl, ) +def _test_pyext_dynamic_deps_cslB(name): + _create_dynamic_deps_helpers(name) + analysis_test( + name = name, + target = name + "_cslB", + impl = _cslB_deps_test_impl, + ) + +def _test_pyext_dynamic_deps_cslC(name): + _create_dynamic_deps_helpers(name) + analysis_test( + name = name, + target = name + "_cslC", + impl = _cslC_deps_test_impl, + ) + +def _cslC_deps_test_impl(env, target): + env.expect.that_target(target).has_provider(CcSharedLibraryInfo) + csl_info = target[CcSharedLibraryInfo] + + # Derive labels + test_name = target.label.name[:-5] # remove "_cslC" + libC_label = target.label.same_package_label(test_name + "_libC") + + env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(libC_label)]) + if hasattr(csl_info, "link_once_static_libs"): + env.expect.that_collection([str(l) for l in csl_info.link_once_static_libs]).contains_exactly([str(libC_label)]) + +def _cslB_deps_test_impl(env, target): + env.expect.that_target(target).has_provider(CcSharedLibraryInfo) + csl_info = target[CcSharedLibraryInfo] + + # Derive labels + test_name = target.label.name[:-5] # remove "_cslB" + libB_label = target.label.same_package_label(test_name + "_libB") + libC_label = target.label.same_package_label(test_name + "_libC") + cslC_label = target.label.same_package_label(test_name + "_cslC") + + env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(libB_label)]) + if hasattr(csl_info, "link_once_static_libs"): + static_libs = [str(l) for l in csl_info.link_once_static_libs] + env.expect.that_collection(static_libs).contains(str(libB_label)) + env.expect.that_collection(static_libs).contains_none_of([str(libC_label)]) + + if hasattr(csl_info, "dynamic_deps"): + dynamic_deps = [str(d.linker_input.owner) for d in csl_info.dynamic_deps.to_list()] + env.expect.that_collection(dynamic_deps).contains(str(cslC_label)) + def _pyext_dynamic_deps_test_impl(env, target): env.expect.that_target(target).has_provider(CcSharedLibraryInfo) + csl_info = target[CcSharedLibraryInfo] -# Test 3: CSL A -> CSL B, CL C (Static sharing) -def _test_csl_static_sharing(name): + # Derive labels + test_name = target.label.name[:-7] # remove "_pyextA" + libA_label = target.label.same_package_label(test_name + "_libA") + libB_label = target.label.same_package_label(test_name + "_libB") + libC_label = target.label.same_package_label(test_name + "_libC") + + env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(libA_label)]) + if hasattr(csl_info, "link_once_static_libs"): + static_libs = [str(l) for l in csl_info.link_once_static_libs] + env.expect.that_collection(static_libs).contains(str(libA_label)) + env.expect.that_collection(static_libs).contains_none_of([str(libB_label), str(libC_label)]) + +# For tests 3 and 4 +def _create_static_sharing_helpers(name): util.helper_target( cc_library, name = name + "_libC", @@ -152,6 +197,10 @@ def _test_csl_static_sharing(name): copts = ["-fPIC"], deps = [":" + name + "_libB", ":" + name + "_libC"], ) + +# Test 3: CSL A -> CSL B, CL C (Static sharing) +def _test_csl_static_sharing_top(name): + _create_static_sharing_helpers(name) util.helper_target( cc_shared_library, name = name + "_cslA", @@ -166,37 +215,23 @@ def _test_csl_static_sharing(name): def _csl_static_sharing_test_impl(env, target): env.expect.that_target(target).has_provider(CcSharedLibraryInfo) + csl_info = target[CcSharedLibraryInfo] + + # Derive labels + test_name = target.label.name[:-5] # remove "_cslA" + libA_label = target.label.same_package_label(test_name + "_libA") + libB_label = target.label.same_package_label(test_name + "_libB") + libC_label = target.label.same_package_label(test_name + "_libC") + + env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(libA_label)]) + if hasattr(csl_info, "link_once_static_libs"): + static_libs = [str(l) for l in csl_info.link_once_static_libs] + env.expect.that_collection(static_libs).contains(str(libA_label)) + env.expect.that_collection(static_libs).contains_none_of([str(libB_label), str(libC_label)]) # Test 4: Same as 3, but A is py_extension -def _test_pyext_static_sharing(name): - util.helper_target( - cc_library, - name = name + "_libC", - srcs = ["test_lib_c.c"], - hdrs = ["test_symbols.h"], - copts = ["-fPIC"], - ) - util.helper_target( - cc_library, - name = name + "_libB", - srcs = ["test_lib_b.c"], - hdrs = ["test_symbols.h"], - copts = ["-fPIC"], - deps = [":" + name + "_libC"], - ) - util.helper_target( - cc_shared_library, - name = name + "_cslB", - deps = [":" + name + "_libB", ":" + name + "_libC"], - ) - util.helper_target( - cc_library, - name = name + "_libA", - srcs = ["test_lib_a.c"], - hdrs = ["test_symbols.h"], - copts = ["-fPIC"], - deps = [":" + name + "_libB", ":" + name + "_libC"], - ) +def _test_pyext_static_sharing_top(name): + _create_static_sharing_helpers(name) py_extension( name = name + "_pyextA", deps = [":" + name + "_libA"], @@ -208,16 +243,54 @@ def _test_pyext_static_sharing(name): impl = _pyext_static_sharing_test_impl, ) +def _test_pyext_static_sharing_cslB(name): + _create_static_sharing_helpers(name) + analysis_test( + name = name, + target = name + "_cslB", + impl = _cslB_static_sharing_test_impl, + ) + +def _cslB_static_sharing_test_impl(env, target): + env.expect.that_target(target).has_provider(CcSharedLibraryInfo) + csl_info = target[CcSharedLibraryInfo] + + # Derive labels + test_name = target.label.name[:-5] # remove "_cslB" + libB_label = target.label.same_package_label(test_name + "_libB") + libC_label = target.label.same_package_label(test_name + "_libC") + + env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(libB_label), str(libC_label)]) + if hasattr(csl_info, "link_once_static_libs"): + static_libs = [str(l) for l in csl_info.link_once_static_libs] + env.expect.that_collection(static_libs).contains_exactly([str(libB_label), str(libC_label)]) + def _pyext_static_sharing_test_impl(env, target): env.expect.that_target(target).has_provider(CcSharedLibraryInfo) + csl_info = target[CcSharedLibraryInfo] + + # Derive labels + test_name = target.label.name[:-7] # remove "_pyextA" + libA_label = target.label.same_package_label(test_name + "_libA") + libB_label = target.label.same_package_label(test_name + "_libB") + libC_label = target.label.same_package_label(test_name + "_libC") + + env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(libA_label)]) + if hasattr(csl_info, "link_once_static_libs"): + static_libs = [str(l) for l in csl_info.link_once_static_libs] + env.expect.that_collection(static_libs).contains(str(libA_label)) + env.expect.that_collection(static_libs).contains_none_of([str(libB_label), str(libC_label)]) def dependency_graph_test_suite(name): test_suite( name = name, tests = [ - _test_csl_dynamic_deps, - _test_pyext_dynamic_deps, - _test_csl_static_sharing, - _test_pyext_static_sharing, + _test_csl_dynamic_deps_top, + _test_pyext_dynamic_deps_top, + _test_pyext_dynamic_deps_cslB, + _test_pyext_dynamic_deps_cslC, + _test_csl_static_sharing_top, + _test_pyext_static_sharing_top, + _test_pyext_static_sharing_cslB, ], ) From 12bc59ea05eceecc1c1a9be0d6147c8779aa9978 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 7 Jul 2026 19:50:07 +0000 Subject: [PATCH 44/58] Re-format files --- python/private/cc/py_extension_macro.bzl | 2 +- python/private/cc/py_extension_rule.bzl | 10 +++++----- tests/cc/py_extension/dependency_graph_tests.bzl | 14 +++++++------- 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/python/private/cc/py_extension_macro.bzl b/python/private/cc/py_extension_macro.bzl index 5bf5e6c8e3..ea25f92bb8 100644 --- a/python/private/cc/py_extension_macro.bzl +++ b/python/private/cc/py_extension_macro.bzl @@ -78,9 +78,9 @@ def py_extension( # 5. Select default libc constraint if not provided if "libc" not in kwargs: kwargs["libc"] = select({ - "//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", + "//conditions:default": "glibc", }) # 6. Wrap with py_extension_wrapper for PEP 3149 naming & PyInfo diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index a504a49b4d..86fd801ace 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -65,16 +65,16 @@ def _py_extension_wrapper_impl(ctx): ] PY_EXTENSION_WRAPPER_ATTRS = COMMON_ATTRS | { - "src": lambda: attrb.Label( - mandatory = True, - providers = [CcSharedLibraryInfo], - doc = "The cc_shared_library target to wrap.", - ), "libc": lambda: attrb.String(default = "glibc"), "module_name": lambda: attrb.String(), "py_limited_api": lambda: attrb.String( default = "", ), + "src": lambda: attrb.Label( + mandatory = True, + providers = [CcSharedLibraryInfo], + doc = "The cc_shared_library target to wrap.", + ), "_constraints": lambda: attrb.LabelList( default = sorted({ c: None diff --git a/tests/cc/py_extension/dependency_graph_tests.bzl b/tests/cc/py_extension/dependency_graph_tests.bzl index be82931c98..bdfaa86865 100644 --- a/tests/cc/py_extension/dependency_graph_tests.bzl +++ b/tests/cc/py_extension/dependency_graph_tests.bzl @@ -78,7 +78,7 @@ def _csl_dynamic_deps_test_impl(env, target): csl_info = target[CcSharedLibraryInfo] # Derive labels - test_name = target.label.name[:-5] # remove "_cslA" + test_name = target.label.name[:-5] # remove "_cslA" libA_label = target.label.same_package_label(test_name + "_libA") libB_label = target.label.same_package_label(test_name + "_libB") libC_label = target.label.same_package_label(test_name + "_libC") @@ -124,7 +124,7 @@ def _cslC_deps_test_impl(env, target): csl_info = target[CcSharedLibraryInfo] # Derive labels - test_name = target.label.name[:-5] # remove "_cslC" + test_name = target.label.name[:-5] # remove "_cslC" libC_label = target.label.same_package_label(test_name + "_libC") env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(libC_label)]) @@ -136,7 +136,7 @@ def _cslB_deps_test_impl(env, target): csl_info = target[CcSharedLibraryInfo] # Derive labels - test_name = target.label.name[:-5] # remove "_cslB" + test_name = target.label.name[:-5] # remove "_cslB" libB_label = target.label.same_package_label(test_name + "_libB") libC_label = target.label.same_package_label(test_name + "_libC") cslC_label = target.label.same_package_label(test_name + "_cslC") @@ -156,7 +156,7 @@ def _pyext_dynamic_deps_test_impl(env, target): csl_info = target[CcSharedLibraryInfo] # Derive labels - test_name = target.label.name[:-7] # remove "_pyextA" + test_name = target.label.name[:-7] # remove "_pyextA" libA_label = target.label.same_package_label(test_name + "_libA") libB_label = target.label.same_package_label(test_name + "_libB") libC_label = target.label.same_package_label(test_name + "_libC") @@ -218,7 +218,7 @@ def _csl_static_sharing_test_impl(env, target): csl_info = target[CcSharedLibraryInfo] # Derive labels - test_name = target.label.name[:-5] # remove "_cslA" + test_name = target.label.name[:-5] # remove "_cslA" libA_label = target.label.same_package_label(test_name + "_libA") libB_label = target.label.same_package_label(test_name + "_libB") libC_label = target.label.same_package_label(test_name + "_libC") @@ -256,7 +256,7 @@ def _cslB_static_sharing_test_impl(env, target): csl_info = target[CcSharedLibraryInfo] # Derive labels - test_name = target.label.name[:-5] # remove "_cslB" + test_name = target.label.name[:-5] # remove "_cslB" libB_label = target.label.same_package_label(test_name + "_libB") libC_label = target.label.same_package_label(test_name + "_libC") @@ -270,7 +270,7 @@ def _pyext_static_sharing_test_impl(env, target): csl_info = target[CcSharedLibraryInfo] # Derive labels - test_name = target.label.name[:-7] # remove "_pyextA" + test_name = target.label.name[:-7] # remove "_pyextA" libA_label = target.label.same_package_label(test_name + "_libA") libB_label = target.label.same_package_label(test_name + "_libB") libC_label = target.label.same_package_label(test_name + "_libC") From b1589755a691a98e91e4616e9ffd98e5615b2b22 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 7 Jul 2026 20:36:24 +0000 Subject: [PATCH 45/58] Address buildifier and buildifier-lint warnings. --- python/private/cc/py_extension_macro.bzl | 4 + .../py_extension/dependency_graph_tests.bzl | 88 +++++++++---------- tests/cc/py_extension/py_extension_tests.bzl | 4 +- 3 files changed, 49 insertions(+), 47 deletions(-) diff --git a/python/private/cc/py_extension_macro.bzl b/python/private/cc/py_extension_macro.bzl index ea25f92bb8..3a5aa0d72f 100644 --- a/python/private/cc/py_extension_macro.bzl +++ b/python/private/cc/py_extension_macro.bzl @@ -16,6 +16,7 @@ def py_extension( exports_filter = None, user_link_flags = None, visibility = None, + data = None, **kwargs): """Creates a Python extension module. @@ -83,6 +84,9 @@ def py_extension( "//conditions:default": "glibc", }) + if data != None: + kwargs["data"] = data + # 6. Wrap with py_extension_wrapper for PEP 3149 naming & PyInfo py_extension_wrapper( name = name, diff --git a/tests/cc/py_extension/dependency_graph_tests.bzl b/tests/cc/py_extension/dependency_graph_tests.bzl index bdfaa86865..16ee883465 100644 --- a/tests/cc/py_extension/dependency_graph_tests.bzl +++ b/tests/cc/py_extension/dependency_graph_tests.bzl @@ -79,15 +79,15 @@ def _csl_dynamic_deps_test_impl(env, target): # Derive labels test_name = target.label.name[:-5] # remove "_cslA" - libA_label = target.label.same_package_label(test_name + "_libA") - libB_label = target.label.same_package_label(test_name + "_libB") - libC_label = target.label.same_package_label(test_name + "_libC") + lib_a_label = target.label.same_package_label(test_name + "_libA") + lib_b_label = target.label.same_package_label(test_name + "_libB") + lib_c_label = target.label.same_package_label(test_name + "_libC") - env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(libA_label)]) + env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(lib_a_label)]) if hasattr(csl_info, "link_once_static_libs"): - static_libs = [str(l) for l in csl_info.link_once_static_libs] - env.expect.that_collection(static_libs).contains(str(libA_label)) - env.expect.that_collection(static_libs).contains_none_of([str(libB_label), str(libC_label)]) + static_libs = [str(lbl) for lbl in csl_info.link_once_static_libs] + env.expect.that_collection(static_libs).contains(str(lib_a_label)) + env.expect.that_collection(static_libs).contains_none_of([str(lib_b_label), str(lib_c_label)]) # Test 2: py_extension A -> CSL B -> CSL C (Dynamic deps) def _test_pyext_dynamic_deps_top(name): @@ -125,11 +125,11 @@ def _cslC_deps_test_impl(env, target): # Derive labels test_name = target.label.name[:-5] # remove "_cslC" - libC_label = target.label.same_package_label(test_name + "_libC") + lib_c_label = target.label.same_package_label(test_name + "_libC") - env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(libC_label)]) + env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(lib_c_label)]) if hasattr(csl_info, "link_once_static_libs"): - env.expect.that_collection([str(l) for l in csl_info.link_once_static_libs]).contains_exactly([str(libC_label)]) + env.expect.that_collection([str(lbl) for lbl in csl_info.link_once_static_libs]).contains_exactly([str(lib_c_label)]) def _cslB_deps_test_impl(env, target): env.expect.that_target(target).has_provider(CcSharedLibraryInfo) @@ -137,19 +137,19 @@ def _cslB_deps_test_impl(env, target): # Derive labels test_name = target.label.name[:-5] # remove "_cslB" - libB_label = target.label.same_package_label(test_name + "_libB") - libC_label = target.label.same_package_label(test_name + "_libC") - cslC_label = target.label.same_package_label(test_name + "_cslC") + lib_b_label = target.label.same_package_label(test_name + "_libB") + lib_c_label = target.label.same_package_label(test_name + "_libC") + csl_c_label = target.label.same_package_label(test_name + "_cslC") - env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(libB_label)]) + env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(lib_b_label)]) if hasattr(csl_info, "link_once_static_libs"): - static_libs = [str(l) for l in csl_info.link_once_static_libs] - env.expect.that_collection(static_libs).contains(str(libB_label)) - env.expect.that_collection(static_libs).contains_none_of([str(libC_label)]) + static_libs = [str(lbl) for lbl in csl_info.link_once_static_libs] + env.expect.that_collection(static_libs).contains(str(lib_b_label)) + env.expect.that_collection(static_libs).contains_none_of([str(lib_c_label)]) if hasattr(csl_info, "dynamic_deps"): dynamic_deps = [str(d.linker_input.owner) for d in csl_info.dynamic_deps.to_list()] - env.expect.that_collection(dynamic_deps).contains(str(cslC_label)) + env.expect.that_collection(dynamic_deps).contains(str(csl_c_label)) def _pyext_dynamic_deps_test_impl(env, target): env.expect.that_target(target).has_provider(CcSharedLibraryInfo) @@ -157,15 +157,15 @@ def _pyext_dynamic_deps_test_impl(env, target): # Derive labels test_name = target.label.name[:-7] # remove "_pyextA" - libA_label = target.label.same_package_label(test_name + "_libA") - libB_label = target.label.same_package_label(test_name + "_libB") - libC_label = target.label.same_package_label(test_name + "_libC") + lib_a_label = target.label.same_package_label(test_name + "_libA") + lib_b_label = target.label.same_package_label(test_name + "_libB") + lib_c_label = target.label.same_package_label(test_name + "_libC") - env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(libA_label)]) + env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(lib_a_label)]) if hasattr(csl_info, "link_once_static_libs"): - static_libs = [str(l) for l in csl_info.link_once_static_libs] - env.expect.that_collection(static_libs).contains(str(libA_label)) - env.expect.that_collection(static_libs).contains_none_of([str(libB_label), str(libC_label)]) + static_libs = [str(lbl) for lbl in csl_info.link_once_static_libs] + env.expect.that_collection(static_libs).contains(str(lib_a_label)) + env.expect.that_collection(static_libs).contains_none_of([str(lib_b_label), str(lib_c_label)]) # For tests 3 and 4 def _create_static_sharing_helpers(name): @@ -219,15 +219,15 @@ def _csl_static_sharing_test_impl(env, target): # Derive labels test_name = target.label.name[:-5] # remove "_cslA" - libA_label = target.label.same_package_label(test_name + "_libA") - libB_label = target.label.same_package_label(test_name + "_libB") - libC_label = target.label.same_package_label(test_name + "_libC") + lib_a_label = target.label.same_package_label(test_name + "_libA") + lib_b_label = target.label.same_package_label(test_name + "_libB") + lib_c_label = target.label.same_package_label(test_name + "_libC") - env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(libA_label)]) + env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(lib_a_label)]) if hasattr(csl_info, "link_once_static_libs"): - static_libs = [str(l) for l in csl_info.link_once_static_libs] - env.expect.that_collection(static_libs).contains(str(libA_label)) - env.expect.that_collection(static_libs).contains_none_of([str(libB_label), str(libC_label)]) + static_libs = [str(lbl) for lbl in csl_info.link_once_static_libs] + env.expect.that_collection(static_libs).contains(str(lib_a_label)) + env.expect.that_collection(static_libs).contains_none_of([str(lib_b_label), str(lib_c_label)]) # Test 4: Same as 3, but A is py_extension def _test_pyext_static_sharing_top(name): @@ -257,13 +257,13 @@ def _cslB_static_sharing_test_impl(env, target): # Derive labels test_name = target.label.name[:-5] # remove "_cslB" - libB_label = target.label.same_package_label(test_name + "_libB") - libC_label = target.label.same_package_label(test_name + "_libC") + lib_b_label = target.label.same_package_label(test_name + "_libB") + lib_c_label = target.label.same_package_label(test_name + "_libC") - env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(libB_label), str(libC_label)]) + env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(lib_b_label), str(lib_c_label)]) if hasattr(csl_info, "link_once_static_libs"): - static_libs = [str(l) for l in csl_info.link_once_static_libs] - env.expect.that_collection(static_libs).contains_exactly([str(libB_label), str(libC_label)]) + static_libs = [str(lbl) for lbl in csl_info.link_once_static_libs] + env.expect.that_collection(static_libs).contains_exactly([str(lib_b_label), str(lib_c_label)]) def _pyext_static_sharing_test_impl(env, target): env.expect.that_target(target).has_provider(CcSharedLibraryInfo) @@ -271,15 +271,15 @@ def _pyext_static_sharing_test_impl(env, target): # Derive labels test_name = target.label.name[:-7] # remove "_pyextA" - libA_label = target.label.same_package_label(test_name + "_libA") - libB_label = target.label.same_package_label(test_name + "_libB") - libC_label = target.label.same_package_label(test_name + "_libC") + lib_a_label = target.label.same_package_label(test_name + "_libA") + lib_b_label = target.label.same_package_label(test_name + "_libB") + lib_c_label = target.label.same_package_label(test_name + "_libC") - env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(libA_label)]) + env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(lib_a_label)]) if hasattr(csl_info, "link_once_static_libs"): - static_libs = [str(l) for l in csl_info.link_once_static_libs] - env.expect.that_collection(static_libs).contains(str(libA_label)) - env.expect.that_collection(static_libs).contains_none_of([str(libB_label), str(libC_label)]) + static_libs = [str(lbl) for lbl in csl_info.link_once_static_libs] + env.expect.that_collection(static_libs).contains(str(lib_a_label)) + env.expect.that_collection(static_libs).contains_none_of([str(lib_b_label), str(lib_c_label)]) def dependency_graph_test_suite(name): test_suite( diff --git a/tests/cc/py_extension/py_extension_tests.bzl b/tests/cc/py_extension/py_extension_tests.bzl index b21880f83f..13fb4e9e0c 100644 --- a/tests/cc/py_extension/py_extension_tests.bzl +++ b/tests/cc/py_extension/py_extension_tests.bzl @@ -17,7 +17,7 @@ load("@rules_cc//cc/common:cc_shared_library_info.bzl", "CcSharedLibraryInfo") 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") +load("//python/private:py_info.bzl", "PyInfo") # buildifier: disable=bzl-visibility _tests = [] @@ -43,7 +43,6 @@ _tests.append(_test_static_deps) def _test_data_deps_impl(env, target): env.expect.that_target(target).has_provider(PyInfo) - py_info = target[PyInfo] env.expect.that_target(target).has_provider(CcSharedLibraryInfo) # Check that data file is in runfiles @@ -65,7 +64,6 @@ def _test_dynamic_deps_impl(env, target): env.expect.that_target(target).has_provider(PyInfo) py_info = target[PyInfo] env.expect.that_target(target).has_provider(CcSharedLibraryInfo) - csl_info = target[CcSharedLibraryInfo] # The .so should be in PyInfo env.expect.that_collection(py_info.transitive_sources.to_list()).has_size(1) From 6e1763e6793a1e8ca4e415f2bcf91895a97c0d3d Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Thu, 9 Jul 2026 20:31:35 +0000 Subject: [PATCH 46/58] Use text instead of code file for data. --- tests/cc/py_extension/BUILD.bazel | 2 +- tests/cc/py_extension/some_data.txt | 1 + tests/cc/py_extension/test_symbols.h | 8 -------- 3 files changed, 2 insertions(+), 9 deletions(-) create mode 100644 tests/cc/py_extension/some_data.txt delete mode 100644 tests/cc/py_extension/test_symbols.h diff --git a/tests/cc/py_extension/BUILD.bazel b/tests/cc/py_extension/BUILD.bazel index 65d37b73da..8c49343db6 100644 --- a/tests/cc/py_extension/BUILD.bazel +++ b/tests/cc/py_extension/BUILD.bazel @@ -42,7 +42,7 @@ cc_library( py_extension( # An extension that also depends on a data file name = "ext_with_data", - data = ["test_symbols.h"], + data = ["some_data.txt"], deps = [":static_dep"], ) diff --git a/tests/cc/py_extension/some_data.txt b/tests/cc/py_extension/some_data.txt new file mode 100644 index 0000000000..4b5dc1d64c --- /dev/null +++ b/tests/cc/py_extension/some_data.txt @@ -0,0 +1 @@ +This is a data file diff --git a/tests/cc/py_extension/test_symbols.h b/tests/cc/py_extension/test_symbols.h deleted file mode 100644 index 59ab3b02e7..0000000000 --- a/tests/cc/py_extension/test_symbols.h +++ /dev/null @@ -1,8 +0,0 @@ -#ifndef TEST_SYMBOLS_H -#define TEST_SYMBOLS_H - -void fnC(); -void fnB(); -void fnA(); - -#endif // TEST_SYMBOLS_H From d324300b736f8207c234454eac923bfd45ff2ad9 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Thu, 9 Jul 2026 20:40:31 +0000 Subject: [PATCH 47/58] Ignore unusual edge case. --- tests/cc/py_extension/BUILD.bazel | 21 ------------------- .../cc/py_extension/ext_init_in_dynamic_dep.c | 20 ------------------ 2 files changed, 41 deletions(-) delete mode 100644 tests/cc/py_extension/ext_init_in_dynamic_dep.c diff --git a/tests/cc/py_extension/BUILD.bazel b/tests/cc/py_extension/BUILD.bazel index 8c49343db6..3903d30537 100644 --- a/tests/cc/py_extension/BUILD.bazel +++ b/tests/cc/py_extension/BUILD.bazel @@ -142,27 +142,6 @@ cc_library( ##### -py_extension( - # An extension with its PyInit_* function in a dynamic dependency - name = "ext_init_in_dynamic_dep", - dynamic_deps = [":ext_init_in_dynamic_dep_2"], -) - -cc_shared_library( - name = "ext_init_in_dynamic_dep_2", - deps = [":ext_init_in_dynamic_dep_impl"], -) - -cc_library( - name = "ext_init_in_dynamic_dep_impl", - srcs = ["ext_init_in_dynamic_dep.c"], - deps = [ - "@rules_python//python/cc:current_py_cc_headers", - ], -) - -##### - py_test( name = "py_extension_test", srcs = ["py_extension_test.py"], diff --git a/tests/cc/py_extension/ext_init_in_dynamic_dep.c b/tests/cc/py_extension/ext_init_in_dynamic_dep.c deleted file mode 100644 index ad196adb06..0000000000 --- a/tests/cc/py_extension/ext_init_in_dynamic_dep.c +++ /dev/null @@ -1,20 +0,0 @@ - -#include - -// No methods defined; we're just testing the init function. -static PyMethodDef ModuleMethods[] = { - {NULL, NULL, 0, NULL} /* Sentinel */ -}; - -static struct PyModuleDef ext_init_in_dynamic_dep_module = { - PyModuleDef_HEAD_INIT, - "ext_init_in_dynamic_dep", /* 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 -}; - -PyMODINIT_FUNC PyInit_ext_init_in_dynamic_dep(void) { - return PyModule_Create(&ext_init_in_dynamic_dep_module); -} From 0f4edda0cf664c1cfba8f921ab816ce3d771626c Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Thu, 9 Jul 2026 20:44:18 +0000 Subject: [PATCH 48/58] Restore missing file. --- tests/cc/py_extension/test_symbols.h | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 tests/cc/py_extension/test_symbols.h diff --git a/tests/cc/py_extension/test_symbols.h b/tests/cc/py_extension/test_symbols.h new file mode 100644 index 0000000000..59ab3b02e7 --- /dev/null +++ b/tests/cc/py_extension/test_symbols.h @@ -0,0 +1,8 @@ +#ifndef TEST_SYMBOLS_H +#define TEST_SYMBOLS_H + +void fnC(); +void fnB(); +void fnA(); + +#endif // TEST_SYMBOLS_H From bae75f780c97686497bff79ec6d9cccfc6926366 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Fri, 10 Jul 2026 18:31:29 +0000 Subject: [PATCH 49/58] Remove the CcSharedLibraryInfo provider for now. --- python/private/cc/py_extension_rule.bzl | 3 +- .../py_extension/dependency_graph_tests.bzl | 60 ------------------- 2 files changed, 1 insertion(+), 62 deletions(-) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 86fd801ace..7736985111 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -61,7 +61,6 @@ def _py_extension_wrapper_impl(ctx): transitive_sources = depset([py_dso]), imports = depset([import_path]), ), - csl_target[CcSharedLibraryInfo], ] PY_EXTENSION_WRAPPER_ATTRS = COMMON_ATTRS | { @@ -89,7 +88,7 @@ def create_py_extension_wrapper_rule_builder(**kwargs): builder = ruleb.Rule( implementation = _py_extension_wrapper_impl, attrs = PY_EXTENSION_WRAPPER_ATTRS, - provides = [PyInfo, CcSharedLibraryInfo], + provides = [PyInfo], toolchains = [ ruleb.ToolchainType(PY_CC_TOOLCHAIN_TYPE), ruleb.ToolchainType("@bazel_tools//tools/cpp:toolchain_type"), diff --git a/tests/cc/py_extension/dependency_graph_tests.bzl b/tests/cc/py_extension/dependency_graph_tests.bzl index 16ee883465..497571e128 100644 --- a/tests/cc/py_extension/dependency_graph_tests.bzl +++ b/tests/cc/py_extension/dependency_graph_tests.bzl @@ -19,7 +19,6 @@ load("@rules_cc//cc:cc_shared_library.bzl", "cc_shared_library") load("@rules_cc//cc/common:cc_shared_library_info.bzl", "CcSharedLibraryInfo") load("@rules_testing//lib:analysis_test.bzl", "analysis_test", "test_suite") load("@rules_testing//lib:util.bzl", "util") -load("//python/cc:py_extension.bzl", "py_extension") # For tests 1 and 2 def _create_dynamic_deps_helpers(name): @@ -90,19 +89,6 @@ def _csl_dynamic_deps_test_impl(env, target): env.expect.that_collection(static_libs).contains_none_of([str(lib_b_label), str(lib_c_label)]) # Test 2: py_extension A -> CSL B -> CSL C (Dynamic deps) -def _test_pyext_dynamic_deps_top(name): - _create_dynamic_deps_helpers(name) - py_extension( - name = name + "_pyextA", - deps = [":" + name + "_libA"], - dynamic_deps = [":" + name + "_cslB", ":" + name + "_cslC"], - ) - analysis_test( - name = name, - target = name + "_pyextA", - impl = _pyext_dynamic_deps_test_impl, - ) - def _test_pyext_dynamic_deps_cslB(name): _create_dynamic_deps_helpers(name) analysis_test( @@ -151,22 +137,6 @@ def _cslB_deps_test_impl(env, target): dynamic_deps = [str(d.linker_input.owner) for d in csl_info.dynamic_deps.to_list()] env.expect.that_collection(dynamic_deps).contains(str(csl_c_label)) -def _pyext_dynamic_deps_test_impl(env, target): - env.expect.that_target(target).has_provider(CcSharedLibraryInfo) - csl_info = target[CcSharedLibraryInfo] - - # Derive labels - test_name = target.label.name[:-7] # remove "_pyextA" - lib_a_label = target.label.same_package_label(test_name + "_libA") - lib_b_label = target.label.same_package_label(test_name + "_libB") - lib_c_label = target.label.same_package_label(test_name + "_libC") - - env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(lib_a_label)]) - if hasattr(csl_info, "link_once_static_libs"): - static_libs = [str(lbl) for lbl in csl_info.link_once_static_libs] - env.expect.that_collection(static_libs).contains(str(lib_a_label)) - env.expect.that_collection(static_libs).contains_none_of([str(lib_b_label), str(lib_c_label)]) - # For tests 3 and 4 def _create_static_sharing_helpers(name): util.helper_target( @@ -230,18 +200,6 @@ def _csl_static_sharing_test_impl(env, target): env.expect.that_collection(static_libs).contains_none_of([str(lib_b_label), str(lib_c_label)]) # Test 4: Same as 3, but A is py_extension -def _test_pyext_static_sharing_top(name): - _create_static_sharing_helpers(name) - py_extension( - name = name + "_pyextA", - deps = [":" + name + "_libA"], - dynamic_deps = [":" + name + "_cslB"], - ) - analysis_test( - name = name, - target = name + "_pyextA", - impl = _pyext_static_sharing_test_impl, - ) def _test_pyext_static_sharing_cslB(name): _create_static_sharing_helpers(name) @@ -265,32 +223,14 @@ def _cslB_static_sharing_test_impl(env, target): static_libs = [str(lbl) for lbl in csl_info.link_once_static_libs] env.expect.that_collection(static_libs).contains_exactly([str(lib_b_label), str(lib_c_label)]) -def _pyext_static_sharing_test_impl(env, target): - env.expect.that_target(target).has_provider(CcSharedLibraryInfo) - csl_info = target[CcSharedLibraryInfo] - - # Derive labels - test_name = target.label.name[:-7] # remove "_pyextA" - lib_a_label = target.label.same_package_label(test_name + "_libA") - lib_b_label = target.label.same_package_label(test_name + "_libB") - lib_c_label = target.label.same_package_label(test_name + "_libC") - - env.expect.that_collection([str(e) for e in csl_info.exports]).contains_exactly([str(lib_a_label)]) - if hasattr(csl_info, "link_once_static_libs"): - static_libs = [str(lbl) for lbl in csl_info.link_once_static_libs] - env.expect.that_collection(static_libs).contains(str(lib_a_label)) - env.expect.that_collection(static_libs).contains_none_of([str(lib_b_label), str(lib_c_label)]) - def dependency_graph_test_suite(name): test_suite( name = name, tests = [ _test_csl_dynamic_deps_top, - _test_pyext_dynamic_deps_top, _test_pyext_dynamic_deps_cslB, _test_pyext_dynamic_deps_cslC, _test_csl_static_sharing_top, - _test_pyext_static_sharing_top, _test_pyext_static_sharing_cslB, ], ) From b7f81121de8f509a958443515f47ba6b01d88d06 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Fri, 10 Jul 2026 22:11:40 +0000 Subject: [PATCH 50/58] Calculate the platform tag from the constraints directly, instead of using the obsolete PLATFORMS global. --- python/private/cc/py_extension_macro.bzl | 12 +++ python/private/cc/py_extension_rule.bzl | 97 +++++++----------------- 2 files changed, 38 insertions(+), 71 deletions(-) diff --git a/python/private/cc/py_extension_macro.bzl b/python/private/cc/py_extension_macro.bzl index 3a5aa0d72f..715ae5e3b6 100644 --- a/python/private/cc/py_extension_macro.bzl +++ b/python/private/cc/py_extension_macro.bzl @@ -92,5 +92,17 @@ def py_extension( name = name, src = ":" + csl_name, visibility = visibility, + os = select({ + "@platforms//os:linux": "linux", + "@platforms//os:macos": "macos", + "@platforms//os:windows": "windows", + "//conditions:default": "unknown", + }), + cpu = select({ + "@platforms//cpu:x86_64": "x86_64", + "@platforms//cpu:aarch64": "aarch64", + "@platforms//cpu:x86_32": "x86_32", + "//conditions:default": "unknown", + }), **kwargs ) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index 7736985111..744d090c09 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -74,13 +74,8 @@ PY_EXTENSION_WRAPPER_ATTRS = COMMON_ATTRS | { providers = [CcSharedLibraryInfo], doc = "The cc_shared_library target to wrap.", ), - "_constraints": lambda: attrb.LabelList( - default = sorted({ - c: None - for info in PLATFORMS.values() - for c in info.compatible_with - }.keys()), - ), + "os": lambda: attrb.String(doc = "OS determined by macro select."), + "cpu": lambda: attrb.String(doc = "CPU determined by macro select."), } def create_py_extension_wrapper_rule_builder(**kwargs): @@ -118,69 +113,16 @@ def _get_extension(cc_toolchain): ext = "pyd" if is_windows else "so" return ext -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] - - # 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 - 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: - # 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) - - return None - def _get_platform(ctx): """Derives the PEP 3149 platform tag from the target constraints. + Linux platform tags are standardized here: + - https://peps.python.org/pep-3149/ + Windows platform tags, such as they are, are defined in this issue and + commit (treated as a de facto standard): + - https://github.com/python/cpython/issues/67169 + - https://github.com/python/cpython/commit/03a144bb6ac3d7631a3bdb895e2a1f2d021fb08b + Apple platform tag is always just "darwin", discussed briefly here: + - https://github.com/python/cpython/commit/3b8124884c3655b4cf2629d741b18c1a38181805 Args: ctx: The rule context. @@ -188,9 +130,22 @@ def _get_platform(ctx): Returns: The platform tag, e.g. "x86_64-linux-gnu" or "win_amd64" """ - platform_tag = _get_platform_from_constraints(ctx) - if platform_tag: - return platform_tag + os = ctx.attr.os + cpu = ctx.attr.cpu + + if os == "windows": + if cpu == "x86_64": + return "win_amd64" + if cpu == "aarch64": + return "win_arm64" + return "win32" + if os == "macos": + return "darwin" + if os == "linux": + libc = "gnu" + if ctx.attr.libc == "musl": + libc = "musl" + return '{}-{}-{}'.format(cpu, os, libc) fail( """ From 1b31b7e17519532c53316f9a09b1fa738b4e0a0f Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Mon, 13 Jul 2026 20:22:47 +0000 Subject: [PATCH 51/58] Remove unused load() --- 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 744d090c09..ef1193d504 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -1,7 +1,6 @@ """Implementation of the _py_extension_wrapper rule.""" load("@rules_cc//cc/common:cc_shared_library_info.bzl", "CcSharedLibraryInfo") -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") From 32c37e5fc2fb00a691577eb97742df185fd58a73 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 14 Jul 2026 03:03:05 +0000 Subject: [PATCH 52/58] Fix tests. --- tests/cc/py_extension/py_extension_tests.bzl | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/tests/cc/py_extension/py_extension_tests.bzl b/tests/cc/py_extension/py_extension_tests.bzl index 13fb4e9e0c..0d66f07eec 100644 --- a/tests/cc/py_extension/py_extension_tests.bzl +++ b/tests/cc/py_extension/py_extension_tests.bzl @@ -14,7 +14,6 @@ """Tests for py_extension.""" -load("@rules_cc//cc/common:cc_shared_library_info.bzl", "CcSharedLibraryInfo") 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") # buildifier: disable=bzl-visibility @@ -24,7 +23,6 @@ _tests = [] def _test_static_deps_impl(env, target): env.expect.that_target(target).has_provider(PyInfo) py_info = target[PyInfo] - env.expect.that_target(target).has_provider(CcSharedLibraryInfo) # The .so should be in PyInfo env.expect.that_collection(py_info.transitive_sources.to_list()).has_size(1) @@ -43,12 +41,11 @@ _tests.append(_test_static_deps) def _test_data_deps_impl(env, target): env.expect.that_target(target).has_provider(PyInfo) - env.expect.that_target(target).has_provider(CcSharedLibraryInfo) # Check that data file is in runfiles default_info = target[DefaultInfo] env.expect.that_depset_of_files(default_info.default_runfiles.files).contains_predicate( - matching.file_basename_equals("test_symbols.h"), + matching.file_basename_equals("some_data.txt"), ) def _test_data_deps(name): @@ -63,7 +60,6 @@ _tests.append(_test_data_deps) def _test_dynamic_deps_impl(env, target): env.expect.that_target(target).has_provider(PyInfo) py_info = target[PyInfo] - env.expect.that_target(target).has_provider(CcSharedLibraryInfo) # The .so should be in PyInfo env.expect.that_collection(py_info.transitive_sources.to_list()).has_size(1) @@ -71,9 +67,6 @@ def _test_dynamic_deps_impl(env, target): matching.file_basename_equals("ext_shared.cpython-311-x86_64-linux-gnu.so"), ) - # CcSharedLibraryInfo provider should be present and non-empty - env.expect.that_target(target).has_provider(CcSharedLibraryInfo) - def _test_dynamic_deps(name): analysis_test( name = name, From 421eb5c22c828ab0b1e19fdc75fee3c2d3fb52f7 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 14 Jul 2026 03:13:08 +0000 Subject: [PATCH 53/58] Include free-threading 't' flag in the abi_tag, if the interpreter is free-threaded. --- python/private/py_cc_toolchain_rule.bzl | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/python/private/py_cc_toolchain_rule.bzl b/python/private/py_cc_toolchain_rule.bzl index 4067df668d..56b2e9ccc5 100644 --- a/python/private/py_cc_toolchain_rule.bzl +++ b/python/private/py_cc_toolchain_rule.bzl @@ -21,6 +21,7 @@ https://github.com/bazel-contrib/rules_python/issues/824 is considered done. load("@bazel_skylib//rules:common_settings.bzl", "BuildSettingInfo") load("@rules_cc//cc/common:cc_info.bzl", "CcInfo") load(":common_labels.bzl", "labels") +load(":flags.bzl", "FreeThreadedFlag") load(":py_cc_toolchain_info.bzl", "PyCcToolchainInfo") load(":sentinel.bzl", "SentinelInfo") @@ -47,9 +48,12 @@ def _py_cc_toolchain_impl(ctx): abi_tag = ctx.attr.abi_tag if not abi_tag: - # Derive default: cpython-XX + # Derive default: cpython-XX[t] version_parts = ctx.attr.python_version.split(".") - abi_tag = "cpython-{}{}".format(version_parts[0], version_parts[1]) + abi_flags = "" + if ctx.attr._py_freethreaded_flag[BuildSettingInfo].value == FreeThreadedFlag.YES: + abi_flags += "t" + abi_tag = "cpython-{}{}{}".format(version_parts[0], version_parts[1], abi_flags) py_cc_toolchain = PyCcToolchainInfo( abi_tag = abi_tag, @@ -107,6 +111,9 @@ attribute is available or not. doc = "The Major.minor Python version, e.g. 3.11", mandatory = True, ), + "_py_freethreaded_flag": attr.label( + default = labels.PY_FREETHREADED, + ), "_visible_for_testing": attr.label( default = labels.VISIBLE_FOR_TESTING, ), From 320c25a3f05ac11ea7f08057b2e0c892f067e11c Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 14 Jul 2026 03:51:50 +0000 Subject: [PATCH 54/58] Calculate the platform tag and include it as part of the toolchain info. --- python/private/cc/py_extension_macro.bzl | 12 ------ python/private/cc/py_extension_rule.bzl | 41 +++--------------- python/private/py_cc_toolchain_info.bzl | 5 +++ python/private/py_cc_toolchain_macro.bzl | 19 ++++++++ python/private/py_cc_toolchain_rule.bzl | 55 ++++++++++++++++++++++++ 5 files changed, 86 insertions(+), 46 deletions(-) diff --git a/python/private/cc/py_extension_macro.bzl b/python/private/cc/py_extension_macro.bzl index 715ae5e3b6..3a5aa0d72f 100644 --- a/python/private/cc/py_extension_macro.bzl +++ b/python/private/cc/py_extension_macro.bzl @@ -92,17 +92,5 @@ def py_extension( name = name, src = ":" + csl_name, visibility = visibility, - os = select({ - "@platforms//os:linux": "linux", - "@platforms//os:macos": "macos", - "@platforms//os:windows": "windows", - "//conditions:default": "unknown", - }), - cpu = select({ - "@platforms//cpu:x86_64": "x86_64", - "@platforms//cpu:aarch64": "aarch64", - "@platforms//cpu:x86_32": "x86_32", - "//conditions:default": "unknown", - }), **kwargs ) diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index ef1193d504..aa217683e1 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -73,8 +73,6 @@ PY_EXTENSION_WRAPPER_ATTRS = COMMON_ATTRS | { providers = [CcSharedLibraryInfo], doc = "The cc_shared_library target to wrap.", ), - "os": lambda: attrb.String(doc = "OS determined by macro select."), - "cpu": lambda: attrb.String(doc = "CPU determined by macro select."), } def create_py_extension_wrapper_rule_builder(**kwargs): @@ -113,15 +111,7 @@ def _get_extension(cc_toolchain): return ext def _get_platform(ctx): - """Derives the PEP 3149 platform tag from the target constraints. - Linux platform tags are standardized here: - - https://peps.python.org/pep-3149/ - Windows platform tags, such as they are, are defined in this issue and - commit (treated as a de facto standard): - - https://github.com/python/cpython/issues/67169 - - https://github.com/python/cpython/commit/03a144bb6ac3d7631a3bdb895e2a1f2d021fb08b - Apple platform tag is always just "darwin", discussed briefly here: - - https://github.com/python/cpython/commit/3b8124884c3655b4cf2629d741b18c1a38181805 + """Derives the PEP 3149 platform tag from the active Python C++ toolchain. Args: ctx: The rule context. @@ -129,29 +119,12 @@ def _get_platform(ctx): Returns: The platform tag, e.g. "x86_64-linux-gnu" or "win_amd64" """ - os = ctx.attr.os - cpu = ctx.attr.cpu - - if os == "windows": - if cpu == "x86_64": - return "win_amd64" - if cpu == "aarch64": - return "win_arm64" - return "win32" - if os == "macos": - return "darwin" - if os == "linux": - libc = "gnu" - if ctx.attr.libc == "musl": - libc = "musl" - return '{}-{}-{}'.format(cpu, os, libc) + py_toolchain = ctx.toolchains[PY_CC_TOOLCHAIN_TYPE] + py_cc_toolchain = py_toolchain.py_cc_toolchain + if hasattr(py_cc_toolchain, "platform_tag") and py_cc_toolchain.platform_tag: + return py_cc_toolchain.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, - ), + "ERROR: Unable to resolve platform_tag from Python C++ toolchain for {self}. " + + "Please ensure the active py_cc_toolchain provides a non-empty platform_tag.", ) diff --git a/python/private/py_cc_toolchain_info.bzl b/python/private/py_cc_toolchain_info.bzl index da7938503f..982e209da3 100644 --- a/python/private/py_cc_toolchain_info.bzl +++ b/python/private/py_cc_toolchain_info.bzl @@ -96,6 +96,11 @@ If available, information about C libraries, struct with fields: considered private and should be forward along as-is (this better allows e.g. `:current_py_cc_headers` to act as the underlying headers target it represents). +""", + "platform_tag": """\ +:type: str | None + +The PEP 3149 / PEP 425 platform tag for extension modules, e.g. 'x86_64-linux-gnu', 'darwin', or 'win_amd64'. """, "python_version": """ :type: str diff --git a/python/private/py_cc_toolchain_macro.bzl b/python/private/py_cc_toolchain_macro.bzl index 416caac2ab..19a277b94b 100644 --- a/python/private/py_cc_toolchain_macro.bzl +++ b/python/private/py_cc_toolchain_macro.bzl @@ -30,4 +30,23 @@ def py_cc_toolchain(**kwargs): # This tag is added to easily identify usages through other macros. add_tag(kwargs, "@rules_python//python:py_cc_toolchain") + + if "os" not in kwargs: + kwargs["os"] = select({ + "@platforms//os:macos": "macos", + "@platforms//os:windows": "windows", + "//conditions:default": "linux", + }) + if "cpu" not in kwargs: + kwargs["cpu"] = select({ + "@platforms//cpu:aarch64": "aarch64", + "@platforms//cpu:x86_32": "x86_32", + "//conditions:default": "x86_64", + }) + if "libc" not in kwargs: + kwargs["libc"] = select({ + Label("//python/config_settings:_is_py_linux_libc_musl"): "musl", + "//conditions:default": "gnu", + }) + _py_cc_toolchain(**kwargs) diff --git a/python/private/py_cc_toolchain_rule.bzl b/python/private/py_cc_toolchain_rule.bzl index 56b2e9ccc5..32fca0b180 100644 --- a/python/private/py_cc_toolchain_rule.bzl +++ b/python/private/py_cc_toolchain_rule.bzl @@ -25,6 +25,42 @@ load(":flags.bzl", "FreeThreadedFlag") load(":py_cc_toolchain_info.bzl", "PyCcToolchainInfo") load(":sentinel.bzl", "SentinelInfo") +def _get_platform_tag(os, cpu, libc): + """ + Derives the PEP 3149 platform tag string based on target OS, CPU, and + libc. Note that these are platform tags for C extension filenames, not + PEP 425 tags for wheels. + + Linux platform tags are standardized here: + - https://peps.python.org/pep-3149/ + Windows platform tags, such as they are, are defined in this issue and + commit (treated as a de facto standard): + - https://github.com/python/cpython/issues/67169 + - https://github.com/python/cpython/commit/03a144bb6ac3d7631a3bdb895e2a1f2d021fb08b + Apple platform tag is always just "darwin", discussed briefly here: + - https://github.com/python/cpython/commit/3b8124884c3655b4cf2629d741b18c1a38181805 + + Args: + os: Target OS, e.g. "windows", "macos", "linux" + cpu: Target CPU architecture, e.g. "x86_64", "aarch64", "x86_32" + libc: Target C library variant, e.g. "gnu", "musl" + + Returns: + The platform tag, e.g. "x86_64-linux-gnu", "darwin", or "win_amd64" + """ + if os == "windows": + if cpu == "x86_64": + return "win_amd64" + if cpu == "aarch64": + return "win_arm64" + return "win32" + if os == "macos": + return "darwin" + + cpu_val = cpu if cpu else "x86_64" + libc_val = libc if libc else "gnu" + return "{}-linux-{}".format(cpu_val, libc_val) + def _py_cc_toolchain_impl(ctx): if ctx.attr.libs: libs = struct( @@ -55,8 +91,15 @@ def _py_cc_toolchain_impl(ctx): abi_flags += "t" abi_tag = "cpython-{}{}{}".format(version_parts[0], version_parts[1], abi_flags) + platform_tag = _get_platform_tag( + os = ctx.attr.os, + cpu = ctx.attr.cpu, + libc = ctx.attr.libc, + ) + py_cc_toolchain = PyCcToolchainInfo( abi_tag = abi_tag, + platform_tag = platform_tag, headers = struct( providers_map = { "CcInfo": ctx.attr.headers[CcInfo], @@ -82,6 +125,10 @@ py_cc_toolchain = rule( doc = "The ABI tag for extension modules, e.g. 'cpython-311'", default = "", ), + "cpu": attr.string( + doc = "Target CPU architecture, e.g. 'x86_64', 'aarch64', 'x86_32'", + default = "", + ), "headers": attr.label( doc = ("Target that provides the Python headers. Typically this " + "is a cc_library target."), @@ -102,11 +149,19 @@ attribute is available or not. default = "//python:none", providers = [[SentinelInfo], [CcInfo]], ), + "libc": attr.string( + doc = "Target C library variant, e.g. 'gnu', 'musl'", + default = "", + ), "libs": attr.label( doc = ("Target that provides the Python runtime libraries for linking. " + "Typically this is a cc_library target of `.so` files."), providers = [CcInfo], ), + "os": attr.string( + doc = "Target OS, e.g. 'linux', 'macos', 'windows'", + default = "", + ), "python_version": attr.string( doc = "The Major.minor Python version, e.g. 3.11", mandatory = True, From 4183634644c3efa5c9194e5f28075a19df994c11 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 14 Jul 2026 04:20:47 +0000 Subject: [PATCH 55/58] Pass the right set of additional kwargs to the internal generated targets. --- python/private/cc/py_extension_macro.bzl | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/python/private/cc/py_extension_macro.bzl b/python/private/cc/py_extension_macro.bzl index 3a5aa0d72f..73462ebdba 100644 --- a/python/private/cc/py_extension_macro.bzl +++ b/python/private/cc/py_extension_macro.bzl @@ -2,7 +2,7 @@ load("@rules_cc//cc:cc_library.bzl", "cc_library") load("@rules_cc//cc:cc_shared_library.bzl", "cc_shared_library") -load("//python/private:util.bzl", "add_tag") +load("//python/private:util.bzl", "add_tag", "copy_propagating_kwargs") load(":py_extension_rule.bzl", "py_extension_wrapper") def py_extension( @@ -53,6 +53,7 @@ def py_extension( defines = defines, deps = ["@rules_python//python/cc:current_py_cc_headers"], visibility = ["//visibility:private"], + **copy_propagating_kwargs(kwargs) ) csl_deps.append(":" + impl_lib_name) @@ -62,7 +63,7 @@ def py_extension( # 4. Create the underlying cc_shared_library csl_name = "_" + name + "_csl" - csl_kwargs = {} + csl_kwargs = copy_propagating_kwargs(kwargs) if exports_filter: csl_kwargs["exports_filter"] = exports_filter if user_link_flags: From aab3cee454ffd2d56bb21aab2876e446bb31bab6 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Tue, 14 Jul 2026 04:52:19 +0000 Subject: [PATCH 56/58] Adjust import logic to account for package names. --- python/private/cc/py_extension_macro.bzl | 10 ++++ python/private/cc/py_extension_rule.bzl | 55 +++++++++++++------ tests/cc/py_extension/BUILD.bazel | 16 ++++++ tests/cc/py_extension/ext_pkg_test.c | 22 ++++++++ .../cc/py_extension/py_extension_pkg_test.py | 17 ++++++ 5 files changed, 102 insertions(+), 18 deletions(-) create mode 100644 tests/cc/py_extension/ext_pkg_test.c create mode 100644 tests/cc/py_extension/py_extension_pkg_test.py diff --git a/python/private/cc/py_extension_macro.bzl b/python/private/cc/py_extension_macro.bzl index 73462ebdba..28027f4857 100644 --- a/python/private/cc/py_extension_macro.bzl +++ b/python/private/cc/py_extension_macro.bzl @@ -20,6 +20,16 @@ def py_extension( **kwargs): """Creates a Python extension module. + By default, extensions are created within their workspace package directory + (e.g., `pkg/ext.so`) and imported using standard Python package paths + (e.g., `from pkg import ext`). + + To customize import path behavior: + - `imports`: Pass `imports = ["..."]` to append custom search directories to + `sys.path` (matching `py_library`). + - `module_name`: Pass `module_name = "custom_name"` to override the base module + filename. + Args: name: Target name. srcs: Optional C/C++ source files to compile directly for this extension. diff --git a/python/private/cc/py_extension_rule.bzl b/python/private/cc/py_extension_rule.bzl index aa217683e1..e38f9288c3 100644 --- a/python/private/cc/py_extension_rule.bzl +++ b/python/private/cc/py_extension_rule.bzl @@ -1,8 +1,9 @@ """Implementation of the _py_extension_wrapper rule.""" +load("@bazel_skylib//lib:dicts.bzl", "dicts") load("@rules_cc//cc/common:cc_shared_library_info.bzl", "CcSharedLibraryInfo") load("//python/private:attr_builders.bzl", "attrb") -load("//python/private:attributes.bzl", "COMMON_ATTRS") +load("//python/private:attributes.bzl", "COMMON_ATTRS", "IMPORTS_ATTRS") load("//python/private:builders.bzl", "builders") load("//python/private:py_info.bzl", "PyInfo") load("//python/private:rule_builders.bzl", "ruleb") @@ -10,10 +11,6 @@ load("//python/private:toolchain_types.bzl", "PY_CC_TOOLCHAIN_TYPE") def _py_extension_wrapper_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 - if ctx.label.package: - import_path = repo_name + "/" + ctx.label.package cc_toolchain = ctx.toolchains["@bazel_tools//tools/cpp:toolchain_type"].cc ext = _get_extension(cc_toolchain) @@ -51,6 +48,24 @@ def _py_extension_wrapper_impl(ctx): runfiles_builder.add(csl_target[DefaultInfo].default_runfiles) runfiles = runfiles_builder.build(ctx) + # Resolve imports paths relative to the target package and repository: + # 1. Default (imports = []): No extra search paths are added to sys.path, + # enforcing clean package-qualified imports (e.g. `from foo.bar import ext`). + # 2. Relative paths (e.g. imports = ["."]): Resolved relative to `repo_name/package_name` + # so passing `imports = ["."]` adds the target's package directory to sys.path. + # 3. Absolute paths (starting with "/"): Stripped of leading "/" and resolved relative to runfiles root. + imports_list = [] + repo_name = ctx.label.workspace_name or ctx.workspace_name + for path in ctx.attr.imports: + if path.startswith("/"): + imports_list.append(path[1:]) + else: + pkg = ctx.label.package + full_path = "{}/{}".format(pkg, path) if pkg else path + if repo_name: + full_path = "{}/{}".format(repo_name, full_path) + imports_list.append(full_path) + return [ DefaultInfo( files = depset([py_dso]), @@ -58,22 +73,26 @@ def _py_extension_wrapper_impl(ctx): ), PyInfo( transitive_sources = depset([py_dso]), - imports = depset([import_path]), + imports = depset(imports_list), ), ] -PY_EXTENSION_WRAPPER_ATTRS = COMMON_ATTRS | { - "libc": lambda: attrb.String(default = "glibc"), - "module_name": lambda: attrb.String(), - "py_limited_api": lambda: attrb.String( - default = "", - ), - "src": lambda: attrb.Label( - mandatory = True, - providers = [CcSharedLibraryInfo], - doc = "The cc_shared_library target to wrap.", - ), -} +PY_EXTENSION_WRAPPER_ATTRS = dicts.add( + COMMON_ATTRS, + IMPORTS_ATTRS, + { + "libc": lambda: attrb.String(default = "glibc"), + "module_name": lambda: attrb.String(), + "py_limited_api": lambda: attrb.String( + default = "", + ), + "src": lambda: attrb.Label( + mandatory = True, + providers = [CcSharedLibraryInfo], + doc = "The cc_shared_library target to wrap.", + ), + }, +) def create_py_extension_wrapper_rule_builder(**kwargs): """Create a rule builder for the wrapper.""" diff --git a/tests/cc/py_extension/BUILD.bazel b/tests/cc/py_extension/BUILD.bazel index 3903d30537..5ba8ff8aca 100644 --- a/tests/cc/py_extension/BUILD.bazel +++ b/tests/cc/py_extension/BUILD.bazel @@ -51,6 +51,7 @@ py_extension( py_extension( # A python extension that dynamically links to another shared library name = "ext_shared", + imports = ["."], dynamic_deps = [ ":add_one_shared", ], @@ -152,6 +153,21 @@ py_test( ], ) +##### + +py_extension( + name = "ext_pkg_test", + srcs = ["ext_pkg_test.c"], +) + +py_test( + name = "py_extension_pkg_test", + srcs = ["py_extension_pkg_test.py"], + deps = [ + ":ext_pkg_test", + ], +) + py_extension_analysis_test_suite( name = "py_extension_analysis_tests", ) diff --git a/tests/cc/py_extension/ext_pkg_test.c b/tests/cc/py_extension/ext_pkg_test.c new file mode 100644 index 0000000000..c151f0162b --- /dev/null +++ b/tests/cc/py_extension/ext_pkg_test.c @@ -0,0 +1,22 @@ +#include + +static PyObject* get_magic_number(PyObject* self, PyObject* args) { + return PyLong_FromLong(42); +} + +static PyMethodDef ModuleMethods[] = { + {"get_magic_number", get_magic_number, METH_NOARGS, "Returns 42."}, + {NULL, NULL, 0, NULL} +}; + +static struct PyModuleDef ext_pkg_test_module = { + PyModuleDef_HEAD_INIT, + "ext_pkg_test", + NULL, + -1, + ModuleMethods +}; + +PyMODINIT_FUNC PyInit_ext_pkg_test(void) { + return PyModule_Create(&ext_pkg_test_module); +} diff --git a/tests/cc/py_extension/py_extension_pkg_test.py b/tests/cc/py_extension/py_extension_pkg_test.py new file mode 100644 index 0000000000..e9aac21dcd --- /dev/null +++ b/tests/cc/py_extension/py_extension_pkg_test.py @@ -0,0 +1,17 @@ +import unittest + +from tests.cc.py_extension import ext_pkg_test + + +class PyExtensionPkgTest(unittest.TestCase): + + def test_import_via_package(self): + self.assertEqual(ext_pkg_test.get_magic_number(), 42) + + def test_direct_import(self): + with self.assertRaises(ModuleNotFoundError): + import ext_pkg_test # buildifier: disable=g-import-not-at-top # noqa: F401 + + +if __name__ == "__main__": + unittest.main() From 652a50a9724ad2f762a3ac958927513ae0afad84 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Thu, 16 Jul 2026 04:41:57 +0000 Subject: [PATCH 57/58] Add some C-specific linking attributes to pass to the internal generated targets. --- python/private/cc/py_extension_macro.bzl | 29 ++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/python/private/cc/py_extension_macro.bzl b/python/private/cc/py_extension_macro.bzl index 28027f4857..b3307cdada 100644 --- a/python/private/cc/py_extension_macro.bzl +++ b/python/private/cc/py_extension_macro.bzl @@ -11,6 +11,10 @@ def py_extension( hdrs = None, copts = None, defines = None, + includes = None, + linkopts = None, + linkshared = None, + linkstatic = None, deps = None, dynamic_deps = None, exports_filter = None, @@ -36,6 +40,10 @@ def py_extension( hdrs: Optional header files for the srcs. copts: Optional compiler flags for srcs. defines: Optional preprocessor defines for srcs. + includes: Optional header include search paths passed to internal cc_library. + linkopts: Optional link options passed to internal cc_library and cc_shared_library. + linkshared: Deprecated and ignored. Extensions are always linked dynamically. + linkstatic: Optional linkstatic flag passed to internal cc_library. deps: cc_library targets to statically link into the extension. dynamic_deps: cc_shared_library targets to dynamically link. exports_filter: Filter for exported symbols passed to cc_shared_library. @@ -45,6 +53,7 @@ def py_extension( **kwargs: Additional arguments passed to the underlying wrapper rule. """ add_tag(kwargs, "@rules_python//python/cc:py_extension") + _ = linkshared # buildifier: disable=unused-variable csl_deps = [] @@ -55,6 +64,13 @@ def py_extension( # 2. If srcs or hdrs are specified, create an implicit cc_library for them if srcs or hdrs: impl_lib_name = "_" + name + "_impl" + impl_lib_kwargs = copy_propagating_kwargs(kwargs) + if includes: + impl_lib_kwargs["includes"] = includes + if linkopts: + impl_lib_kwargs["linkopts"] = linkopts + if linkstatic != None: + impl_lib_kwargs["linkstatic"] = linkstatic cc_library( name = impl_lib_name, srcs = srcs, @@ -63,7 +79,7 @@ def py_extension( defines = defines, deps = ["@rules_python//python/cc:current_py_cc_headers"], visibility = ["//visibility:private"], - **copy_propagating_kwargs(kwargs) + **impl_lib_kwargs ) csl_deps.append(":" + impl_lib_name) @@ -76,8 +92,9 @@ def py_extension( csl_kwargs = copy_propagating_kwargs(kwargs) if exports_filter: csl_kwargs["exports_filter"] = exports_filter - if user_link_flags: - csl_kwargs["user_link_flags"] = user_link_flags + effective_user_link_flags = user_link_flags or linkopts + if effective_user_link_flags: + csl_kwargs["user_link_flags"] = effective_user_link_flags cc_shared_library( name = csl_name, @@ -98,7 +115,11 @@ def py_extension( if data != None: kwargs["data"] = data - # 6. Wrap with py_extension_wrapper for PEP 3149 naming & PyInfo + # 6. Filter out C++ specific compilation/linking attributes before invoking wrapper rule + for cc_attr in ("includes", "linkopts", "linkshared", "linkstatic", "features"): + kwargs.pop(cc_attr, None) + + # 7. Wrap with py_extension_wrapper for PEP 3149 naming & PyInfo py_extension_wrapper( name = name, src = ":" + csl_name, From f5cbb95c34512dac1920ed35d855b2c655b0a431 Mon Sep 17 00:00:00 2001 From: Richard Sartor Date: Thu, 16 Jul 2026 04:54:04 +0000 Subject: [PATCH 58/58] Adjust dep handling so we can include headers correctly. --- python/private/cc/py_extension_macro.bzl | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/python/private/cc/py_extension_macro.bzl b/python/private/cc/py_extension_macro.bzl index b3307cdada..afb690e88a 100644 --- a/python/private/cc/py_extension_macro.bzl +++ b/python/private/cc/py_extension_macro.bzl @@ -57,11 +57,7 @@ def py_extension( csl_deps = [] - # 1. Handle user-supplied static deps - if deps: - csl_deps.extend(deps) - - # 2. If srcs or hdrs are specified, create an implicit cc_library for them + # 1. If srcs or hdrs are specified, create an implicit cc_library for them if srcs or hdrs: impl_lib_name = "_" + name + "_impl" impl_lib_kwargs = copy_propagating_kwargs(kwargs) @@ -77,13 +73,15 @@ def py_extension( hdrs = hdrs, copts = (copts or []) + ["-fPIC"], defines = defines, - deps = ["@rules_python//python/cc:current_py_cc_headers"], + deps = (deps or []) + ["@rules_python//python/cc:current_py_cc_headers"], visibility = ["//visibility:private"], **impl_lib_kwargs ) csl_deps.append(":" + impl_lib_name) + elif deps: + csl_deps.extend(deps) - # 3. If no static deps or sources were specified, use empty target for CSL requirement + # 2. If no static deps or sources were specified, use empty target for CSL requirement if not csl_deps: csl_deps.append("//python/private/cc:empty")