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

Supply code-gen rule names via settings #6889

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

Conversation

andponlin-canva
Copy link
Contributor

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

Previous PRs (here and here) have been merged. This works by specifying the code-generator Bazel Rules by using a special tag and running an additional query to find the Targets with tagged rules.

This PR changes the mechanism substantially so that instead of using a tag, the list of rule names that are Python code-generator are supplied in the .bazelproject file like this;

python_code_generator_rule_names:
  test_codegen_files_py
  test_codegen_directory_py

This approach is better because;

  • The user only needs to specify the list of rules names in the settings and not mark each instance of the Bazel targets with the tag and specify the flag setting in the .bazelproject to enable the functionality.
  • There is no longer an additional query to search for the tag.

The list of code-generator rules' names is supplied to the Bazel aspect using an aspect parameter. The aspect logic signals that it has found a code-generator using an additional boolean flag on the aspect outputs (see change to the proto/intellij_ide_info.proto). The plugin logic then picks up on this flag being set. Also the logic in SyncProjectTargetsHelper is adjusted to deal with the code-generator rules' names.

The example at examples/python/simple_code_generator/... has been adjusted so that it better demonstrates how the code-generator system works within a Python Bazel Project. The previous layout of the Targets in same Bazel Project was not revealing some of the problems that can be encountered with code-generators.

The documentation has also been updated accordingly.

A prior PR has allowed that, for some languages, certain
Rules may be specified as being "code generators". This
mechanism was via Bazel tags on the target. This commit
changes the mechanism so that the rule names are specified
by settings instead.
@github-actions github-actions bot added product: CLion CLion plugin product: IntelliJ IntelliJ plugin product: GoLand GoLand plugin awaiting-review Awaiting review from Bazel team on PRs labels Oct 13, 2024
@andponlin-canva
Copy link
Contributor Author

If this is merged, please disregard PRs 6886 and 6887.

I note today that on this PR there are some test issues.

Aspect Tests for IJ OSS Latest Stable on :ubuntu: Ubuntu 22.04 LTS

The error in this case is;

Error in _intellij_aspect_test_fixture: \
Aspect //aspect:intellij_info.bzl%intellij_info_aspect: \
Aspect parameter attribute 'python_code_generator_rule_names' must use the 'values' restriction.

The problem here is that the test fixture is using a Rule to test the Aspect and apparently...

For rule-propagated aspects, int and string parameters must have values specified on them.

I'm not sure the best path forward here as the changes in this PR are using an Aspect Parameter to convey data to the Aspect. It's a legitimate use of the Aspect Parameter but the test rig is not going to work well with it. I could go back to using an Action Env instead but I would agree that the Aspect Parameter is better.

CLion OSS Latest Stable on :ubuntu: Ubuntu 22.04 LTS

Appears to be an issue with Bazel 5. It's not too clear what the problem is from the trace; will require further investigation.

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
Status: Untriaged
Development

Successfully merging this pull request may close these issues.

4 participants