Skip to content

fix(gazelle): Use dougthor42's go-tree-sitter via replace instead of modifying names #2667

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

Draft
wants to merge 16 commits into
base: main
Choose a base branch
from

Conversation

dougthor42
Copy link
Collaborator

@dougthor42 dougthor42 commented Mar 15, 2025

Fixes #2630.

In #2496 we added support for Python 3.12 syntax using the less-than-ideal method of "fork and rename everything". This turned out to be a Bad Idea ™️ for people who happened to be using smacker/go-tree-sitter as a direct dependency.

This PR effectively reverts #2496 and replaces it with a more correct way of doing go module forking: the replace directive for go.mod.

Special thanks goes out to @jbedard for raising the issue and for continuing to poke me to fix it!

If we look at #2496 plus this PR, the net diff to get Python 3.12 support is approximately1:

diff --git a/gazelle/deps.bzl b/gazelle/deps.bzl
index 948d61e5..9872a4d8 100644
--- a/gazelle/deps.bzl
+++ b/gazelle/deps.bzl
@@ -187,9 +184,10 @@ def go_deps():
     )
     go_repository(
         name = "com_github_smacker_go_tree_sitter",
+        commit = "71e8858be7ad43fb866e2daabe83e95e6eff91bd",
         importpath = "github.com/smacker/go-tree-sitter",
-        sum = "h1:7QZKUmQfnxncZIJGyvX8M8YeMfn8kM10j3J/2KwVTN4=",
-        version = "v0.0.0-20240422154435-0628b34cbf9c",
+        remote = "https://github.com/dougthor42/go-tree-sitter",
+        vcs = "git",
     )
     go_repository(
         name = "com_github_stretchr_objx",
diff --git a/gazelle/go.mod b/gazelle/go.mod
index 4b65e71d..11461b22 100644
--- a/gazelle/go.mod
+++ b/gazelle/go.mod
@@ -9,7 +9,7 @@ require (
        github.com/bmatcuk/doublestar/v4 v4.6.1
        github.com/emirpasic/gods v1.18.1
        github.com/ghodss/yaml v1.0.0
-       github.com/smacker/go-tree-sitter v0.0.0-20240422154435-0628b34cbf9c
+       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
@@ -24,3 +24,5 @@ require (
        golang.org/x/tools v0.9.1 // indirect
        gopkg.in/yaml.v3 v3.0.1 // indirect
 )
+
+replace github.com/smacker/go-tree-sitter => github.com/dougthor42/go-tree-sitter v0.0.0-20250311040318-71e8858be7ad
diff --git a/gazelle/go.sum b/gazelle/go.sum
index 46e0127e..25fcf6de 100644
--- a/gazelle/go.sum
+++ b/gazelle/go.sum
@@ -13,9 +13,10 @@ github.com/chzyer/logex v1.1.10/go.mod h1:+Ywpsq7O8HXn0nuIou7OrIPyXbp3wmkHB+jjWR
 github.com/chzyer/readline v0.0.0-20180603132655-2972be24d48e/go.mod h1:nSuG5e5PlCu98SY8svDHJxuZscDgtXS6KTTbou5AhLI=
 github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1/go.mod h1:Q3SI9o4m/ZMnBNeIyt5eFwwo7qiLfzFZmjNmxjkiQlU=
 github.com/client9/misspell v0.3.4/go.mod h1:qj6jICC3Q7zFZvVWo7KLAzC3yx5G7kyvSDkc90ppPyw=
-github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 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-20250311040318-71e8858be7ad h1:IwRobcK7dpOliiJ/oqBkMIULmY9iYeqgAJNRbSreG7Q=
+github.com/dougthor42/go-tree-sitter v0.0.0-20250311040318-71e8858be7ad/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=
@@ -46,6 +47,8 @@ github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZN
 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=

Footnotes

  1. Manually generated; ignores the Python 3.12 test

@jbedard
Copy link
Contributor

jbedard commented Mar 17, 2025

That looks right, but the CI error seems to be the "fatal error: ../array.h: No such file or directory" error which required the custom BUILD files? Yet only bzlmod is failing? :/

@dougthor42
Copy link
Collaborator Author

The issue is that go.mod replace is only respected for root Bazel modules:

The root module can override certain aspects of the dependency resolution performed by the go_deps extension.

So while the WORKSPACE stuff works, and the unit tests in gazelle/python/testdata pass, anything that uses rules_python_gazelle_plugin as a bzlmod dependency (like the examples dir), fails.

@jbedard
Copy link
Contributor

jbedard commented Mar 20, 2025

So this solution won't work? There must be another way to do this other then today's solution 😢

@jbedard
Copy link
Contributor

jbedard commented Mar 25, 2025

@dougthor42 how about adding to the default_gazelle_overrides.bzl. I've never investigated it before so I'm not entirely sure if that's right, and it seems to be in a "bzlmod" directory.

Can we just contribute to that file to get gazelle properly generating BUILDs for go-tree-sitter instead of doing any of this forking/patching?

@dougthor42
Copy link
Collaborator Author

So this solution won't work?

As is, no it will not work 😞

how about adding to the default_gazelle_overrides.bzl

default_gazelle_overrides.bzl applies a set of gazelle directives when a particular go module is downloaded a gazelle is automatically run on it (eg: "name the files BUILD.bazel instead of BUILD when running Gazelle on 'github.com/envoyproxy/protoc-gen-validate'"). It doesn't support patching/editing the generated BUILD files.

Other solutions?

Option 1: Split brain

Given that the WORKSPACE stuff in this PR seems to work, I think our next step is to raise bazelbuild/bazel-central-registry#3366 from the dead

  • WORKSPACE will use the dougthor42 fork
    • until we drop WORKSPACE support
  • bzlmod will pull from BCR.

@aignas @rickeylev thoughts on this approach?

Option 2: Fix Gazelle itself

Basically get upstream Gazelle to fix bazel-contrib/bazel-gazelle#2059.

There's got to be a way to have Gazelle generate the correct code for C code that has an up-level import like is used in go-tree-sitter https://github.com/smacker/go-tree-sitter/blob/dd81d9e9be82a8cac96ed1d50c7389c5f1997c02/python/scanner.c#L1:

#include "../array.h"

However, this sort of fix is outside of my wheelhouse.

@aignas
Copy link
Collaborator

aignas commented Mar 26, 2025

I think option 2 is the right fix here. Let's see what the maintainers have to say.

If option 2 is not possible, then option 1 is OK for the time being.

@jbedard
Copy link
Contributor

jbedard commented Mar 26, 2025

default_gazelle_overrides.bzl applies a set of gazelle directives when a particular go module is downloaded a gazelle is automatically run on it

I was wondering if a set of gazelle directives could possibly fix this issue, to generate the correct files. I don't know the actual bug well enough to understand though, I've always just patched it 🤷

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

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