feat: updates to the py_extension rule for building C extensionsPy extension#3875
feat: updates to the py_extension rule for building C extensionsPy extension#3875rsartor-cmd wants to merge 27 commits into
Conversation
…tions and may produce false-positives.
There was a problem hiding this comment.
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.
|
The mypy CI failure appears to be caused by the check action using Python 3.9, but the newest mypy only supporting 3.10+ For what it's worth, I ran |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
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