-
Notifications
You must be signed in to change notification settings - Fork 543
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(gazelle): Update to latest version of go-tree-sitter #2413
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... still seems to not work on our pyle
repo. I'll have more time to investigate early next week.
Please add a test case for python 3.12 syntax to gazelle/python/testdata
. Specifically, we want to make sure that imports, gazelle annotations, and if __name__
that come after py3.12-only syntax are still parsed. For example:
# my_baz.py
import foo
# gazelle:include_dep //:bar
def baz[T]() -> T: # 3.12-only syntax
pass
# these still get parsed
import foobar
# gazelle:include_dep //:hello
if __name__ == "__main__":
pass
Should generate (assuming per-file target):
py_binary(
name = "my_baz",
srcs = ["my_baz.py"],
deps = [
"//:bar",
"//:foo",
"//:foobar",
"//:hello",
],
)
If you only get 2 deps or a py_library
, then the parsing still failed 😢
CHANGELOG.md
Outdated
@@ -52,7 +52,7 @@ Unreleased changes template. | |||
|
|||
{#v0-0-0-changed} | |||
### Changed | |||
* Nothing yet. | |||
* (gazelle): Update to latest version of go-tree-sitter. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please include that this adds support for python 3.12 grammar (and 3.13?)
gazelle/go-tree-sitter.patch
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: please move to a "patches" directory
maybe we can publish go-tree-sitter with patches to bazel registry? like https://registry.bazel.build/modules/circl |
While investigating #2396 and why #2413 doesn't appear to be working for us, I realized that one of the things I was making heavy use of was additional parser logging that I had added. This adds some of that logging. I also throw in some documentation because I found it helpful. Users can (attempt to) get additional parse failure information by setting the `GAZELLE_VERBOSE` environment variable to `1`. Eg: ```console $ GAZELLE_VERBOSE=1 bazel run //:gazelle ``` Here are some example logs: ```console $ GAZELLE_VERBOSE=1 bazel run //:gazelle INFO: Invocation ID: a4e026d8-17df-426c-b1cc-d3980690dd53 ... INFO: Running command line: bazel-bin/gazelle INFO: Streaming build results to: https://btx.cloud.google.com/invocations/a4e026d8-17df-426c-b1cc-d3980690dd53 gazelle: WARNING: failed to parse "hello/get_deps.py". The resulting BUILD target may be incorrect. gazelle: Parse error at {Row:1 Column:0}: def search_one_more_level[T](): gazelle: The above was parsed as: (ERROR (identifier) (call function: (list (identifier)) arguments: (argument_list))) gazelle: ERROR: failed to generate target "//hello:get_deps" of kind "py_library": a target of kind "pyle_py_binary" with the same name already exists. Use the '# gazelle:python_library_naming_convention' directive to change the naming convention. $ $ bazel run //:gazelle INFO: Invocation ID: 726c9fd6-f566-4c30-95ef-c4781ad155de ... INFO: Running command line: bazel-bin/gazelle INFO: Streaming build results to: https://btx.cloud.google.com/invocations/726c9fd6-f566-4c30-95ef-c4781ad155de gazelle: WARNING: failed to parse "hello/get_deps.py". The resulting BUILD target may be incorrect. gazelle: ERROR: failed to generate target "//hello:get_deps" of kind "py_library": a target of kind "pyle_py_binary" with the same name already exists. Use the '# gazelle:python_library_naming_convention' directive to change the naming convention. ``` --------- Co-authored-by: Richard Levasseur <[email protected]> Co-authored-by: Richard Levasseur <[email protected]>
Include a patch to the build files generated by the go_repository to support building with uplevel files references.
de98fae
to
f915bd1
Compare
I think I got things working now. Had to remove I also moved the patch file into a |
Just relaying info that @maffoo and I had at work: This still doesn't work for
It's looking like we might have to do that. We can apply patches like @maffoo has done, but then those patches won't be used when some other project uses @hunshcn How do you recommend we publish to BCR? Fork go-tree-sitter, add MODULE.bazel and rules, and go from there? Or is there another option? |
Just like bazelbuild/bazel-central-registry#674 |
Ah thanks, that process looks pretty easy. I'll start it now. |
I filed bazel-contrib/bazel-gazelle#1984 to propose allowing default patches on libraries included by go_deps and put up bazel-contrib/bazel-gazelle#1985 to actually implement this. We'll see what the bazel-gazelle maintainers say... |
The latest version of go-tree-sitter includes grammer changes for python 3.12. To use it, we need to patch the build files generated by the go_repository to support building with uplevel files references. This should fix #2396.