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

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

Open
nirvdrum opened this issue Nov 25, 2024 · 7 comments
Open

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

nirvdrum opened this issue Nov 25, 2024 · 7 comments
Labels
bug Something isn't working vscode This pull request should be included in the VS Code extension's release notes

Comments

@nirvdrum
Copy link

nirvdrum commented Nov 25, 2024

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:

this.yjitEnabled = (yjit && major > 3) || (major === 3 && minor >= 2);

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 nirvdrum added bug Something isn't working vscode This pull request should be included in the VS Code extension's release notes labels Nov 25, 2024
@nirvdrum nirvdrum changed the title Reliance on YJIT breaks other Ruby interpreters LSP doesn't run without YJIT (breaks on non-CRuby) Nov 25, 2024
@andyw8
Copy link
Contributor

andyw8 commented Nov 25, 2024

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: #2292

@andyw8
Copy link
Contributor

andyw8 commented Nov 25, 2024

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

@nirvdrum
Copy link
Author

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.

@andyw8
Copy link
Contributor

andyw8 commented Nov 25, 2024

Ah, found it: #549

I've restored the branch: vs/test_against_truffle_and_jruby

@vinistock
Copy link
Member

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.

@nirvdrum
Copy link
Author

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.

@vinistock
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working vscode This pull request should be included in the VS Code extension's release notes
Projects
None yet
Development

No branches or pull requests

3 participants