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

Support for Python Rules that are Code Generators #6726

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

andponlin-canva
Copy link
Contributor

@andponlin-canva andponlin-canva commented Sep 4, 2024

Checklist

  • I have filed an issue about this change and discussed potential changes with the maintainers.
  • I have received the approval from the maintainers to make this change.
  • This is not a stylistic, refactoring, or cleanup change.

Please note that the maintainers will not be reviewing this change until all checkboxes are ticked. See
the Contributions section in the README for more
details.

Discussion thread for this change

Issue number: #6725

Description of this change

Discovery and handling of code-generator Rules

The change introduces the ability for a code-generator Rule being able to carry a Tag intellij-py-code-generator. The presence of this tags signals...

  1. to the aspect logic that it should treat the outputs of the Rule as sources and also to consider the output imports provided on the PyInfo Provider as the imports for the Rule. These output imports need to be mapped so that they look like input imports such as would be configured with the likes of the py_library Rule.
  2. to the base Plugin logic signalling that the Rule should be further processed for sources.
  3. to the base Plugin logic signalling that the Rule is of py_library Kind as it will feign being a py_library.

In this way, the Plugin will deal with the output of the Rule as sources + imports rather than the input Rule attributes of srcs and imports.

After the logic in SyncProjectTargetsHelper issues a bazel query ... to find Targets, it will issue a second bazel query ... to search for the Tag intellij-py-code-generator. The mechanism has been implemented in such a way that other languages could potentially use this Tag-based mechanism in the future so the review should consider the generality of the solution. It would be possible to issue a single cquery (with Starlark code) to perform both queries in one go, but such a cquery would be in consideration of build options which is probably not what is required in this case.

I have also considered the option of querying for the PyInfo provider as a signal that a Rule were a code-generator for a Python project. I rejected this approach so far because...

  • there may be Rules which don't want to be recognized as a code-generator even though they emit a PyInfo.
  • the Tag approach with intellij-py-code-generator is opt-in so is less risky.
  • for other languages, a Tag approach will be an option whereas there may not be a specific Provider to work from.

A second change, this time in AbstractPyImportResolverStrategy, will alter the logic of collecting Python sources such that it is able to cope with processing a directory because this scenario is likely to come up with code generators. The source directory is walked looking for .py files. If it encounters a "boundry file" such as WORKSPACE or BUILD.bazel then it will stop traversing the directories. In reality this won't happen with code-generators but it is implemented to ensure that the directory processing will work more generally.

The pull request also carries some light documentation and a working example to demonstrate the feature.

It is possible to write Bazel rules that generate Python code and
act as a `py_library`. The plugin is augmented with this change to
have a means of detecting these sort of rules and be able to work
with them.
@andponlin-canva
Copy link
Contributor Author

Please hold off on review as I am investigating a change in the way the logic will detect the code-generator scenario.

@andponlin-canva
Copy link
Contributor Author

I have done some investigation to try to avoid the use of the intellij-py-code-generator tag. I have experimented with adding an OutputGroupInfo Provider to the code-generator Rules which carries a py_generated_srcs depset. For example;

OutputGroupInfo(
    py_generated_srcs = depset([output_directory]),
),

It is possible to adjust the Python-related Aspect logic in the Plugin to derive the generated Python source from target[OutputGroupInfo].py_generated_srcs instead of from target[DefaultInfo].default_runfiles.

The problem is filtering the code-gen Targets to run with the Plugin's Aspect. The Plugin primarily does this filtering by running a Bazel query to find the Targets and Kinds and then filters on the Kind; py_binary, py_library, java_library, go_binary etc.... It has no means of knowing that some bespoke Rule with a Kind such as some_simple_py_generator provides the OutputGroupInfo with attribute py_generated_srcs.

Whilst it is possible to run a second Bazel query to be able to identify Targets that carry the intellij-py-code-generator tag, a similar query (rather than cquery or aquery) seems to be not possible for finding the OutputGroupInfo with attribute py_generated_srcs so that those Targets can be included. I assume a cquery or aquery would be unwanted in the Plugin because they would be governed by build settings.

The same problem applies for adding an additional attribute to PyInfo.

From this investigation, given the constraints of the technology, it seems that the solution of identifying the Python code-generators using an intellij-py-code-generator tag remains the best path forward.

Aside from Python... whilst I can see that the JavaInfo Provider has java_outputs to presumably convey generated sources, should a code-generator Rule exist for producing Java sources that were not one of the ones in JavaBlazeRules.java then I am guessing that the Plugin is going to ignore the Rule and that a mechanism such as this with a simiar tag intellij-java-code-generator might be likewise useful in the future and that this implies the code added to LanguageClass also makes sense in this PR.

@tpasternak
Copy link
Collaborator

@ilanKeshet would it be useful in your case?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review Awaiting review from Bazel team on PRs product: CLion CLion plugin product: GoLand GoLand plugin product: IntelliJ IntelliJ plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants