gazelle dougthor42/go-tree-sitter dependency conflicts with standard go-tree-sitter

[jbedard]

The use of a forked version of go-tree-sitter causes conflicts when other gazelle languages use tree sitter.

🐞 bug report

Is this a regression?

Yes

Description

It seems 15cc0b3 changed to a fork of tree-sitter, just to change the BUILD files? Shouldn't gazelle and/or rules_go be used to do that instead of forking an entire repository?

🔬 Minimal Reproduction

Use go-tree-sitter in another gazelle language.

[aignas]

One of the reasons why @dougthor42 went with a fork, it was easier to support WORKSPACE and MODULE.bazel with the same codebase: #2496 (comment)

@dougthor42, I think I am missing some other for the comments.

[dougthor42]

That's the root of it, yeah.

The current (at the time of the PR) version of bazel-contrib/bazel-gazelle does not support the file structure that tree-sitter moved to in smacker/go-tree-sitter#158. Specifically, bazel-gazelle would not correctly reference the toplevel array.h file when autogenerating the BUILD.bazel files for tree-sitter/python (and others, but we don't care about those here 🙃 ).

So sadly we can't just rely on bazel-gazelle to autogenerated the files for us.

We also can't use patches to modify the autogenerated BUILD.bazel files after bazel-gazelle runs because things like go_deps.module_override and will only work on the root module. So while a patch would work to get rules_python's tests to pass, anyone using rules_python_gazelle_plugin would see failures. Or they'd have to manually patch things themselves.

I'm happy to hear alternatives to forking - I don't want to maintain the fork, haha, but I'm willing to.

BCR is one option (bazelbuild/bazel-central-registry#3366) but that won't help WORKSPACE users who are on python3.12 or higher.

[jbedard]

I'm using go-tree-sitter at HEAD and it works fine for me, although I still have rules_go in WORKSPACE. I just needed a simple patch for the python language: https://github.com/aspect-build/aspect-cli/blob/main/third_party/github.com/smacker/go-tree-sitter/cc_library.patch

My version of rules_python is out of date though so maybe you ran into new issues? But this is blocking me from updating so idk 🤷

[jbedard]

Maybe I just don't know go mod well, but how can I move forward when I have other gazelle languages using a non-forked tree-sitter?

[jbedard]

@dougthor42 was there any reason you can't simply use a patch like I have?

[aignas]

@jbedard could you give an example how you used it? As far as I remember among the many options this was the most appealing that works on Workspace and bzlmod.

[dougthor42]

how can I move forward when I have other gazelle languages using a non-forked tree-sitter?

Sadly I don't have an answer for you there - I also don't know go mod very well.

---

was there any reason you can't simply use a patch like I have?

I did initially try things that way (eg use go_repository(..., patches=[...]) but that was not working.

Sadly I don't exactly recall why. I think this issue might have been that the patches do not get applied when an outer module depends on rules_python_gazelle_plugin. Or maybe it was that gazelle/WORKSPACE uses gazelle/deps.bzl and thus can apply patches, but gazelle/MODULE.bazel uses use_repo(go_deps, ...) and does't support patches? Or maybe I just couldn't reconcile the two.

I'll try investigating again and document why go_repository(..., patches=[...]) won't work. Heck, maybe we'll get lucky and I'll learn something new that makes it work! Then I don't have to maintain the tree-sitter fork!

That said, I'm not sure when I'll have time to do such investigation.

Aside: here's some test that I `git stash`ed back when I was working on this

@@ -192,7 +194,16 @@ def go_deps():
         importpath = "github.com/smacker/go-tree-sitter",
         sum = "h1:6C8qej6f1bStuePVkLSFxoU22XBS165D3klxlzRg8F4=",
         version = "v0.0.0-20240827094217-dd81d9e9be82",
-        patches = ["go-tree-sitter.patch"],
+        # patches = ["go-tree-sitter.patch"],
+        patches = [
+            # "go-tree-sitter1.patch",
+            "go-tree-sitter2.patch",
+            ":go-tree-sitter1.patch",
+            "//:go-tree-sitter2.patch",
+            "//gazelle:go-tree-sitter2.patch",
+            "//patches:go-tree-sitter2.patch",
+        ],
     )
     go_repository(
         name = "com_github_stretchr_objx",

[jbedard]

Can we at least avoid changing the go mod name? Since you've renamed the smacker/go-tree-sitter module when you forked it that means I can't even use the go.mod replace to switch back...

[jbedard]

@jbedard could you give an example how you used it? As far as I remember among the many options this was the most appealing that works on Workspace and bzlmod.

Standard patching of go modules, in this case it's WORKSPACE though: https://github.com/aspect-build/aspect-cli/blob/16252422fbf16b6ee952944e42ba62cb66043312/go.bzl#L1206

But bzlmod has an equivalent. You might still have issues when repos are also using go-tree-sitter and importing rules_python/gazelle, then those repos need to apply your patch. But that's better then what you have today where this scenario isn't even possible because you renamed the module.

[aignas]

I think the problem for us was how to solve this so that the patch works for our users - having to maintain the patching in BCR (since only root module patches are respected and gazelle has a feature to use the BCR versin if it exists) and also to have the patches in the workspace deps of the gazelle plugin seemed a lot of work and we chose the fork approach.

@jbedard, do you have any suggestions how to not do double the work?

[jbedard]

Are only root module patches respected? This is a go module patch, not bazel patch. The patch is listed in either go_repository (generated for WORKSPACE) or MODULE go_deps.module_override().

Either way your current solution completely breaks any other gazelle languages that use tree-sitter. I don't understand how that's acceptable at all? Requiring patches sounds better then completely breaking other gazelle languages.

[dougthor42]

Bazel patches are only respected at the root level, that's correct.

I'm not 100% sure about go patches - I'm testing that right now. 🤞 🙏

[dougthor42]

Nope, go patches also only apply at the root module. 👎

ERROR: /usr/local/google/home/dthor/.cache/bazel/_bazel_dthor/0f8c52850e7230283fc2f8033149fba2/external/gazelle~/internal/bzlmod/go_deps.bzl:100:13: Traceback (most recent call last):
        File "/usr/local/google/home/dthor/.cache/bazel/_bazel_dthor/0f8c52850e7230283fc2f8033149fba2/external/gazelle~/internal/bzlmod/go_deps.bzl", line 381, column 27, in _go_deps_impl
                _process_overrides(module_ctx, module, "module_override", module_overrides, _process_module_override, archive_overrides)
        File "/usr/local/google/home/dthor/.cache/bazel/_bazel_dthor/0f8c52850e7230283fc2f8033149fba2/external/gazelle~/internal/bzlmod/go_deps.bzl", line 214, column 32, in _process_overrides
                _fail_on_non_root_overrides(module_ctx, module, override_type)
        File "/usr/local/google/home/dthor/.cache/bazel/_bazel_dthor/0f8c52850e7230283fc2f8033149fba2/external/gazelle~/internal/bzlmod/go_deps.bzl", line 100, column 13, in _fail_on_non_root_overrides
                fail(_FORBIDDEN_OVERRIDE_TAG.format(
Error in fail: Using the "go_deps.module_override" tag in a non-root Bazel module is forbidden, but module "rules_python_gazelle_plugin" requests it.

Which is what I said in #2630 (comment)

[jbedard]

Well those couple go_deps.module_override lines adding a patch are still significantly easier then patching rules_python to use smacker/go-tree-sitter and adding the patches 🫤

[dougthor42]

Can we at least avoid changing the go mod name?

We're getting into territories that I know less about, so take my reply with a grain of salt.

AIUI, go mod et. al. pull go packages directly from GitHub. So if we wanted to keep the same smacker/go-tree-sitter, the code (BUILD.bazel files) would have to live in that repo. It could be a branch, as go mod supports pulling via commit hash, but it would have to be in https://github.com/smacker/go-tree-sitter`. I don't have any access to that repo and the maintainer doesn't seem very interested in supporting it (example: smacker/go-tree-sitter#175 has no replies from the maintainer(s), and it looks like none of the other Issue do either).

If you're more familiar with go and know how to keep the name the same, please do tell!

---

@aignas Do we have a roadmap for dropping WORKSPACE support? Bazel 8 already disables it by default, and it'll be removed in Bazel 9 which will probably be released by end of 2025.

Once WORKSPACE is dropped we can use BCR.

maintain the patching in BCR ... and also to have the patches in the workspace deps of the gazelle plugin seemed a lot of work

Agreed. Work might be minimized if the WORKSPACE patches only applied to the python code, but IMO the BCR patch should support all langues that go-tree-sitter supports. Which ends up being ~1600 lines.

@aignas Do you think that would work? Will a go_repository patch work on non-root modules? I don't have any way of (easily) testing WORKSPACE stuff.

---

Another option for you @jbedard is to patch rules_python_gazelle_plugin to use the version of tree-sitter that you want. It's not pretty, but it should unblock you while we figure out the best path forward.

Here's the patch. It reverts things back to smacker/go-tree-sitter and also bumps it to the latest version smacker/go-tree-sitter@dd81d9e9. You'll still need to apply another patch to get python 3.12 to work, because Bazel will not be able to build go-tree-sitter from that commit.

The patch to revert rules_python_gazelle_plugin back to smacker/go-tree-sitter

diff --git a/gazelle/MODULE.bazel b/gazelle/MODULE.bazel
index 6bbc74bc..0a553831 100644
--- a/gazelle/MODULE.bazel
+++ b/gazelle/MODULE.bazel
@@ -21,9 +21,9 @@ use_repo(
     go_deps,
     "com_github_bazelbuild_buildtools",
     "com_github_bmatcuk_doublestar_v4",
-    "com_github_dougthor42_go_tree_sitter",
     "com_github_emirpasic_gods",
     "com_github_ghodss_yaml",
+    "com_github_smacker_go_tree_sitter",
     "com_github_stretchr_testify",
     "in_gopkg_yaml_v2",
     "org_golang_x_sync",
diff --git a/gazelle/deps.bzl b/gazelle/deps.bzl
index fbb5285a..0f538040 100644
--- a/gazelle/deps.bzl
+++ b/gazelle/deps.bzl
@@ -183,10 +183,10 @@ def go_deps():
         version = "v0.0.0-20190812154241-14fe0d1b01d4",
     )
     go_repository(
-        name = "com_github_dougthor42_go_tree_sitter",
-        importpath = "github.com/dougthor42/go-tree-sitter",
-        sum = "h1:b9s96BulIARx0konX36sJ5oZhWvAvjQBBntxp1eUukQ=",
-        version = "v0.0.0-20241210060307-2737e1d0de6b",
+        name = "com_github_smacker_go_tree_sitter",
+        importpath = "github.com/smacker/go-tree-sitter",
+        sum = "h1:6C8qej6f1bStuePVkLSFxoU22XBS165D3klxlzRg8F4=",
+        version = "v0.0.0-20240827094217-dd81d9e9be82",
     )
     go_repository(
         name = "com_github_stretchr_objx",
diff --git a/gazelle/go.mod b/gazelle/go.mod
index 33ee6bb0..5a410e53 100644
--- a/gazelle/go.mod
+++ b/gazelle/go.mod
@@ -7,9 +7,9 @@ require (
        github.com/bazelbuild/buildtools v0.0.0-20231103205921-433ea8554e82
        github.com/bazelbuild/rules_go v0.41.0
        github.com/bmatcuk/doublestar/v4 v4.7.1
-       github.com/dougthor42/go-tree-sitter v0.0.0-20241210060307-2737e1d0de6b
        github.com/emirpasic/gods v1.18.1
        github.com/ghodss/yaml v1.0.0
+       github.com/smacker/go-tree-sitter v0.0.0-20240827094217-dd81d9e9be82
        github.com/stretchr/testify v1.9.0
        golang.org/x/sync v0.2.0
        gopkg.in/yaml.v2 v2.4.0
diff --git a/gazelle/go.sum b/gazelle/go.sum
index 5acd4a6d..17941d69 100644
--- a/gazelle/go.sum
+++ b/gazelle/go.sum
@@ -17,8 +17,6 @@ github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMn
 github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
-github.com/dougthor42/go-tree-sitter v0.0.0-20241210060307-2737e1d0de6b h1:b9s96BulIARx0konX36sJ5oZhWvAvjQBBntxp1eUukQ=
-github.com/dougthor42/go-tree-sitter v0.0.0-20241210060307-2737e1d0de6b/go.mod h1:87UkDyPt18bTH/FvinLc/kj587VNYOdRKZT1la4T8Hg=
 github.com/emirpasic/gods v1.18.1 h1:FXtiHYKDGKCW2KzwZKx0iC0PQmdlorYgdFG9jPXJ1Bc=
 github.com/emirpasic/gods v1.18.1/go.mod h1:8tpGGwCnJ5H4r6BWwaV6OrWmMoPhUl5jm/FMNAnJvWQ=
 github.com/envoyproxy/go-control-plane v0.9.1-0.20191026205805-5f8ba28d4473/go.mod h1:YTl/9mNaCwkRvm6d1a2C3ymFceY/DCBVvsKhRF0iEA4=
@@ -47,6 +45,14 @@ github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeN
 github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
 github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
 github.com/prometheus/client_model v0.0.0-20190812154241-14fe0d1b01d4/go.mod h1:xMI15A0UPsDsEKsMN9yxemIoYk6Tm2C1GtYGdfGttqA=
+github.com/smacker/go-tree-sitter v0.0.0-20240422154435-0628b34cbf9c h1:7QZKUmQfnxncZIJGyvX8M8YeMfn8kM10j3J/2KwVTN4=
+github.com/smacker/go-tree-sitter v0.0.0-20240422154435-0628b34cbf9c/go.mod h1:q99oHDsbP0xRwmn7Vmob8gbSMNyvJ83OauXPSuHQuKE=
+github.com/smacker/go-tree-sitter v0.0.0-20240827094217-dd81d9e9be82 h1:6C8qej6f1bStuePVkLSFxoU22XBS165D3klxlzRg8F4=
+github.com/smacker/go-tree-sitter v0.0.0-20240827094217-dd81d9e9be82/go.mod h1:xe4pgH49k4SsmkQq5OT8abwhWmnzkhpgnXeekbx2efw=
+github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
+github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw=
+github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg=
+github.com/stretchr/testify v1.7.4/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU=
 github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
 github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
 go.starlark.net v0.0.0-20210223155950-e043a3d3c984/go.mod h1:t3mmBBPzAVvK0L0n1drDmrQsJ8FoIx4INCqVMTr/Zo0=
@@ -102,6 +108,7 @@ gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+
 gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
 gopkg.in/yaml.v2 v2.4.0 h1:D8xgwECY7CYvx+Y2n4sBz93Jn9JRvxdiyyo8CTfuKaY=
 gopkg.in/yaml.v2 v2.4.0/go.mod h1:RDklbk79AGWmwhnvt/jBztapEOGDOx6ZbXqjP6csGnQ=
+gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
 gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM=
 honnef.co/go/tools v0.0.0-20190102054323-c2f93a96b099/go.mod h1:rf3lG4BRIbNafJWhAfAdb/ePZxsR/4RtNHQocxwk9r4=
diff --git a/gazelle/python/BUILD.bazel b/gazelle/python/BUILD.bazel
index 893c82e8..627a867c 100644
--- a/gazelle/python/BUILD.bazel
+++ b/gazelle/python/BUILD.bazel
@@ -39,11 +39,11 @@ go_library(
         "@bazel_gazelle//rule:go_default_library",
         "@com_github_bazelbuild_buildtools//build:go_default_library",
         "@com_github_bmatcuk_doublestar_v4//:doublestar",
-        "@com_github_dougthor42_go_tree_sitter//:go-tree-sitter",
-        "@com_github_dougthor42_go_tree_sitter//python",
         "@com_github_emirpasic_gods//lists/singlylinkedlist",
         "@com_github_emirpasic_gods//sets/treeset",
         "@com_github_emirpasic_gods//utils",
+        "@com_github_smacker_go_tree_sitter//:go-tree-sitter",
+        "@com_github_smacker_go_tree_sitter//python",
         "@org_golang_x_sync//errgroup",
     ],
 )
diff --git a/gazelle/python/file_parser.go b/gazelle/python/file_parser.go
index c147984f..a1f47f40 100644
--- a/gazelle/python/file_parser.go
+++ b/gazelle/python/file_parser.go
@@ -22,8 +22,8 @@ import (
        "path/filepath"
        "strings"
 
-       sitter "github.com/dougthor42/go-tree-sitter"
-       "github.com/dougthor42/go-tree-sitter/python"
+       sitter "github.com/smacker/go-tree-sitter"
+       "github.com/smacker/go-tree-sitter/python"
 )
 
 const (
@@ -115,10 +115,10 @@ func (p *FileParser) parseMain(ctx context.Context, node *sitter.Node) bool {
                                a, b = b, a
                        }
                        if a.Type() == sitterNodeTypeIdentifier && a.Content(p.code) == "__name__" &&
-                               // at github.com/dougthor42/go-tree-sitter@latest (after v0.0.0-20240422154435-0628b34cbf9c we used)
+                               // at github.com/smacker/go-tree-sitter@latest (after v0.0.0-20240422154435-0628b34cbf9c we used)
                                // "__main__" is the second child of b. But now, it isn't.
                                // we cannot use the latest go-tree-sitter because of the top level reference in scanner.c.
-                               // https://github.com/dougthor42/go-tree-sitter/blob/04d6b33fe138a98075210f5b770482ded024dc0f/python/scanner.c#L1
+                               // https://github.com/smacker/go-tree-sitter/blob/04d6b33fe138a98075210f5b770482ded024dc0f/python/scanner.c#L1
                                b.Type() == sitterNodeTypeString && string(p.code[b.StartByte()+1:b.EndByte()-1]) == "__main__" {
                                return true
                        }

The patch to get python to work with the bad version of smacker/go-tree-sitter

--- a/BUILD.bazel +++ b/BUILD.bazel @@ -1,5 +1,11 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") +filegroup( + name = "common", + srcs = ["alloc.h", "api.h", "array.h"], + visibility = [":__subpackages__"], +) + go_library( name = "go-tree-sitter", srcs = [ --- a/python/BUILD.bazel +++ b/python/BUILD.bazel @@ -3,6 +3,7 @@ load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "python", srcs = [ + "//:common", "binding.go", "parser.c", "parser.h",