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

Fix: Add treesitter option to use pre-compiled binaries #654

Closed
wants to merge 1 commit into from

Conversation

feoh
Copy link
Collaborator

@feoh feoh commented Feb 27, 2024

On Windows, when new users install kickstart it blows up by default because having a C compiler in PATH is not a reaonle expectation on hat platform.

Also, this should decrease first use startup time substantially on all platforms because curl-ing down a binary is much faster than compiling from scratch.

Also updated README as Windows folks are expected to turn on developer mode so symlinks and 'curl' will work.

On Windows, when new users install kickstart it blows up by default
because having a C compiler in PATH is not a reaonle expectation on hat
platform.

Also, this should decrease first use startup time substantially on all
platforms because curl-ing down a binary is much faster than compiling
from scratch.

Also updated README as Windows folks are expected to turn on developer
mode so symlinks and 'curl' will work.
@feoh feoh requested a review from tjdevries February 28, 2024 01:40
@feoh feoh self-assigned this Feb 28, 2024
@feoh
Copy link
Collaborator Author

feoh commented Feb 28, 2024

@dam9000 @sodoku @rubin @kdheepak and anyone else - could folks please give this a look? It will help us not 💥 by default at startup on Windows and speed up startup times for everyone by not requiring folks to compile treesitter language extensions when binaries are available from Github releases.

@dam9000
Copy link
Contributor

dam9000 commented Feb 28, 2024

This looks like a great idea, I mostly use Linux so I don't have an immediate way to test it out, but might try sometime later.

@feoh
Copy link
Collaborator Author

feoh commented Feb 28, 2024

@dam9000 Thanks! It will help (and work just fine!) for Linux folks as well.

It should speed up the initial Neovim startup because rather than compiling all those treesitter extensions, it'll just automatically fetch the Linux compiled binaries from Github releases.

@dam9000
Copy link
Contributor

dam9000 commented Feb 28, 2024

Hmm, by reading the page you linked:
https://github.com/nvim-treesitter/nvim-treesitter/wiki/Windows-support

it does not sound to me that this change will actually use the prebuilt binaries but just change the way the (source?) packages are downloaded to curl instead of git clone and it seems on linux curl is already the default method while on windows is git because it requires an additional Admin mode step:

enable creating symbolic links for non-admin users (enable "Developer Mode": https://www.ghacks.net/2016/12/04/windows-10-creators-update-symlinks-without-elevation) or refrain from using parsers that use symbolic links in their repos (tree-sitter-typescript, tree-sitter-ocaml). Otherwise tar will fail on extracting those archives on Windows

So in a way, although it might be faster compared to git on Windows this add an additional preparation step that would not be required otherwise?

As for the binaries it says:

Pre-compiled parsers (disabled at the moment -> we want to create a proper binary releases in future)
Important: this feature is disabled at the moment. We want to create proper binary releases for people to download in future

Unless the doc is wrong and the pre-compiled parsers actually work?

@feoh
Copy link
Collaborator Author

feoh commented Feb 28, 2024

Yup I think the doc is wrong.

I say that because with this change I can start neovim with kickstart and get zero errors, with Treesitter operational.

Without that change, it explodes every single time unless I use a Powershell with the C compiler in PATH, which is apparently a BIG antipattern according to every expert on Windows I've talked to.

I'll go poke them about updating that doc or telling me I'm mistaken :)

@feoh
Copy link
Collaborator Author

feoh commented Feb 28, 2024

Hmm, by reading the page you linked: https://github.com/nvim-treesitter/nvim-treesitter/wiki/Windows-support

it does not sound to me that this change will actually use the prebuilt binaries but just change the way the (source?) packages are downloaded to curl instead of git clone and it seems on linux curl is already the default method while on windows is git because it requires an additional Admin mode step:

enable creating symbolic links for non-admin users (enable "Developer Mode": https://www.ghacks.net/2016/12/04/windows-10-creators-update-symlinks-without-elevation) or refrain from using parsers that use symbolic links in their repos (tree-sitter-typescript, tree-sitter-ocaml). Otherwise tar will fail on extracting those archives on Windows

So in a way, although it might be faster compared to git on Windows this add an additional preparation step that would not be required otherwise?

Just realized I didn't address this.

While startup will be faster, the critical issue here is that without this change we blow up on startup on Windows every time unless there is a C compiler in the user's PATH.

On Windows, this is a cardinal sin, because it exposes the user to massive malware risk. Forcing users to keep a C compiler in PATH is like bundling a handgun pointed at the uesr's feet with the safety off and a hair trigger.

I know this is all super confusing for folks who never use Windows, I'm trying to bridge that gap and help folks who might otherwise simply throw up their hands in disgust and walk away from Neovim actually be able to use our project.

Thanks for bearing with me, and I appreciate the questions and feedback.

@dam9000
Copy link
Contributor

dam9000 commented Feb 29, 2024

I tested your branch on Windows without a C compiler and it still errors out about it:

Error detected while processing C:\Users\Damjan\AppData\Local\nvim\init.lua:
No C compiler found! "cc", "gcc", "clang", "cl", "zig" are not executable.
No C compiler found! "cc", "gcc", "clang", "cl", "zig" are not executable.
No C compiler found! "cc", "gcc", "clang", "cl", "zig" are not executable.
No C compiler found! "cc", "gcc", "clang", "cl", "zig" are not executable.
No C compiler found! "cc", "gcc", "clang", "cl", "zig" are not executable.
No C compiler found! "cc", "gcc", "clang", "cl", "zig" are not executable.
No C compiler found! "cc", "gcc", "clang", "cl", "zig" are not executable.
Press ENTER or type command to continue

This is for sure the error message coming from TreeSitter, as it is the same if I run manually:

:TSInstall c

I also dig a bit in the treesitter code for installing the parsers and from my understanding the package that is downloaded is this:
https://github.com/tree-sitter/tree-sitter-c/archive/refs/tags/v0.20.7.tar.gz

which contains no pre-built binaries (libraries)

For the record, I used these commands to install nvim, git and ripgrep:

winget install neovim
winget install git.git
winget install BurntSushi.ripgrep.MSVC

So my conclusion is that prefer_git option only changes the download method - curl vs git, the package is the same, there are no prebuilt libraries and the error about the C compiler is still there.

@feoh
Copy link
Collaborator Author

feoh commented Feb 29, 2024

thanks so much for digging so deeply into this.

can't blame me for trying I guess :-)

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.

2 participants