ruby-lsp icon indicating copy to clipboard operation
ruby-lsp copied to clipboard

LSP doesn't run without YJIT (breaks on non-CRuby)

Open nirvdrum opened this issue 1 year ago • 8 comments

Description

The collection of the Ruby LSP information will not work, presumably because it assumes YJIT is available (e.g., doesn't work with TruffleRuby). I've collected it from another window running with CRuby and tried to manually hack it together:

Ruby LSP Info

### Ruby LSP Information

#### VS Code Version

1.95.3

#### Ruby LSP Extension Version

0.8.14

#### Ruby LSP Server Version

0.22.1

#### Ruby LSP Addons



#### Ruby Version

3.3.5

#### Ruby Version Manager

none

Failed to setup the bundle: Command failed: gem list ruby-lsp language_server-protocol prism rbs sorbet-runtime ERROR: truffleruby: invalid option --yjit (Use --help for usage instructions.) . See Troubleshooting for help

Reproduction steps

  1. Use TruffleRuby for your project (e.g., truffleruby-24.1.1 from ruby-build)
  2. Start the Ruby LSP using VS Code
  3. Open a Ruby file
  4. See the LSP fail to activate because the --yjit flag is invalid

Code snippet or error message

The console output suggests that it will only enable YJIT if RubyVM::YJIT is defined:

2024-11-25 11:16:27.635 [info] (nuevo-protobuf) Discovered version manager shadowenv
2024-11-25 11:16:27.641 [info] (nuevo-protobuf) Found shadowenv executable at /opt/homebrew/bin
2024-11-25 11:16:27.642 [info] (nuevo-protobuf) Running command: `/opt/homebrew/bin/shadowenv exec -- ruby -W0 -rjson -e 'STDERR.print("RUBY_LSP_ACTIVATION_SEPARATOR" + { env: ENV.to_h, yjit: !!defined?(RubyVM::YJIT), version: RUBY_VERSION, gemPath: Gem.path }.to_json + "RUBY_LSP_ACTIVATION_SEPARATOR")'` in /Users/nirvdrum/src/github.com/Shopify/nuevo-protobuf using shell: /opt/homebrew/bin/fish

However, I see an error dialog with:

Failed to setup the bundle: Command failed: gem list ruby-lsp language_server-protocol prism rbs sorbet-runtime ERROR: truffleruby: invalid option --yjit (Use --help for usage instructions.) . See [Troubleshooting](https://shopify.github.io/ruby-lsp/troubleshooting.html) for help

Browsing the code, I think the issue is:

https://github.com/Shopify/ruby-lsp/blob/35fc5e90ef8b763ca2e1b0ff5acbe9c47d19c12a/vscode/src/ruby.ts#L235

TruffleRuby 24.1.1 reports as being Ruby 3.2.4 compatible. While dev builds of TruffleRuby 24.2.0-dev now report as 3.3.5. The check for whether YJIT is available needs to consider more context than just the Ruby version number. I think the check for RubyVM::YJIT is the best approach since it's possible to have a CRuby build without YJIT, so checking for RUBY_ENGINE == "ruby" may be insufficient.

I haven't tried running with JRuby, but I strongly suspect the LSP doesn't run there for similar reasons.

nirvdrum avatar Nov 25 '24 16:11 nirvdrum

I haven't tried running with JRuby, but I strongly suspect the LSP doesn't run there for similar reasons.

JRuby doesn't work because RBS isn't yet supported: https://github.com/Shopify/ruby-lsp/issues/2292

andyw8 avatar Nov 25 '24 16:11 andyw8

I suspect there are other places with implicit assumptions that we're always running on MRI. We should probably run against TrufleRuby in CI.

andyw8 avatar Nov 25 '24 16:11 andyw8

I worked with @vinistock on adding TruffleRuby to CI a year or so ago. At the time there were some bugs in TruffleRuby that we've since fixed and I'd expect CI to pass. But, maybe that old branch is still around and we can resurrect it.

nirvdrum avatar Nov 25 '24 16:11 nirvdrum

Ah, found it: https://github.com/Shopify/ruby-lsp/pull/549

I've restored the branch: vs/test_against_truffle_and_jruby

andyw8 avatar Nov 25 '24 16:11 andyw8

Kevin is correct in his assessment that the issue is in how we turn on YJIT for Ruby 3.1 and 3.2 on the extension side. We need to also check the Ruby engine as part of enabling YJIT, which should fix this issue.

The TruffleRuby CI was for the server only, so it wouldn't have caught this issue. To get extension/server integration tests running on different Ruby implementations, we would need a new matrix on CI.

vinistock avatar Nov 26 '24 14:11 vinistock

Maybe the detection can be unified. Something is doing a defined?(RubyVM::YJIT) check and I think that's probably the best. It'll be undefined on CRuby if it's built without YJIT support (e.g., Windows builds) and on any alternative Ruby implementation.

nirvdrum avatar Nov 26 '24 19:11 nirvdrum

We're already doing that for 3.3+, but the problem is that RubyVM::YJIT.enable didn't exist in Ruby 3.2 and the LSP gets a lot of benefit from YJIT on 3.2.

It shouldn't be too hard to avoid it (hopefully), we just need to check the engine and avoid turning it on if it's not MRI.

vinistock avatar Nov 26 '24 20:11 vinistock

This issue is being marked as stale because there was no activity in the last 2 months

github-actions[bot] avatar Jan 26 '25 12:01 github-actions[bot]