Skip to content

Commit

Permalink
Check if bundle is valid before restarting
Browse files Browse the repository at this point in the history
  • Loading branch information
vinistock committed Jan 15, 2025
1 parent 7fb9adf commit 12d6d3b
Show file tree
Hide file tree
Showing 8 changed files with 143 additions and 11 deletions.
31 changes: 24 additions & 7 deletions exe/ruby-lsp-launcher
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,24 @@

setup_error = nil
install_error = nil
reboot = false

# Read the initialize request before even starting the server. We need to do this to figure out the workspace URI.
# Editors are not required to spawn the language server process on the same directory as the workspace URI, so we need
# to ensure that we're setting up the bundle in the right place
$stdin.binmode
headers = $stdin.gets("\r\n\r\n")
content_length = headers[/Content-Length: (\d+)/i, 1].to_i
raw_initialize = $stdin.read(content_length)
workspace_uri = ARGV.first

raw_initialize = if workspace_uri && !workspace_uri.start_with?("--")
# If there's an argument without `--`, then it's the server asking to compose the bundle and passing to this
# executable the workspace URI. We can't require gems at this point, so we built a fake initialize request manually
reboot = true
"{\"params\":{\"workspaceFolders\":[{\"uri\":\"#{workspace_uri}\"}]}}"
else
# Read the initialize request before even starting the server. We need to do this to figure out the workspace URI.
# Editors are not required to spawn the language server process on the same directory as the workspace URI, so we need
# to ensure that we're setting up the bundle in the right place
$stdin.binmode
headers = $stdin.gets("\r\n\r\n")
content_length = headers[/Content-Length: (\d+)/i, 1].to_i
$stdin.read(content_length)
end

# Compose the Ruby LSP bundle in a forked process so that we can require gems without polluting the main process
# `$LOAD_PATH` and `Gem.loaded_specs`. Windows doesn't support forking, so we need a separate path to support it
Expand Down Expand Up @@ -91,6 +101,13 @@ rescue StandardError => e
$LOAD_PATH.unshift(File.expand_path("../lib", __dir__))
end

# When performing a lockfile re-boot, this executable is invoked to set up the composed bundle ahead of time. In this
# flow, we are not booting the LSP yet, just checking if the bundle is valid before rebooting
if reboot
# Use the exit status to signal to the server if composing the bundle succeeded
exit(install_error || setup_error ? 1 : 0)
end

# Now that the bundle is set up, we can begin actually launching the server. Note that `Bundler.setup` will have already
# configured the load path using the version of the Ruby LSP present in the composed bundle. Do not push any Ruby LSP
# paths into the load path manually or we may end up requiring the wrong version of the gem
Expand Down
28 changes: 28 additions & 0 deletions lib/ruby_lsp/server.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ def process_message(message)
end,
),
)
when "rubyLsp/composeBundle"
compose_bundle(message)
when "$/cancelRequest"
@global_state.synchronize { @cancelled_requests << message[:params][:id] }
when nil
Expand Down Expand Up @@ -283,6 +285,7 @@ def run_initialize(message)
document_range_formatting_provider: true,
experimental: {
addon_detection: true,
compose_bundle: true,
},
),
serverInfo: {
Expand Down Expand Up @@ -1279,5 +1282,30 @@ def window_show_message_request(message)

addon.handle_window_show_message_response(result[:title])
end

sig { params(message: T::Hash[Symbol, T.untyped]).void }
def compose_bundle(message)
already_composed_path = File.join(@global_state.workspace_path, ".ruby-lsp", "bundle_is_composed")
command = "#{Gem.ruby} #{File.expand_path("../../exe/ruby-lsp-launcher", __dir__)} #{@global_state.workspace_uri}"
id = message[:id]

# We compose the bundle in a thread so that the LSP continues to work while we're checking for its validity. Once
# we return the response back to the editor, then the restart is triggered
Thread.new do
send_log_message("Recomposing the bundle ahead of restart")
pid = Process.spawn(command)
_, status = Process.wait2(pid)

if status&.exitstatus == 0
# Create a signal for the restart that it can skip composing the bundle and launch directly
FileUtils.touch(already_composed_path)
send_message(Result.new(id: id, response: { success: true }))
else
# This special error code makes the extension avoid restarting in case we already know that the composed
# bundle is not valid
send_message(Error.new(id: id, code: BUNDLE_COMPOSE_FAILED_CODE, message: "Failed to compose bundle"))
end
end
end
end
end
18 changes: 18 additions & 0 deletions lib/ruby_lsp/setup_bundler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ def initialize(project_path, **options)
@lockfile_hash_path = T.let(@custom_dir + "main_lockfile_hash", Pathname)
@last_updated_path = T.let(@custom_dir + "last_updated", Pathname)
@error_path = T.let(@custom_dir + "install_error", Pathname)
@already_composed_path = T.let(@custom_dir + "bundle_is_composed", Pathname)

dependencies, bundler_version = load_dependencies
@dependencies = T.let(dependencies, T::Hash[String, T.untyped])
Expand All @@ -71,6 +72,23 @@ def initialize(project_path, **options)
def setup!
raise BundleNotLocked if !@launcher && @gemfile&.exist? && !@lockfile&.exist?

# If the bundle was composed ahead of time using our custom `rubyLsp/composeBundle` request, then we can skip the
# entire process and just return the composed environment
if @already_composed_path.exist?
$stderr.puts("Ruby LSP> Composed bundle was set up ahead of time. Skipping...")
@already_composed_path.delete

env = bundler_settings_as_env
env["BUNDLE_GEMFILE"] = @custom_gemfile.exist? ? @custom_gemfile.to_s : @gemfile.to_s

if env["BUNDLE_PATH"]
env["BUNDLE_PATH"] = File.expand_path(env["BUNDLE_PATH"], @project_path)
end

env["BUNDLER_VERSION"] = @bundler_version.to_s if @bundler_version
return env
end

# Automatically create and ignore the .ruby-lsp folder for users
@custom_dir.mkpath unless @custom_dir.exist?
ignore_file = @custom_dir + ".gitignore"
Expand Down
2 changes: 2 additions & 0 deletions lib/ruby_lsp/utils.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ class DelegateRequestError < StandardError
CODE = -32900
end

BUNDLE_COMPOSE_FAILED_CODE = -33000

# A notification to be sent to the client
class Message
extend T::Sig
Expand Down
1 change: 1 addition & 0 deletions project-words
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ dont
eglot
Eglot
eruby
exitstatus
EXTGLOB
fakehome
FIXEDENCODING
Expand Down
23 changes: 23 additions & 0 deletions test/server_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -995,6 +995,29 @@ def test_edits_outside_of_declarations_do_not_trigger_indexing
assert_equal(1, entries.length)
end

def test_compose_bundle_creates_file_to_skip_next_compose
Dir.mktmpdir do |dir|
Dir.chdir(dir) do
@server.process_message({
id: 1,
method: "initialize",
params: {
initializationOptions: {},
capabilities: { general: { positionEncodings: ["utf-8"] } },
workspaceFolders: [{ uri: URI::Generic.from_path(path: dir).to_s }],
},
})

capture_subprocess_io do
@server.process_message({ id: 2, method: "rubyLsp/composeBundle" })
end
result = find_message(RubyLsp::Result, id: 2)
assert(result.response[:success])
assert_path_exists(File.join(dir, ".ruby-lsp", "bundle_is_composed"))
end
end
end

private

def with_uninstalled_rubocop(&block)
Expand Down
18 changes: 18 additions & 0 deletions test/setup_bundler_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -858,6 +858,24 @@ def test_update_does_not_fail_if_gems_are_uninstalled
end
end

def test_only_returns_environment_if_bundle_was_composed_ahead_of_time
Dir.mktmpdir do |dir|
Dir.chdir(dir) do
FileUtils.mkdir(".ruby-lsp")
FileUtils.touch(File.join(".ruby-lsp", "bundle_is_composed"))

require "bundler/cli/update"
require "bundler/cli/install"
Bundler::CLI::Update.expects(:new).never
Bundler::CLI::Install.expects(:new).never

assert_output("", "Ruby LSP> Composed bundle was set up ahead of time. Skipping...\n") do
refute_empty(RubyLsp::SetupBundler.new(dir, launcher: true).setup!)
end
end
end
end

private

def with_default_external_encoding(encoding, &block)
Expand Down
33 changes: 29 additions & 4 deletions vscode/src/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,8 @@ export class Workspace implements WorkspaceInterface {
return this.start();
}

let canRestart = false;

switch (this.lspClient.state) {
// If the server is still starting, then it may not be ready to handle a shutdown request yet. Trying to send
// one could lead to a hanging process. Instead we set a flag and only restart once the server finished booting
Expand All @@ -214,10 +216,18 @@ export class Workspace implements WorkspaceInterface {
break;
// If the server is running, we want to stop it, dispose of the client and start a new one
case State.Running:
await this.stop();
await this.lspClient.dispose();
this.lspClient = undefined;
await this.start();
// If the server doesn't support checking the validity of the composed bundle or if composing the bundle was
// successful, then we can restart
canRestart =
!this.lspClient.initializeResult?.capabilities.experimental
.compose_bundle || (await this.composingBundleSucceeds());

if (canRestart) {
await this.stop();
await this.lspClient.dispose();
this.lspClient = undefined;
await this.start();
}
break;
// If the server is already stopped, then we need to dispose it and start a new one
case State.Stopped:
Expand Down Expand Up @@ -441,4 +451,19 @@ export class Workspace implements WorkspaceInterface {
hash.update(fileContents.toString());
return hash.digest("hex");
}

private async composingBundleSucceeds(): Promise<boolean> {
if (!this.lspClient) {
return false;
}

try {
const response: { success: boolean } = await this.lspClient.sendRequest(
"rubyLsp/composeBundle",
);
return response.success;
} catch (error: any) {
return false;
}
}
}

0 comments on commit 12d6d3b

Please sign in to comment.