From b811f32c6b165c082dd93aa565f345f25217c28b Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Tue, 22 Oct 2024 16:27:29 -0400 Subject: [PATCH] Use Bundler CLI directly and send errors to telemetry --- exe/ruby-lsp-launcher | 37 +++++++++++++----- lib/ruby_lsp/base_server.rb | 1 + lib/ruby_lsp/scripts/compose_bundle.rb | 4 +- lib/ruby_lsp/server.rb | 22 +++++++---- lib/ruby_lsp/setup_bundler.rb | 53 ++++++++++++++++++++++++++ lib/ruby_lsp/utils.rb | 8 ++++ sorbet/rbi/shims/bundler.rbi | 26 +++++++++++-- test/integration_test.rb | 16 +++----- test/setup_bundler_test.rb | 53 ++++++++++++++++++++++++++ 9 files changed, 187 insertions(+), 33 deletions(-) diff --git a/exe/ruby-lsp-launcher b/exe/ruby-lsp-launcher index 3e8f49d9f..2cdf97dbc 100755 --- a/exe/ruby-lsp-launcher +++ b/exe/ruby-lsp-launcher @@ -22,11 +22,15 @@ locked_bundler_version_file = File.join(".ruby-lsp", "locked_bundler_version") # 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 pid = if Gem.win_platform? - spawn(Gem.ruby, File.expand_path("../lib/ruby_lsp/scripts/compose_bundle_windows.rb", __dir__), raw_initialize) + Process.spawn( + ENV.to_hash, + Gem.ruby, + File.expand_path("../lib/ruby_lsp/scripts/compose_bundle_windows.rb", __dir__), + raw_initialize, + ) else fork do - $LOAD_PATH.unshift(File.expand_path("../lib", __dir__)) - require "ruby_lsp/scripts/compose_bundle" + require_relative "../lib/ruby_lsp/scripts/compose_bundle" compose(raw_initialize) end end @@ -64,10 +68,15 @@ rescue StandardError => e Gem::Specification.find_by_name("ruby-lsp").activate end -# Now that the bundle is set up, we can begin actually launching the server +error_path = File.join(".ruby-lsp", "install_error") -$LOAD_PATH.unshift(File.expand_path("../lib", __dir__)) +install_error = if File.exist?(error_path) + Marshal.load(File.read(error_path)) +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 require "ruby_lsp/load_sorbet" require "ruby_lsp/internal" @@ -91,7 +100,17 @@ $> = $stderr initialize_request = JSON.parse(raw_initialize, symbolize_names: true) if raw_initialize -RubyLsp::Server.new( - setup_error: setup_error, - initialize_request: initialize_request, -).start +begin + RubyLsp::Server.new( + install_error: install_error, + setup_error: setup_error, + initialize_request: initialize_request, + ).start +rescue ArgumentError + # If the launcher is booting an outdated version of the server, then the initializer doesn't accept a keyword splat + # and we already read the initialize request from the stdin pipe. In this case, we need to process the initialize + # request manually and then start the main loop + server = RubyLsp::Server.new + server.process_message(initialize_request) + server.start +end diff --git a/lib/ruby_lsp/base_server.rb b/lib/ruby_lsp/base_server.rb index d487d0fbd..64ac34519 100644 --- a/lib/ruby_lsp/base_server.rb +++ b/lib/ruby_lsp/base_server.rb @@ -12,6 +12,7 @@ class BaseServer def initialize(**options) @test_mode = T.let(options[:test_mode], T.nilable(T::Boolean)) @setup_error = T.let(options[:setup_error], T.nilable(StandardError)) + @install_error = T.let(options[:install_error], T.nilable(StandardError)) @writer = T.let(Transport::Stdio::Writer.new, Transport::Stdio::Writer) @reader = T.let(Transport::Stdio::Reader.new, Transport::Stdio::Reader) @incoming_queue = T.let(Thread::Queue.new, Thread::Queue) diff --git a/lib/ruby_lsp/scripts/compose_bundle.rb b/lib/ruby_lsp/scripts/compose_bundle.rb index d54dfd89c..afd0d53c5 100644 --- a/lib/ruby_lsp/scripts/compose_bundle.rb +++ b/lib/ruby_lsp/scripts/compose_bundle.rb @@ -2,10 +2,10 @@ # frozen_string_literal: true def compose(raw_initialize) - require "ruby_lsp/setup_bundler" + require_relative "../setup_bundler" require "json" require "uri" - require "core_ext/uri" + require_relative "../../core_ext/uri" initialize_request = JSON.parse(raw_initialize, symbolize_names: true) workspace_uri = initialize_request.dig(:params, :workspaceFolders, 0, :uri) diff --git a/lib/ruby_lsp/server.rb b/lib/ruby_lsp/server.rb index 9fe2172ff..56144dbca 100644 --- a/lib/ruby_lsp/server.rb +++ b/lib/ruby_lsp/server.rb @@ -292,15 +292,21 @@ def run_initialize(message) global_state_notifications.each { |notification| send_message(notification) } if @setup_error - message = <<~MESSAGE - An error occurred while setting up Bundler. This may be due to a failure when installing dependencies. - The Ruby LSP will continue to run, but features related to the missing dependencies will be limited. - - Error: - #{@setup_error.full_message} - MESSAGE + send_message(Notification.telemetry( + type: "error", + errorMessage: @setup_error.message, + errorClass: @setup_error.class, + stack: @setup_error.backtrace&.join("\n"), + )) + end - send_message(Notification.window_log_message(message, type: Constant::MessageType::ERROR)) + if @install_error + send_message(Notification.telemetry( + type: "error", + errorMessage: @install_error.message, + errorClass: @install_error.class, + stack: @install_error.backtrace&.join("\n"), + )) end end diff --git a/lib/ruby_lsp/setup_bundler.rb b/lib/ruby_lsp/setup_bundler.rb index 11e5a6e47..d75dace95 100644 --- a/lib/ruby_lsp/setup_bundler.rb +++ b/lib/ruby_lsp/setup_bundler.rb @@ -3,6 +3,9 @@ require "sorbet-runtime" require "bundler" +require "bundler/cli" +require "bundler/cli/install" +require "bundler/cli/update" require "fileutils" require "pathname" require "digest" @@ -48,6 +51,7 @@ def initialize(project_path, **options) @custom_lockfile = T.let(@custom_dir + (@lockfile&.basename || "Gemfile.lock"), Pathname) @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) dependencies, bundler_version = load_dependencies @dependencies = T.let(dependencies, T::Hash[String, T.untyped]) @@ -187,6 +191,55 @@ def run_bundle_install(bundle_gemfile = @gemfile) env["BUNDLE_PATH"] = File.expand_path(env["BUNDLE_PATH"], @project_path) end + return run_bundle_install_through_command(env) unless @launcher + + # This same check happens conditionally when running through the command. For invoking the CLI directly, it's + # important that we ensure the Bundler version is set to avoid restarts + if @bundler_version + env["BUNDLER_VERSION"] = @bundler_version.to_s + install_bundler_if_needed + end + + begin + run_bundle_install_directly(env) + # If no error occurred, then clear previous errors + @error_path.delete if @error_path.exist? + $stderr.puts("Ruby LSP> Composed bundle setup complete") + rescue => e + # Write the error object to a file so that we can read it from the parent process + @error_path.write(Marshal.dump(e)) + end + + env + end + + sig { params(env: T::Hash[String, String]).returns(T::Hash[String, String]) } + def run_bundle_install_directly(env) + T.unsafe(ENV).merge!(env) + + unless should_bundle_update? + RubyVM::YJIT.enable if defined?(RubyVM::YJIT.enable) + Bundler::CLI::Install.new({}).run + + return env + end + + # If any of `ruby-lsp`, `ruby-lsp-rails` or `debug` are not in the Gemfile, try to update them to the latest + # version + gems = [] + gems << "ruby-lsp" unless @dependencies["ruby-lsp"] + gems << "debug" unless @dependencies["debug"] + gems << "ruby-lsp-rails" if @rails_app && !@dependencies["ruby-lsp-rails"] + + RubyVM::YJIT.enable if defined?(RubyVM::YJIT.enable) + Bundler::CLI::Update.new({}, gems).run + + @last_updated_path.write(Time.now.iso8601) + env + end + + sig { params(env: T::Hash[String, String]).returns(T::Hash[String, String]) } + def run_bundle_install_through_command(env) base_bundle = base_bundle_command(env) # If `ruby-lsp` and `debug` (and potentially `ruby-lsp-rails`) are already in the Gemfile, then we shouldn't try diff --git a/lib/ruby_lsp/utils.rb b/lib/ruby_lsp/utils.rb index 630c8b168..b73e637be 100644 --- a/lib/ruby_lsp/utils.rb +++ b/lib/ruby_lsp/utils.rb @@ -79,6 +79,14 @@ def window_log_message(message, type: Constant::MessageType::LOG) params: Interface::LogMessageParams.new(type: type, message: message), ) end + + sig { params(data: T::Hash[Symbol, T.untyped]).returns(Notification) } + def telemetry(data) + new( + method: "telemetry/event", + params: data, + ) + end end extend T::Sig diff --git a/sorbet/rbi/shims/bundler.rbi b/sorbet/rbi/shims/bundler.rbi index 16b184e08..f48604ba0 100644 --- a/sorbet/rbi/shims/bundler.rbi +++ b/sorbet/rbi/shims/bundler.rbi @@ -1,6 +1,26 @@ # typed: true -class Bundler::Settings - sig { params(name: String).returns(String) } - def self.key_for(name); end +module Bundler + class Settings + sig { params(name: String).returns(String) } + def self.key_for(name); end + end + + module CLI + class Install + sig { params(options: T::Hash[String, T.untyped]).void } + def initialize(options); end + + sig { void } + def run; end + end + + class Update + sig { params(options: T::Hash[String, T.untyped], gems: T::Array[String]).void } + def initialize(options, gems); end + + sig { void } + def run; end + end + end end diff --git a/test/integration_test.rb b/test/integration_test.rb index 939d2af33..7fb93ca77 100644 --- a/test/integration_test.rb +++ b/test/integration_test.rb @@ -82,9 +82,7 @@ def test_launch_mode_with_no_gemfile skip("CI only") unless ENV["CI"] in_temp_dir do |dir| - Bundler.with_unbundled_env do - launch(dir) - end + launch(dir) end end @@ -97,9 +95,7 @@ def test_launch_mode_with_missing_lockfile gem "stringio" RUBY - Bundler.with_unbundled_env do - launch(dir) - end + launch(dir) end end @@ -130,9 +126,7 @@ def test_launch_mode_with_full_bundle LOCKFILE File.write(File.join(dir, "Gemfile.lock"), lockfile_contents) - Bundler.with_unbundled_env do - launch(dir) - end + launch(dir) end end @@ -143,9 +137,9 @@ def test_launch_mode_with_no_gemfile_and_bundle_path Bundler.with_unbundled_env do system("bundle config --local path #{File.join("vendor", "bundle")}") assert_path_exists(File.join(dir, ".bundle", "config")) - - launch(dir) end + + launch(dir) end end diff --git a/test/setup_bundler_test.rb b/test/setup_bundler_test.rb index 4c51d493a..36b97d383 100644 --- a/test/setup_bundler_test.rb +++ b/test/setup_bundler_test.rb @@ -610,6 +610,59 @@ def test_sets_bundler_version_to_avoid_reloads end end + def test_invoke_cli_calls_bundler_directly_for_install + Dir.mktmpdir do |dir| + Dir.chdir(dir) do + File.write(File.join(dir, "gems.rb"), <<~GEMFILE) + source "https://rubygems.org" + gem "irb" + GEMFILE + + Bundler.with_unbundled_env do + capture_subprocess_io do + system("bundle install") + + mock_object = mock("install") + mock_object.expects(:run) + Bundler::CLI::Install.expects(:new).with({}).returns(mock_object) + RubyLsp::SetupBundler.new(dir, launcher: true).setup! + end + end + end + end + end + + def test_invoke_cli_calls_bundler_directly_for_update + Dir.mktmpdir do |dir| + Dir.chdir(dir) do + File.write(File.join(dir, "Gemfile"), <<~GEMFILE) + source "https://rubygems.org" + gem "rdoc" + GEMFILE + + capture_subprocess_io do + Bundler.with_unbundled_env do + # Run bundle install to generate the lockfile + system("bundle install") + + # Run the script once to generate a custom bundle + run_script(dir) + end + end + + capture_subprocess_io do + Bundler.with_unbundled_env do + mock_object = mock("update") + mock_object.expects(:run) + require "bundler/cli/update" + Bundler::CLI::Update.expects(:new).with({}, ["ruby-lsp", "debug"]).returns(mock_object) + RubyLsp::SetupBundler.new(dir, launcher: true).setup! + end + end + end + end + end + private def with_default_external_encoding(encoding, &block)