Skip to content

Match private extension modules against the in-package path only - #2504

Open
LeSingh1 wants to merge 1 commit into
NVIDIA:mainfrom
LeSingh1:fix/cython-abi-private-module-filter
Open

Match private extension modules against the in-package path only#2504
LeSingh1 wants to merge 1 commit into
NVIDIA:mainfrom
LeSingh1:fix/cython-abi-private-module-filter

Conversation

@LeSingh1

@LeSingh1 LeSingh1 commented Aug 4, 2026

Copy link
Copy Markdown

Two related defects in toolshed/check_cython_abi.py's private-module filter.

1. The filter matched the absolute path. generate skipped a module when any(x.startswith("_") for x in so_path.parts), and so_path is absolute — so any ancestor directory beginning with an underscore made every module look private. That is the normal layout under manylinux (/opt/_internal/cpython-*/) and in GitHub Actions containers (/__w/):

>>> build_dir = Path("/opt/_internal/cpython-3.12/lib/python3.12/site-packages/cuda/bindings")
>>> so = build_dir / "driver.cpython-312-x86_64-linux-gnu.so"   # public
>>> any(p.startswith("_") for p in so.parts)
True

generate then writes zero .abi.json files and exits 0 — a green run with no coverage at all.

2. check had no filter. generate deliberately skips private modules as "not part of the public ABI", but check's new-module scan globs everything. Since generate never wrote an .abi.json for them, check reports every private module as New module added on every run and sets has_allowed_changes, so it can never print "No changes found" for a package that ships private submodules — cuda.bindings has _bindings/, _internal/ and _lib/.

The fix extracts the predicate into iter_public_extension_modules(), calls it from both paths, and matches only on the path relative to the package root. Public driver.so under /opt/_internal/... is now kept; _internal/utils.so is still skipped.

Verified by reproducing the path defect directly, as above. I deliberately did not add a unit test: toolshed/README.md says these tools "do not warrant CI coverage, unit tests, or the rest of the production-code apparatus", and pytest.ini's testpaths does not include toolshed/, so a test there would never run. Happy to add one if you would rather have it.

Related but not overlapping: #1030 tracks wiring this tool into CI. This fix would be a prerequisite for that to mean anything.

NOTE: developed with the assistance of an AI coding agent. I reviewed and verified the change before submitting.

check_cython_abi's private-module filter tested `so_path.parts` on the
absolute path, so any ancestor directory starting with an underscore made
every module look private. That is the normal layout under manylinux
(/opt/_internal/cpython-*/) and in GitHub Actions containers (/__w/), where
`generate` then writes zero ABI files and exits 0 -- a green run with no
coverage at all.

`check`'s new-module scan had no filter, while `generate` skipped private
modules. Since `generate` never wrote an .abi.json for them, `check` reported
every private module as "New module added" on every run and set
has_allowed_changes, so it could not print "No changes found" for a package
shipping private submodules (cuda.bindings has _bindings/, _internal/, _lib/).

Extract the predicate into iter_public_extension_modules() so both paths use
it, and match only on the path relative to the package root.
@copy-pr-bot

copy-pr-bot Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

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