Repository navigation
fix: do not include transitive sources from pyi_deps in runtime runfiles - #4178
Conversation
…runfiles `pyi_deps` are documented as build-time only dependencies for type checking and static analysis. Previously, `create_py_info` merged `pyi_deps` targets using `py_info.merge()`, which caused all transitive sources from type stubs and their dependencies to be placed in `PyInfo.transitive_sources` and included in downstream binary/test runfiles. This change updates `create_py_info` to only propagate `imports`, `transitive_pyi_files`, and `transitive_original_sources` from `pyi_deps`, preventing them from leaking into runtime runfiles. Adds unit tests verifying `transitive_sources` and binary runfiles exclusion.
faximan
changed the title
pyi_deps in runtime runfilespyi_deps in runtime runfiles
Unreleased changes must be recorded as news fragment files under news/ rather than editing CHANGELOG.md directly. Revert the direct edit to CHANGELOG.md and add news/4178.fixed.md.
Encapsulate build-time PyInfo merging into PyInfoBuilder.merge_build_time() so create_py_info does not inspect individual PyInfo fields for pyi_deps. Only pyi_files are merged; runtime fields (imports, transitive_sources) and transitive_original_sources are excluded.
A type-checking-only dependency in pyi_deps can include plain .py files or imports that are needed by static type checkers, but should not be included in runtime outputs. Add PyInfo.type_checking_info and PyInfoBuilder.merge_type_checking() to store and propagate type-checking-only PyInfo separately from runtime PyInfo fields.
|
@rickeylev thanks so much for fixing up the PR. Let me know if I can help with any of the code changes or if we can move forward with the review. |
|
I personally think having a config flag gate the behaviour would be better. For debug/analysis rules we may want to propagate the pyi files, but fer fully stripped production artifacts maybe we don't need them. Consider introducing a config flag that would acces "auto" "yes" or "no" values. |
…e pruned from runfiles in opt mode.
This is not something I have a strong opinion on, so I added the flag, with the default 'auto' meaning prune iff PTAL! |
| * `auto`: (default) Automatically decide the effective value based on the | ||
| compilation mode. In `opt` and standard builds, `pyi_deps` are pruned from | ||
| runtime runfiles. |
There was a problem hiding this comment.
This is really nice, thank you.
FYI, @rickeylev, we could do the same in the #4192.
There was a problem hiding this comment.
Yeah I had a similar thought. If we breakup the files in the runtime into some more precise groups, then we can selectively include things. We already partially do that for some venv reasons.
|
PTAL - also fixed a failing test with 69a7e04. |
| * `auto`: (default) Automatically decide the effective value based on the | ||
| compilation mode. In `opt` and standard builds, `pyi_deps` are pruned from | ||
| runtime runfiles. |
There was a problem hiding this comment.
Yeah I had a similar thought. If we breakup the files in the runtime into some more precise groups, then we can selectively include things. We already partially do that for some venv reasons.
3 participants
pyi_depsare documented as build-time only dependencies for type checking and static analysis, but currently leaks into the runfiles.Before:
create_py_infomergedpyi_depstargets usingpy_info.merge(), which caused all transitive sources from type stubs and their dependencies to be placed inPyInfo.transitive_sourcesand included in downstream binary/test runfiles.After:
create_py_infoonly propagatesimports(for module import resolution during type checking),transitive_pyi_files, andtransitive_original_sourcesfrompyi_deps. Adds unit tests verifyingtransitive_sourcesand binary runfiles exclusion.Tested: in our repo using
rules_pythonandpyi_depsfor type checking, this reduced the runfile tree size with 20M for a samplepy_test.