Auto-install tools

[vogelsgesang]

Forking this off from a discussion in https://github.com/bazelbuild/vscode-bazel/pull/369#discussion_r1554639007

It would lower the barrier of entry for new users if this extension would offer an option to automatically download any missing tools (bazelisk, buildifier, language server).

[vogelsgesang]

In the other discussion, @cameron-martin brought up

rust-analyzer used to do that before it came bundled with rust and the user experience was great.

which brings up the question: should we download the tools or package them?

My current thinking: I would lean towards "downloading" instead of "packaging".

For buildifier and bazel(isk), there is a high chance that those tools are already installed on the system. Bundling them as part of the VS Code extension might hence turn out to be a waste of package size. I would ask the user for confirmation before downloading, because it might be that our auto-detection simply failed, and we should give the users a way to point us to the right location before we automatically download it.

For the language server(s) it is very unlikely that those are already installed independently of the VS Code extension. However, they are currently platform-dependent, and bundling them would mean that we need platform-specific packages. I would like to avoid this since I see it as unnecessary complexity in our build. Hence, I would propose downloading those, but without asking for user confirmation

[adam-azarchs]

Another possibility to bring up for some cases at least would be to compile the tools to WASM for embedding into the extension. That could work for e.g. buildifier/buildozer (although go's wasm story isn't super great because it needs to bundle the whole runtime, so you end up with a fairly large blob). It's still important to be able to provide an override in case someone needs to use a different version from what's bundled.

This would not of course be applicable to bazelisk (which can't really do its job within a sandbox) and probably wouldn't make sense for the language server, either, since AFAIKT that's written in Java, and I don't know of any projects trying to compile Java to something cross-platform (unless you count Java, which I don't, personally).

[cameron-martin]

FYI starpls and bazel-lsp are written in Rust. I do suspect compiling to wasm may be more hassle than it's worth, considering binaries for all major platforms are already available.

[adam-azarchs]

Compiling rust to wasm works really well actually; rust is probably the best possible language to use for wasm. But sure, maybe more trouble than it's worth. The benefit would be the ability to embed directly in the extension rather than downloading the platform-specific binary on demand, without significantly sacrificing performance. WASM also has inherent security advantages, though those might be more of a liability for a language server that wants to be able to crawl the whole workspace.

[vogelsgesang]

indeed. It seems that VSCode already shipped support for WASI LSPs in https://code.visualstudio.com/updates/v1_84#_wasmwasi-support-for-language-servers and the example source code doesn't look too bad...

[vogelsgesang]

I tried quickly building the LSPs as web-assembly using

bazel build //:bazel-lsp --platforms=@rules_rust//rust/platform:wasm
bazel build crates/starpls --platforms=@rules_rust//rust/platform:wasm

Unfortunately, both of those calls failed. For bazel-lsp, I got

ERROR: /Users/avogelsgesang/repos/bazel-lsp/BUILD:3:12: While resolving toolchains for target //:bazel-lsp (1d5eb45): No matching toolchains found for types @@bazel_tools//tools/cpp:toolchain_type.
To debug, rerun with --toolchain_resolution_debug='@@bazel_tools//tools/cpp:toolchain_type'

while for starpls, I got

error: the wasm*-unknown-unknown targets are not supported by default, you may need to enable the "js" feature. For more information see: https://docs.rs/getrandom/#webassembly-support
   --> external/crates__getrandom-0.2.12/src/lib.rs:291:9
    |
291 | /         compile_error!("the wasm*-unknown-unknown targets are not supported by \
292 | |                         default, you may need to enable the \"js\" feature. \
293 | |                         For more information see: \
294 | |                         https://docs.rs/getrandom/#webassembly-support");
    | |________________________________________________________________________^

error[E0433]: failed to resolve: use of undeclared crate or module `imp`
   --> external/crates__getrandom-0.2.12/src/lib.rs:347:9
    |
347 |         imp::getrandom_inner(dest)?;
    |         ^^^ use of undeclared crate or module `imp`

error: aborting due to 2 previous errors

If anyone manages to compile either of them to WASM, I think we should bundle the LSPs directly with the VS Code extension

@cameron-martin @withered-magic @jfirebaugh I would also be interested in your opinion on bundling the language servers. (And maybe one of you also knows how to compile the language servers to WASM / WASI)

[adam-azarchs]

The first error is because you don't have a wasm C++ toolchain configured for bazel. That isn't something I've tried before.

The second error

you may need to enable the "js" feature

is coming from the getrandom crate; looks like something along the dependency chain isn't properly enabling that feature, which is required for wasm. Though actually I doubt that getrandom should be required at all for this; it looks like the root cause problem there is the tower crate is making the mistake of not disabling default features for rand.

[vogelsgesang]

I managed to build starpls to WASM. The trick was to remove the (unused) tonic depency

Patch

```patch diff --git a/BUILD.bazel b/BUILD.bazel index 3f88add..be09e97 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -10,22 +10,15 @@ rust_library_group( ], ) -rust_library_group( - name = "tonic_runtime", - deps = [ - ":prost_runtime", - "@crates//:tonic", - ], -) - rust_prost_toolchain( name = "prost_toolchain_impl", prost_plugin = "@crates//:protoc-gen-prost__protoc-gen-prost", prost_runtime = ":prost_runtime", prost_types = "@crates//:prost-types", proto_compiler = "@com_google_protobuf//:protoc", - tonic_plugin = "@crates//:protoc-gen-tonic__protoc-gen-tonic", - tonic_runtime = ":tonic_runtime", + tonic_plugin = None, + tonic_plugin_flag = "", + tonic_runtime = None, ) toolchain( diff --git a/WORKSPACE.bazel b/WORKSPACE.bazel index 15aa31c..ec361aa 100644 --- a/WORKSPACE.bazel +++ b/WORKSPACE.bazel @@ -103,9 +103,6 @@ crates_repository( "protoc-gen-prost": [crate.annotation( gen_binaries = ["protoc-gen-prost"], )], - "protoc-gen-tonic": [crate.annotation( - gen_binaries = ["protoc-gen-tonic"], - )], }, cargo_lockfile = "//:Cargo.lock", lockfile = "//:Cargo.Bazel.lock", diff --git a/crates/starpls_bazel/Cargo.toml b/crates/starpls_bazel/Cargo.toml index aef8681..b598509 100644 --- a/crates/starpls_bazel/Cargo.toml +++ b/crates/starpls_bazel/Cargo.toml @@ -16,8 +16,6 @@ serde_json = "1.0.114" [dev-dependencies] prost-types = "0.12.3" protoc-gen-prost = "0.2.3" -protoc-gen-tonic = "0.3.0" -tonic = "0.11.0" [build-dependencies] prost-build = "0.12.3" ```

The resulting starpls.wasm is unfortunately rather big, with 16MB. I wonder if it has other unused dependencies that could be removed.

I didn't check if the binary actually works in VS Code, though...

[adam-azarchs]

The resulting starpls.wasm is unfortunately rather big, with 16MB. I wonder if it has other unused dependencies that could be removed.

Entirely-unused crates will generally be discarded at link time, because their linker sections should be unreferenced. Unless they're registering some kind of initializer, of course, in which case they won't be unreferenced. Our team has been enforcing cargo-machete in CI for a while now.

$ cargo machete
Analyzing dependencies of crates in this directory...
cargo-machete found the following unused dependencies in starpls:
starpls_bazel -- /mnt/home/adam.azarchs/gopath/src/github.com/withered-magic/starpls/crates/starpls_bazel/Cargo.toml:
        bytes

More important I think is that there are multiple versions of several crates being compiled in, e.g. 3 versions of itertools. We've also been enforcing cargo-deny to prevent this (as well as checking licensing, CVEs, etc) but it's a bit of a PITA to keep compliant with that. Helps with build times, though!

The root of most of the duplication there is seems to be having two versions of prost, which in turn is due to protoc-gen-prost depending on an older version of prost. But it looks like all of prost-types, protoc-gen-prost, and protoc-gen-tonic are unused; dropping them drops the older version of prost and at least one of the itertools versions. But then again those are all dev dependencies so they don't actually affect the build output... at least not directly. They may affect which features are activated in downstream crates.

The other thing to try is enabling LTO for the wasm build. That can have a major impact on output size.

[cameron-martin]

That bazel-lsp error will be because the dummy cc toolchain from rules_rust isn't registered. That's a bug in the bzlmod implementation in rules_rust, but can be manually registered to work around this. This is done in the non-bzlmod case here.

[withered-magic]

I'm actually not super familiar with WASM but my understanding is that it enforces some sandboxing type of functionality where your WASM program doesn't have access anymore to things like the filesystem or running external commands; is there any way to allow the WASM program to do these things from the VSCode side?

For example, the language servers need to be able to spawn bazel subprocesses to gather information about the current workspace, and they also need read access to the output base directory to resolve external dependencies.

[cameron-martin]

WASI is meant to give access to things like the filesystem to webassembly, but I haven't been keeping up to date with the progress of it.

[vogelsgesang]

Probably the best resource which I could find on this: https://code.visualstudio.com/blogs/2023/06/05/vscode-wasm-wasi

That demo even showed a Python interpreter running in WASM, with access to the workspace files.

However, the longer I look into this, the more I think that the WASI integration in VS Code is still experimental

[adam-azarchs]

I don't think it would work so well for a language server. It would, however, work great for e.g. buildifier, which can just communicate over stdio/stdout/stderr.