cursorless-dev / vscode-parse-tree

Syntax trees for VSCode using tree-sitter
MIT License
40 stars 35 forks source link

Yarn buildscripts fail on macOS 14.12.1 + Apple Silicon due to nan-related issues #76

Open cocona20xx opened 6 months ago

cocona20xx commented 6 months ago

Due to an issue with older versions of nan first documented here,, a nan version of at least 2.16.0 is required to build on certain platforms under certain circumstances (at the very least on certain macOS machines)—tree-sitter-clojure and tree-sitter-haskel currently cause build failures on my end related to said issue when building this project.

For tree-sitter-clojure, I've submitted a PR that will fix this issue entirely on their end if it goes through; nothing will need to be changed in this project's package.json. The dependency on tree-sitter-haskel on the other hand needs to be bumped to a newer commit—grabbing the latest commit from master works fine despite tree-sitter-haskel not upgrading their version of nan. This seems to suggest the bug doesn't have to be fixed specifically by bumping the nan version, but I'm pretty unfamiliar with javascript in general so idk what's actually happening in that regard.

For posterity, here's the logs related to both build failures:

tree-sitter-clojure build failure: https://paste.ee/p/ZZvBj tree-sitter-haskel build failure: https://paste.ee/p/tnx6I

pokey commented 6 months ago

Building 40 tree-sitter parsers makes this project a bit of a nightmare to build locally 😅. Fwiw I usually pin my node to 16 and my python to 3.9 when building. Also, looking at the package.json for both of those tree-sitter parsers, it looks like both should be compatible with nan 2.16, so we could prob fix this by bumping our yarn lock file

What are you trying to do? I usually just end up building it myself when someone needs a new parser, because I'll need to manually publish it anyway

So feel free to just file a PR even if it doesn't build and I can clean it up and ship it; they're always small PRs for new parsers

sogaiu commented 6 months ago

We (tree-sitter-clojure maintainers) don't currently test generated bindings or other "build"-related things (except parser.c and friends though hopefully those can be removed at some point) as these are not human-written / -verified source artifacts. Although some such bits exist now, we're moving in the direction of removing them -- possibly after the next tree-sitter release.

We've kept package.json around partly for vscode-parse-tree's build process, but would rather keep the file as simple as possible if it is to remain. If you look at the history of our package.json, you can see it used to contain a fair bit more.

We would prefer not to change tree-sitter-clojure's minimal package.json. We do not want to add information about dependencies / versions as we don't currently have appropriate testing that involves that nor do we plan to add any. We view adding such info as taking on a responsibility to look after it. Although we had dependency information before, we came to the conclusion it was misleading / sending inappropriate signals to others as to what we are providing.

Cursorless (strictly speaking, vscode-parse-tree) had already started using tree-sitter-clojure before we adopted a clearer stance on what we provided so we have been trying to keep things working for that specific case, but we do not want to expand this. May be it's better if a separate branch was made specifically for vscode-parse-tree or perhaps a fork could be maintained by a party with the inclination and resources to look after the relevant files.

Sorry if this is coming off on the harsh side, it's not meant to be -- this is more the consequences of the maintenance, building, and distribution of tree-sitter grammar-related files being far from ideal -- please note also the security concerns linked to.

I'm going to close https://github.com/sogaiu/tree-sitter-clojure/pull/56 for now, but might revisit it (or something similar) later.

cocona20xx commented 6 months ago

Building 40 tree-sitter parsers makes this project a bit of a nightmare to build locally 😅. Fwiw I usually pin my node to 16 and my python to 3.9 when building. Also, looking at the package.json for both of those tree-sitter parsers, it looks like both should be compatible with nan 2.16, so we could prob fix this by bumping our yarn lock file

What are you trying to do? I usually just end up building it myself when someone needs a new parser, because I'll need to manually publish it anyway

So feel free to just file a PR even if it doesn't build and I can clean it up and ship it; they're always small PRs for new parsers

Considering what @sogaiu said above (which is fully understandable tbh), I'll probably do exactly that; trying to add a gdscript parser via https://github.com/PrestonKnopp/tree-sitter-gdscript in order to have cursorless integration for the godot-vscode plugins.