-
-
Notifications
You must be signed in to change notification settings - Fork 570
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
gazelle dougthor42/go-tree-sitter dependency conflicts with standard go-tree-sitter #2630
Comments
One of the reasons why @dougthor42 went with a fork, it was easier to support @dougthor42, I think I am missing some other for the comments. |
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 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 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 |
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 🤷 |
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? |
@dougthor42 was there any reason you can't simply use a patch like I have? |
@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. |
Sadly I don't have an answer for you there - I also don't know
I did initially try things that way (eg use 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 I'll try investigating again and document why 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", |
Can we at least avoid changing the go mod name? Since you've renamed the |
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. |
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? |
Are only root module patches respected? This is a go module patch, not bazel patch. The patch is listed in either 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. |
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. 🤞 🙏 |
Nope, go patches also only apply at the root module. 👎
Which is what I said in #2630 (comment) |
Well those couple |
We're getting into territories that I know less about, so take my reply with a grain of salt. AIUI, If you're more familiar with @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.
Agreed. Work might be minimized if the @aignas Do you think that would work? Will a Another option for you @jbedard is to patch 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 The patch to revert rules_python_gazelle_plugin back to smacker/go-tree-sitterdiff --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",
|
Ah, looks like we had the same idea at the same time, haha. Personally I think it's unreasonable to tell all Admittedly, the route I chose may not have been the best one. But also, you're the first user to experience an issue with it, so... 🤷 . Asking a single user to apply 2 patches is, IMO, much better than asking all users to apply one patch. |
I think you can just use the go.mod I'm on mobile and can't verify any of this atm but I'll try later this week if you don't get to it. |
Thanks @jbedard for pushing me on this. I've learned quite a bit about the go ecosystem! I've got something that looks like it works. I'd like to have you test it out to verify. |
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.
The text was updated successfully, but these errors were encountered: