bazelbuild / rules_python

Bazel Python Rules
https://rules-python.readthedocs.io
Apache License 2.0
538 stars 542 forks source link

feat(gazelle): Update to latest version of go-tree-sitter #2413

Open maffoo opened 1 week ago

maffoo commented 1 week ago

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.

hunshcn commented 1 week ago

maybe we can publish go-tree-sitter with patches to bazel registry? like https://registry.bazel.build/modules/circl

maffoo commented 6 days ago

I think I got things working now. Had to remove go-tree-sitter from go.mod because this seems to override the custom go_respository definition and results in patches not being applied to go-tree-sitter before building. I'm not sure whether we still need go.mod for other dependencies or if they can be handled by the go_repository definitions in deps.bzl too; experts please weight in.

I also moved the patch file into a patches subdirectory and added a test case exercising the py3.12 syntax that fails on main but succeeds on this branch. I tried to create a minimal failing test case based on our internal code; interestingly, I found that the function docstring is key to getting this to fail on main, no idea why that is the case...

dougthor42 commented 5 days ago

Just relaying info that @maffoo and I had at work:

This still doesn't work for pyle:

$ bazel run //:gazelle src/pyle/tools
INFO: Invocation ID: e06d2fbb-5642-4159-9abf-f1402367d298
INFO: Streaming build results to: https://btx.cloud.google.com/invocations/e06d2fbb-5642-4159-9abf-f1402367d298
WARNING: For repository 'bazel_skylib', the root module requires module version bazel_skylib@1.6.1, but got bazel_skylib@1.7.1 in the resolved dependency graph.
ERROR: no such package '@@[unknown repo 'com_github_smacker_go_tree_sitter' requested from @@rules_python_gazelle_plugin~]//python': The repository '@@[unknown repo 'com_github_smacker_go_tree_sitter' requested from @@rules_python_gazelle_plugin~]' could not be resolved: No repository visible as '@com_github_smacker_go_tree_sitter' from repository '@@rules_python_gazelle_plugin~'
ERROR: /usr/local/google/home/dthor/.cache/bazel/_bazel_dthor/dbe74c4144b5c9a438d84a119652bef9/external/rules_python_gazelle_plugin~/python/BUILD.bazel:6:11: no such package '@@[unknown repo 'com_github_smacker_go_tree_sitter' requested from @@rules_python_gazelle_plugin~]//python': The repository '@@[unknown repo 'com_github_smacker_go_tree_sitter' requested from @@rules_python_gazelle_plugin~]' could not be resolved: No repository visible as '@com_github_smacker_go_tree_sitter' from repository '@@rules_python_gazelle_plugin~' and referenced by '@@rules_python_gazelle_plugin~//python:python'
ERROR: Analysis of target '//:gazelle' failed; build aborted: Analysis failed
INFO: Elapsed time: 1.437s, Critical Path: 0.51s
INFO: 1 process: 1 internal.
ERROR: Build did NOT complete successfully
ERROR: Build failed. Not running target

Here's something weird. If I run bazel sync in my rules_pyle/gazelle directory it applies patches to go-tree-sitter:

DEBUG: /usr/local/google/home/maffoo/.cache/bazel/_bazel_maffoo/051c217dd68266c8e1eaa3206da93fe4/external/gazelle~/internal/go_repository.bzl:345:10: bazel_gazelle/go_repository: applying patches. name=com_github_smacker_go_tree_sitter, patches=[Label("//:patches/0001-go-tree-sitter-build.patch")]format_text_overflowformat_text_wrap

but whenever I try to run some other build command I see patches=[], no patches are applied, and the build fails.

I think this is the fundamental problem. The go_repository definitions are only having an effect in the WORKSPACE. The MODULE.bazel uses a different mechanism with the go_deps extension and this doesn't know about the patches we want to apply. There's a way to specify patches for repositories brought in with go_deps but it only works in the root module so we can't use it (it's fine if running bazel in rules_python/gazelle, but when we pull this in as a dependency to pyle the patches get ignored again. The only way I can see to fix this is to update bazel-gazelle itself or add go-tree-sitter to the bazel central registry.

maybe we can publish go-tree-sitter with patches to bazel registry?

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 rules_python_gazelle_plugin because things like go_deps.module_override and will only work on the root module.

@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?

hunshcn commented 5 days ago

Just like https://github.com/bazelbuild/bazel-central-registry/pull/674 Maybe @fmeum can provide some information about.

dougthor42 commented 5 days ago

Ah thanks, that process looks pretty easy. I'll start it now.

maffoo commented 3 days ago

I filed https://github.com/bazel-contrib/bazel-gazelle/issues/1984 to propose allowing default patches on libraries included by go_deps and put up https://github.com/bazel-contrib/bazel-gazelle/pull/1985 to actually implement this. We'll see what the bazel-gazelle maintainers say...