Skip to content
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

Add go-tree-sitter 20240827094217-dd81d9e9be82 #3366

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

dougthor42
Copy link

Add go-tree-sitter @ 20240827094217-dd81d9e9be82.

go-tree-sitter is a dep of rules_python_gazelle_plugin. The goal of getting go-tree-sitter into BCR is to allow us to bump the grammar parser so that PEP 695 (Type Parameter Syntax) is correctly parsed by Gazelle (see bazelbuild/rules_python#2396 and bazelbuild/rules_python#2413).

Similar to circl (#674), the core patch 03_add_autogenerated_build_files.patch was autogenerated by Gazelle. The others were manually tweaked to get builds to work.

Addressing some potential questions:

  1. Is there any chance to upstream the BUILD files?
    • The maintainer of go-tree-sitter does not appear to be open to the idea
  2. Have it in a fork and some maintainers can review any BUILD file change.
    • This might be suitable, though it would just be a personal fork (dougthor42 namespace) and I'm hesitant to have a Bus Factor of 1. I'm willing to maintain it, though. In fact, the patches in PR were based off my branch here: https://github.com/dougthor42/go-tree-sitter/tree/bcr
    • If we go with a fork, then we might not need to add to BCR at all as we can probably just update rules_python_gazelle_plugin to depend on the fork instead.

Related Links:

@bazel-io
Copy link
Member

Hello @bazelbuild/bcr-maintainers, modules without existing maintainers (go-tree-sitter) have been updated in this PR. Please review the changes.

@fmeum fmeum added presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval skip-url-stability-check Skip the URL stability check for the PR labels Dec 10, 2024
Author: Douglas Thor <[email protected]>
Date: Thu Nov 21 04:19:47 2024 +0000

Run gazelle
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add the exact go install / run commands, including Gazelle versions?

@aignas
Copy link
Contributor

aignas commented Dec 10, 2024

+1 for keeping it in BCR instead of Doug's own fork. That reduces the bus factor by a lot.

@dougthor42
Copy link
Author

There is one issue with the BCR option: It doesn't support the old WORKSPACE stuff. (Right?)

github-merge-queue bot pushed a commit to bazelbuild/rules_python that referenced this pull request Dec 14, 2024
… Parameter Syntax) by using dougthor42's fork of go-tree-sitter (#2496)

Replaces #2413.

Fixes #2396.

This updates the `go-tree-sitter` dependency to use my fork that
includes `BUILD.bazel` files. Specifically, the `BUILD.bazel` files in
the fork include references to top-level code like `array.h` which the
original Gazelle-generated files for `go-tree-sitter` were not able to
handle. I also include the test cases that @maffoo created in #2413 and
verified that they (a) fail before the fix and (b) pass after the fix.

The fork is: https://github.com/dougthor42/go-tree-sitter

The branch that includes all changes is:
https://github.com/dougthor42/go-tree-sitter/tree/for-rules-python-gazelle-plugin

A couple notes:
+ I have a PR open to get `go-tree-sitter` into BCR
[here](bazelbuild/bazel-central-registry#3366).
However:
1. I'm having trouble getting tests to pass and to get things running
locally to validate it
2. Using BCR would not fix things for people who still use WORKSPACE
(right?)
+ The fork is _mostly_ [autogenerated BUILD.bazel files from
gazelle](smacker/go-tree-sitter@cfa9bdf)
but also contains:
+ [manual updates so that build files reference the toplevel `array.h`
and other
files](smacker/go-tree-sitter@63f89cd)
+ [replace all `smacker` with `dougthor42` so that `go build`
works](smacker/go-tree-sitter@8a73cbd)
    + various other more minor things.
+ I was unable to get `go mod edit -replace` to work, so I've just
manually updated `go.mod` and whatnot everywhere. If someone with more
go knowledge has a suggestion I'm happy to hear it.

---------

Co-authored-by: Matthew Neeley <[email protected]>
Co-authored-by: Ignas Anikevicius <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
presubmit-auto-run Presubmit jobs will be triggered for new changes automatically without reviewer's approval skip-url-stability-check Skip the URL stability check for the PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants