Skip to content

feat: updates to the py_extension rule for building C extensionsPy extension#3875

Open
rsartor-cmd wants to merge 32 commits into
bazel-contrib:py-extensionfrom
rsartor-cmd:py-extension
Open

feat: updates to the py_extension rule for building C extensionsPy extension#3875
rsartor-cmd wants to merge 32 commits into
bazel-contrib:py-extensionfrom
rsartor-cmd:py-extension

Conversation

@rsartor-cmd

@rsartor-cmd rsartor-cmd commented Jun 30, 2026

Copy link
Copy Markdown

Per feedback from #3851, this PR re-works the platform detection logic to rely on platform constraints instead of the Python runtime/toolchain.

Description

This PR contains updates to the experimental py_extension implementation. It re-works the platform tag detection to rely on modern platform constraints, and refactors the compilation and linking mechanism
to delegate to Bazel's native cc_shared_library and cc_library rules.

This PR targets the main repository's py-extension branch (not main).

Key Changes

1. Platform Tag & ABI Tag Derivation

  • Introduced abi_tag to PyCcToolchainInfo: Added the abi_tag field to the PyCcToolchainInfo provider, populated by the py_cc_toolchain rule. It defaults to deriving the tag from
    python_version (e.g. cpython-311) for backward compatibility.
  • Constraint-Based Detection: Replaced legacy parsing of cc_toolchain CPU names with direct lookup in rules_python's central PLATFORMS registry using platform constraints.
  • Limited API Configuration: Updated how limited API (.abi3.so) suffixes are appended based on the toolchain configuration.

2. Compilation & Linking Delegation (Refactor to cc_shared_library)

  • Macro Fusion: Replaced the custom linking logic in the py_extension rule with a macro of the same name. It now accepts C/C++ source/header files directly (srcs, hdrs, copts, defines),
    implicitly wrapping them in a private cc_library under the hood.
  • Consolidated Attributes: Consolidated static linkage under standard deps (removing the redundant static_deps attribute) and aligned linker arguments with cc_shared_library's user_link_flags.
  • Wrapper Rule (_py_extension_wrapper): Added a lightweight, private rule that wraps the cc_shared_library output to:
  • Query the Python toolchain for platform/ABI tags to resolve PEP 3149 compliant filenames (e.g. .cpython-311-x86_64-linux-gnu.so or .abi3.so).
  • Create a cheap symlink from the CSL output to the PEP 3149 name.
  • Propagate PyInfo (for python rules) and CcSharedLibraryInfo (for dynamic C++ dependencies).
  • ODR Validation: Leveraging native cc_shared_library ensures strict analysis-time validation against One Definition Rule (ODR) violations (e.g., duplicate static linkage in dynamic chains).

3. Runfiles and Data Support

  • Runfiles Propagation: Re-implemented runfiles collection using the repository's standard builders.RunfilesBuilder to support runtime assets (data attribute) and dynamic library dependency
    propagation.

Verification & Testing

  • Added parity test cases in dependency_graph_tests.bzl to verify dynamic dependency chains and static sharing behavior.
  • Added runfiles validation in py_extension_tests.bzl to verify data asset propagation.
  • Updated limited API tests to match CSL output structure.
  • All 14 tests in //tests/cc/py_extension/... are compiling and passing.
    ──────
    TAG=agy
    CONV=23f8a5e8-2d99-401a-9903-1256b7d42b0e

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the py_extension rule to use constraint-based platform tag derivation instead of C++ toolchain-based derivation, introduces abi_tag to PyCcToolchainInfo, and removes the _check_limited_api_compatibility validation logic. The review feedback highlights several critical issues: first, the new constraint-based platform detection is broken for musl Linux platforms because they share the same constraints as gnu platforms; second, changing the limited API check to bool(ctx.attr.py_limited_api) breaks backward compatibility for targets explicitly setting "none"; third, the hardcoded _constraints list should be dynamically derived from PLATFORMS to prevent future maintenance issues; and finally, the documentation for py_limited_api needs to be updated since the dependency validation logic was removed.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread python/private/cc/py_extension_rule.bzl
Comment thread python/private/cc/py_extension_rule.bzl
Comment thread python/private/cc/py_extension_rule.bzl
Comment thread python/private/cc/py_extension_rule.bzl Outdated
@rsartor-cmd

rsartor-cmd commented Jun 30, 2026

Copy link
Copy Markdown
Author

The mypy CI failure appears to be caused by the check action using Python 3.9, but the newest mypy only supporting 3.10+

Run docker build /home/runner/work/_actions/jpetrucciani/mypy-check/master --build-arg "pyver=3.9" -t mypy
#11 3.773 ERROR: Ignored the following versions that require a different python version: 1.20.0 Requires-Python >=3.10; 1.20.1 Requires-Python >=3.10; 1.20.2 Requires-Python >=3.10; 2.0.0 Requires-Python >=3.10; 2.1.0 Requires-Python >=3.10
#11 3.774 ERROR: Could not find a version that satisfies the requirement mypy==2.1.0 (from versions: 0.1, 0.11, 0.12, 0.13, 0.14, 0.15, 0.16, 0.17, 0.18, 0.19, 0.20, 0.21, 0.221, 0.222, 0.223, 0.224, 0.225, 0.226, 0.227, 0.228, 0.229, 0.230, 0.231, 0.232, 0.233, 0.234, 0.235, 0.236, 0.237, 0.238, 0.239, 0.240, 0.241, 0.250, 0.251, 0.252, 0.253, 0.254, 0.255, 0.256, 0.470, 0.471, 0.501, 0.510, 0.511, 0.520, 0.521, 0.530, 0.540, 0.550, 0.560, 0.570, 0.580, 0.590, 0.600, 0.610, 0.620, 0.630, 0.641, 0.650, 0.660, 0.670, 0.700, 0.701, 0.710, 0.711, 0.720, 0.730, 0.740, 0.750, 0.760, 0.761, 0.770, 0.780, 0.781, 0.782, 0.790, 0.800, 0.812, 0.900, 0.901, 0.902, 0.910, 0.920, 0.921, 0.930, 0.931, 0.940, 0.941, 0.942, 0.950, 0.960, 0.961, 0.971, 0.981, 0.982, 0.990, 0.991, 1.0.0, 1.0.1, 1.1.1, 1.2.0, 1.3.0, 1.4.0, 1.4.1, 1.5.0, 1.5.1, 1.6.0, 1.6.1, 1.7.0, 1.7.1, 1.8.0, 1.9.0, 1.10.0, 1.10.1, 1.11.0, 1.11.1, 1.11.2, 1.12.0, 1.12.1, 1.13.0, 1.14.0, 1.14.1, 1.15.0, 1.16.0, 1.16.1, 1.17.0, 1.17.1, 1.18.1, 1.18.2, 1.19.0, 1.19.1)
#11 3.775 ERROR: No matching distribution found for mypy==2.1.0
#11 ERROR: process "/bin/sh -c pip install -U pip &&     pip install -r requirements.txt" did not complete successfully: exit code: 1

For what it's worth, I ran mypy --show-column-numbers --hide-error-context python/runfiles and mypy --show-column-numbers --hide-error-context tests/runfiles locally and it found no issues with 3 files.

)

def _csl_dynamic_deps_test_impl(env, target):
env.expect.that_target(target).has_provider(CcSharedLibraryInfo)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test seems a little light. Don't we need to check more to ensure it actually linked everything correctly? Same for the others

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants