feat(py): propagate type stubs and carry them into venvs - #1538
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Aspect Workflows Tasks📅 Thu Sep 24 01:39:04 UTC 2026 Task Results
⏱ Last updated Thu Sep 24 01:43:16 UTC 2026 · 📊 GitHub API quota 473/7,700 (6% used, resets in 55m) |
py_binary startup benchmark
sys.path quality
Bazel analysis benchmark
py_image_layer benchmark
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3b746b3f4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| runfiles = venv.runtime_runfiles.merge(ctx.runfiles(files = venv_only)) | ||
| if include_sources: | ||
| runfiles = runfiles.merge(ctx.runfiles(transitive_files = venv.transitive_sources)) | ||
| runfiles = runfiles.merge(ctx.runfiles(transitive_files = depset( |
There was a problem hiding this comment.
I don't think PYI should be in the runfiles at all...
| # A resolution target's default outputs stand in for its sources (a wheel | ||
| # install tree has no PyInfo of its own). Stubs among them belong to the | ||
| # pyi depset collected alongside, not to the runtime sources. | ||
| direct = _runtime_sources(target[DefaultInfo].files.to_list()) |
| fields = { | ||
| "transitive_sources": "depset[File] — postorder depset of first-party `.py` sources in the transitive closure.", | ||
| "transitive_sources": "depset[File] — postorder depset of first-party runtime sources in the transitive closure; `.pyi` stubs are excluded.", | ||
| "transitive_pyi_files": "depset[File] — postorder depset of `.pyi` type stubs in the transitive closure: stubs listed in `srcs` plus those carried by deps of either ruleset.", |
There was a problem hiding this comment.
I think we can drop the "stubs listed in srcs plus those carried by deps of either ruleset." - that is an implementation detail of rules, and I don't think we should mention "other rulesets" in these docs
…to venvs Builds on the transitive_pyi_files plumbing from #1523 and makes the field mean the same thing on both sides of the ruleset boundary. - `.pyi` files listed in `srcs` are classified by extension into `PyInfo.transitive_pyi_files`, mirroring rules_python where stubs never count as runtime sources. `transitive_sources` no longer carries them, including through virtual-dependency resolutions. - Venvs that include sources and launchers put both depsets in runfiles, so stubs that only exist in a foreign provider (py_proto_library, a rules_python `pyi_srcs`) land beside the modules they annotate. - Under the migration flag the emitted rules_python PyInfo also populates `direct_pyi_files`. - Drops the redundant resolution wrapper and `has_py_info` guards; the interop accessor already returns an empty depset for foreign-less targets. Tests: analysis coverage in //py/tests/type-stubs for first-party, foreign, virtual-resolution, launcher, venv and compatibility-provider shapes, plus a runtime check that stubs sit in runfiles. e2e: a rules_python `pyi_srcs` library consumed by rules_py, protobuf's py_proto_library `_pb2.pyi` propagating and landing in runfiles, and rules_python's PyInfo builder merging a rules_py stub under the compatibility layer.
e3b746b to
9e29cf9
Compare
1817801 to
42f05e0
Compare
Builds on #1523 (its commit is kept as the first commit here, authorship intact) and makes
transitive_pyi_filesmean the same thing on both sides of the ruleset boundary.Closes #558.
Changes
.pyifiles listed insrcsare classified by extension intoPyInfo.transitive_pyi_files, mirroring rules_python where stubs never count as runtime sources.transitive_sourcesno longer carries them, including through virtual-dependency resolutions. No newpyi_srcsattribute.py_proto_library_pb2.pyi, a rules_pythonpyi_srcs) propagate through rules_py'sPyInfo. Stubs never enter runfiles.--@aspect_rules_py//py:emit_rules_python_providersthe emitted rules_pythonPyInfoalso populatesdirect_pyi_files.PyInfocontributes only itstransitive_sources; itsDefaultInfo.filesare used as sources only for targets withoutPyInfo.Changes are visible to end-users: yes
PyInfo.transitive_sourcesno longer lists.pyifiles fromsrcs; they move toPyInfo.transitive_pyi_files. Resolution targets withPyInfono longer add theirDefaultInfo.filestotransitive_sources.Test plan
//py/tests/type-stubs: analysis coverage for first-party, foreign, virtual-resolution, launcher, venv and compatibility-provider shapes, plus apy_testasserting stubs stay out of runfiles at execution time.e2e/rules-python-interop: a rules_python library withpyi_srcsconsumed by a rules_pypy_library.e2e/rules-python-protobuf: protobuf'spy_proto_librarygeneratingnative_greeting_pb2.pyi, propagated through rules_py'sPyInfo.e2e/rules-python-provider-compat: a rules_py stub surviving the merge into rules_python'sPyInfo.