Skip to content

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

Open
rsartor-cmd wants to merge 27 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 27 commits into
bazel-contrib:py-extensionfrom
rsartor-cmd:py-extension

Conversation

@rsartor-cmd

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.

Key Changes

1. Introduced abi_tag to PyCcToolchainInfo

• Modified py_cc_toolchain_info.bzl to add the abi_tag field to the PyCcToolchainInfo provider.
• Modified py_cc_toolchain_rule.bzl to add an optional abi_tag attribute to the py_cc_toolchain rule.
• If abi_tag is not explicitly specified, the rule automatically derives a default tag (e.g., cpython-311 from python_version = "3.11" ), ensuring 100% backward compatibility with existing toolchains.

2. Refactored py_extension Toolchain Dependency

• Modified py_extension_rule.bzl to switch the rule's toolchain dependency from TARGET_TOOLCHAIN_TYPE to PY_CC_TOOLCHAIN_TYPE .
• Updated _py_extension_impl to retrieve the ABI tag from py_cc_toolchain.abi_tag instead of py_runtime.pyc_tag and py_runtime.abi_flags .
• Simplified the output filename formatting and fixed a bug to use the dynamic extension (e.g., .pyd on Windows) instead of hardcoding .so .

3. Enforced PLATFORMS Registry

• Removed the legacy fallback path in _get_platform that parsed cc_toolchain CPU and target names.
• The rule now strictly requires the target platform to be registered in rules_python 's central PLATFORMS registry, failing the build with a clear error message if it is not found. This aligns the rule
with the modern platform constraints model and cleans up legacy parsing code.

4. Updated and Added Tests

• Updated py_extension_test.py to reflect the correct platform/version-specific extension filename.
• Implemented a new test suite in py_limited_api_tests.bzl to verify that the py_limited_api attribute correctly configures the .abi3.so filename suffix across various dependency configurations.
• Note on Validation: An initial design for compile-time dependency compatibility validation was discarded due to performance concerns regarding depset flattening at analysis time.
──────

Verification

Ran the entire test suite using bazel-8.7.0 :

• 9/9 Limited API Analysis Tests: PASSED
• 2/2 Pre-existing Analysis Tests: PASSED
• Python Integration Unit Test: PASSED
──────
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.

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.

1 participant