Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add pyi attributes/fields, original source fields #2538

Merged
merged 5 commits into from
Dec 31, 2024

Conversation

rickeylev
Copy link
Collaborator

@rickeylev rickeylev commented Dec 27, 2024

This adds attributes and fields of use to static analysis.

For type definition files (usually .pyi files), the pyi_srcs and pyi_deps
fields are added to the rules. They end up in the PyInfo fields direct_pyi_files
and transitive_pyi_files.

So that static analysis tools can retain access to a target's Python source files,
even if precompiling is enabled, direct_original_sources and
transitive_original_sources fields are added to PyInfo.

Work towards #2537, #296

@rickeylev rickeylev force-pushed the feat.orig.sources branch 4 times, most recently from 150941c to 4a4475a Compare December 27, 2024 07:07
@rickeylev rickeylev marked this pull request as ready for review December 27, 2024 07:22
@rickeylev rickeylev requested a review from aignas as a code owner December 27, 2024 07:22
@rickeylev rickeylev changed the title feat: add PyInfo fields for storing original sources of a target feat: add PyInfo fields for original sources and pyi of a target Dec 29, 2024
@rickeylev rickeylev changed the title feat: add PyInfo fields for original sources and pyi of a target feat: add pyi attributes/fields, original source fields Dec 29, 2024
Copy link
Collaborator

@aignas aignas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Gazelle support can come in a later PR.

FYI @ewianda, @dougthor42.


These are dependencies that satisfy imports guarded by `typing.TYPE_CHECKING`.
These are build-time only dependencies and not included as part of a runnable
program (packaging rules may include them, however).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gazelle has a feature that populates the deps or data field for the pyi packages that one may have in the requirement file. I am not sure how big of a lift it is to accommodate for that but it raises some questions.

  1. What happens if pyi sources propagate through deps vs pyi_deps.
  2. Do I understand correctly that the pyi_deps will not be fetched if none of the actions consumes them?

@aignas aignas added this pull request to the merge queue Dec 31, 2024
Merged via the queue into bazelbuild:main with commit c8346f9 Dec 31, 2024
4 checks passed
@rickeylev
Copy link
Collaborator Author

  1. What happens if pyi sources propagate through deps vs pyi_deps.

The deps attribute will try to include things into the final output for runtime. The pyi_deps attribute won't.

Given a target with only pyi files (i.e it only has things for build time, nothing for runtime), it doesn't matter much if something is put into deps or pyi_deps.

Given a target with e.g. both py and pyi files (i.e it has things for build time and things for runtime), putting it in pyi_deps means none of it will end up in the final output. This is useful if a source file has something guarded by typing.TYPE_CHECKING (the type checker needs the additional dependencies, but the runtime doesn't). Putting it in the deps attribute means the py files will be included in the final output (like normal), and the pyi files won't.

  1. Do I understand correctly that the pyi_deps will not be fetched if none of the actions consumes them?

It'll be fetched, but it won't cause any actions to actually run. The analysis phase will need to resolve the target, but (assuming there is no type checker using the pyi files) none of the files/actions will be executed.

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.

2 participants