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 32 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>
…nfirm py_extension behaves similarly to cc_shared_library.
…ry and cc_shared_library targets.
| ) | ||
|
|
||
| def _csl_dynamic_deps_test_impl(env, target): | ||
| env.expect.that_target(target).has_provider(CcSharedLibraryInfo) |
There was a problem hiding this comment.
This test seems a little light. Don't we need to check more to ensure it actually linked everything correctly? Same for the others
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_extensionimplementation. It re-works the platform tag detection to rely on modern platform constraints, and refactors the compilation and linking mechanismto delegate to Bazel's native
cc_shared_libraryandcc_libraryrules.This PR targets the main repository's
py-extensionbranch (notmain).Key Changes
1. Platform Tag & ABI Tag Derivation
abi_tagtoPyCcToolchainInfo: Added theabi_tagfield to thePyCcToolchainInfoprovider, populated by thepy_cc_toolchainrule. It defaults to deriving the tag frompython_version(e.g.cpython-311) for backward compatibility.cc_toolchainCPU names with direct lookup in rules_python's centralPLATFORMSregistry using platform constraints..abi3.so) suffixes are appended based on the toolchain configuration.2. Compilation & Linking Delegation (Refactor to
cc_shared_library)py_extensionrule 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_libraryunder the hood.deps(removing the redundantstatic_depsattribute) and aligned linker arguments withcc_shared_library'suser_link_flags._py_extension_wrapper): Added a lightweight, private rule that wraps thecc_shared_libraryoutput to:.cpython-311-x86_64-linux-gnu.soor.abi3.so).symlinkfrom the CSL output to the PEP 3149 name.PyInfo(for python rules) andCcSharedLibraryInfo(for dynamic C++ dependencies).cc_shared_libraryensures strict analysis-time validation against One Definition Rule (ODR) violations (e.g., duplicate static linkage in dynamic chains).3. Runfiles and Data Support
builders.RunfilesBuilderto support runtime assets (dataattribute) and dynamic library dependencypropagation.
Verification & Testing
dependency_graph_tests.bzlto verify dynamic dependency chains and static sharing behavior.py_extension_tests.bzlto verify data asset propagation.//tests/cc/py_extension/...are compiling and passing.──────
TAG=agy
CONV=23f8a5e8-2d99-401a-9903-1256b7d42b0e