diff --git a/exe/ruby-lsp-launcher b/exe/ruby-lsp-launcher index 3e8f49d9f..44b915421 100755 --- a/exe/ruby-lsp-launcher +++ b/exe/ruby-lsp-launcher @@ -16,58 +16,78 @@ headers = $stdin.gets("\r\n\r\n") content_length = headers[/Content-Length: (\d+)/i, 1].to_i raw_initialize = $stdin.read(content_length) -bundle_gemfile_file = File.join(".ruby-lsp", "bundle_gemfile") -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) + # Since we can't fork on Windows and spawn won't carry over the existing load paths, we need to explicitly pass that + # down to the child process or else requiring gems during composing the bundle will fail + load_path = $LOAD_PATH.flat_map do |path| + ["-I", File.expand_path(path)] + end + + Process.spawn( + Gem.ruby, + *load_path, + 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 -# Wait until the composed Bundle is finished -Process.wait(pid) +begin + # Wait until the composed Bundle is finished + Process.wait(pid) +rescue Errno::ECHILD + # In theory, the child process can finish before we even get to the wait call, but that is not an error +end begin + bundle_env_path = File.join(".ruby-lsp", "bundle_env") # We can't require `bundler/setup` because that file prematurely exits the process if setup fails. However, we can't # simply require bundler either because the version required might conflict with the one locked in the composed # bundle. We need the composed bundle sub-process to inform us of the locked Bundler version, so that we can then # activate the right spec and require the exact Bundler version required by the app - if File.exist?(locked_bundler_version_file) - locked_bundler_version = File.read(locked_bundler_version_file) - Gem::Specification.find_by_name("bundler", locked_bundler_version).activate - end + if File.exist?(bundle_env_path) + env = File.readlines(bundle_env_path).to_h { |line| line.chomp.split("=", 2) } + ENV.merge!(env) - require "bundler" - Bundler.ui.level = :silent + if env["BUNDLER_VERSION"] + Gem::Specification.find_by_name("bundler", env["BUNDLER_VERSION"]).activate + end - # The composed bundle logic informs the main process about which Gemfile to setup Bundler with - if File.exist?(bundle_gemfile_file) - ENV["BUNDLE_GEMFILE"] = File.read(bundle_gemfile_file) + require "bundler" + Bundler.ui.level = :silent Bundler.setup + $stderr.puts("Composed Bundle set up successfully") end rescue StandardError => e # If installing gems failed for any reason, we don't want to exit the process prematurely. We can still provide most # features in a degraded mode. We simply save the error so that we can report to the user that certain gems might be # missing, but we respect the LSP life cycle setup_error = e - - # If we failed to set up the bundle, the minimum we need is to have our own dependencies activated so that we can - # require the gems the LSP depends on. If even that fails, then there's no way we can continue to run the language - # server - Gem::Specification.find_by_name("ruby-lsp").activate + $stderr.puts("Failed to set up composed Bundle\n#{e.full_message}") + + # If Bundler.setup fails, we need to restore the original $LOAD_PATH so that we can still require the Ruby LSP server + # in degraded mode + $LOAD_PATH.unshift(File.expand_path("../lib", __dir__)) +ensure + require "fileutils" + FileUtils.rm(bundle_env_path) if File.exist?(bundle_env_path) 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 +111,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..ef58b16ef 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) @@ -13,6 +13,8 @@ def compose(raw_initialize) workspace_path ||= Dir.pwd env = RubyLsp::SetupBundler.new(workspace_path, launcher: true).setup! - File.write(File.join(".ruby-lsp", "bundle_gemfile"), env["BUNDLE_GEMFILE"]) - File.write(File.join(".ruby-lsp", "locked_bundler_version"), env["BUNDLER_VERSION"]) if env["BUNDLER_VERSION"] + File.write( + File.join(".ruby-lsp", "bundle_env"), + env.map { |k, v| "#{k}=#{v}" }.join("\n"), + ) end diff --git a/lib/ruby_lsp/scripts/compose_bundle_windows.rb b/lib/ruby_lsp/scripts/compose_bundle_windows.rb index 3be82ad01..3bdc94115 100644 --- a/lib/ruby_lsp/scripts/compose_bundle_windows.rb +++ b/lib/ruby_lsp/scripts/compose_bundle_windows.rb @@ -1,7 +1,6 @@ # typed: strict # frozen_string_literal: true -$LOAD_PATH.unshift(File.expand_path("../../../lib", __dir__)) require_relative "compose_bundle" # When this is invoked on Windows, we pass the raw initialize as an argument to this script. On other platforms, we diff --git a/lib/ruby_lsp/server.rb b/lib/ruby_lsp/server.rb index 7e6b86a3c..09d9eefd3 100644 --- a/lib/ruby_lsp/server.rb +++ b/lib/ruby_lsp/server.rb @@ -140,6 +140,11 @@ def process_message(message) sig { params(include_project_addons: T::Boolean).void } def load_addons(include_project_addons: true) + # If invoking Bundler.setup failed, then the load path will not be configured properly and trying to load add-ons + # with Gem.find_files will find every single version installed of an add-on, leading to requiring several + # different versions of the same files. We cannot load add-ons if Bundler.setup failed + return if @setup_error + errors = Addon.load_addons(@global_state, @outgoing_queue, include_project_addons: include_project_addons) if errors.any? @@ -292,15 +297,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 @@ -309,20 +320,22 @@ def run_initialized load_addons RubyVM::YJIT.enable if defined?(RubyVM::YJIT.enable) - if defined?(Requests::Support::RuboCopFormatter) - begin - @global_state.register_formatter("rubocop", Requests::Support::RuboCopFormatter.new) - rescue RuboCop::Error => e - # The user may have provided unknown config switches in .rubocop or - # is trying to load a non-existent config file. - send_message(Notification.window_show_message( - "RuboCop configuration error: #{e.message}. Formatting will not be available.", - type: Constant::MessageType::ERROR, - )) + unless @setup_error + if defined?(Requests::Support::RuboCopFormatter) + begin + @global_state.register_formatter("rubocop", Requests::Support::RuboCopFormatter.new) + rescue RuboCop::Error => e + # The user may have provided unknown config switches in .rubocop or + # is trying to load a non-existent config file. + send_message(Notification.window_show_message( + "RuboCop configuration error: #{e.message}. Formatting will not be available.", + type: Constant::MessageType::ERROR, + )) + end + end + if defined?(Requests::Support::SyntaxTreeFormatter) + @global_state.register_formatter("syntax_tree", Requests::Support::SyntaxTreeFormatter.new) end - end - if defined?(Requests::Support::SyntaxTreeFormatter) - @global_state.register_formatter("syntax_tree", Requests::Support::SyntaxTreeFormatter.new) end perform_initial_indexing @@ -1150,6 +1163,7 @@ def end_progress(id) sig { void } def check_formatter_is_available + return if @setup_error # Warn of an unavailable `formatter` setting, e.g. `rubocop` on a project which doesn't have RuboCop. # Syntax Tree will always be available via Ruby LSP so we don't need to check for it. return unless @global_state.formatter == "rubocop" diff --git a/lib/ruby_lsp/setup_bundler.rb b/lib/ruby_lsp/setup_bundler.rb index 20237cd06..7716c2ced 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" @@ -49,6 +52,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]) @@ -108,17 +112,19 @@ def setup! def custom_bundle_dependencies @custom_bundle_dependencies ||= T.let( begin + original_bundle_gemfile = ENV["BUNDLE_GEMFILE"] + if @custom_lockfile.exist? ENV["BUNDLE_GEMFILE"] = @custom_gemfile.to_s Bundler::LockfileParser.new(@custom_lockfile.read).dependencies else {} end + ensure + ENV["BUNDLE_GEMFILE"] = original_bundle_gemfile end, T.nilable(T::Hash[String, T.untyped]), ) - ensure - ENV.delete("BUNDLE_GEMFILE") end sig { void } @@ -188,6 +194,52 @@ 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 installation 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) + RubyVM::YJIT.enable if defined?(RubyVM::YJIT.enable) + T.unsafe(ENV).merge!(env) + + unless should_bundle_update? + Bundler::CLI::Install.new({}).run + correct_relative_remote_paths if @custom_lockfile.exist? + return env + end + + # Try to auto upgrade the gems we depend on, unless they are in the Gemfile as that would result in undesired + # source control changes + gems = ["ruby-lsp", "debug", "prism"].reject { |dep| @dependencies[dep] } + gems << "ruby-lsp-rails" if @rails_app && !@dependencies["ruby-lsp-rails"] + + Bundler::CLI::Update.new({ conservative: true }, gems).run + correct_relative_remote_paths if @custom_lockfile.exist? + @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..f8670e401 100644 --- a/test/integration_test.rb +++ b/test/integration_test.rb @@ -2,8 +2,13 @@ # frozen_string_literal: true require "test_helper" +require "open3" class IntegrationTest < Minitest::Test + def setup + @root = Bundler.root + end + def test_ruby_lsp_doctor_works skip("CI only") unless ENV["CI"] @@ -79,8 +84,6 @@ def test_avoids_bundler_version_if_local_bin_is_in_path end 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) @@ -89,8 +92,6 @@ def test_launch_mode_with_no_gemfile end def test_launch_mode_with_missing_lockfile - skip("CI only") unless ENV["CI"] - in_temp_dir do |dir| File.write(File.join(dir, "Gemfile"), <<~RUBY) source "https://rubygems.org" @@ -104,8 +105,6 @@ def test_launch_mode_with_missing_lockfile end def test_launch_mode_with_full_bundle - skip("CI only") unless ENV["CI"] - in_temp_dir do |dir| File.write(File.join(dir, "Gemfile"), <<~RUBY) source "https://rubygems.org" @@ -137,13 +136,10 @@ def test_launch_mode_with_full_bundle end def test_launch_mode_with_no_gemfile_and_bundle_path - skip("CI only") unless ENV["CI"] - in_temp_dir do |dir| 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 end @@ -152,7 +148,29 @@ def test_launch_mode_with_no_gemfile_and_bundle_path private def launch(workspace_path) - initialize_request = { + specification = Gem::Specification.find_by_name("ruby-lsp") + paths = [specification.full_gem_path] + paths.concat(specification.dependencies.map { |dep| dep.to_spec.full_gem_path }) + + load_path = $LOAD_PATH.filter_map do |path| + next unless paths.any? { |gem_path| path.start_with?(gem_path) } || !path.start_with?(Bundler.bundle_path.to_s) + + ["-I", File.expand_path(path)] + end.uniq.flatten + + stdin, stdout, stderr, wait_thr = T.unsafe(Open3).popen3( + Gem.ruby, + *load_path, + File.join(@root, "exe", "ruby-lsp-launcher"), + ) + stdin.sync = true + stdin.binmode + stdout.sync = true + stdout.binmode + stderr.sync = true + stderr.binmode + + send_message(stdin, { id: 1, method: "initialize", params: { @@ -160,34 +178,32 @@ def launch(workspace_path) capabilities: { general: { positionEncodings: ["utf-8"] } }, workspaceFolders: [{ uri: URI::Generic.from_path(path: workspace_path).to_s }], }, - }.to_json - - $stdin.expects(:gets).with("\r\n\r\n").once.returns("Content-Length: #{initialize_request.bytesize}") - $stdin.expects(:read).with(initialize_request.bytesize).once.returns(initialize_request) - - # Make `new` return a mock that raises so that we don't print to stdout and stop immediately after boot - server_object = mock("server") - server_object.expects(:start).once.raises(StandardError.new("stop")) - RubyLsp::Server.expects(:new).returns(server_object) - - # We load the launcher binary in the same process as the tests are running. We cannot try to re-activate a different - # Bundler version, because that throws an error - if File.exist?(File.join(workspace_path, "Gemfile.lock")) - spec_mock = mock("specification") - spec_mock.expects(:activate).once - Gem::Specification.expects(:find_by_name).with do |name, version| - name == "bundler" && !version.empty? - end.returns(spec_mock) - end + }) + send_message(stdin, { id: 2, method: "shutdown" }) + send_message(stdin, { method: "exit" }) - # Verify that we are setting up the bundle, but there's no actual need to do it - Bundler.expects(:setup).once + # Wait until the process exits + wait_thr.join - assert_raises(StandardError) do - load(File.expand_path("../exe/ruby-lsp-launcher", __dir__)) + # If the child process failed, it is really difficult to diagnose what's happening unless we read what was printed + # to stderr + unless T.unsafe(wait_thr.value).success? + require "timeout" + + Timeout.timeout(5) do + flunk("Process failed\n#{stderr.read}") + end end - assert_path_exists(File.join(workspace_path, ".ruby-lsp", "bundle_gemfile")) + assert_path_exists(File.join(workspace_path, ".ruby-lsp", "Gemfile")) + assert_path_exists(File.join(workspace_path, ".ruby-lsp", "Gemfile.lock")) + refute_path_exists(File.join(workspace_path, ".ruby-lsp", "install_error")) + end + + def send_message(stdin, message) + json_message = message.to_json + stdin.write("Content-Length: #{json_message.bytesize}\r\n\r\n#{json_message}") + stdin.flush end def in_temp_dir diff --git a/test/setup_bundler_test.rb b/test/setup_bundler_test.rb index 53e5235ab..7bf9492b7 100644 --- a/test/setup_bundler_test.rb +++ b/test/setup_bundler_test.rb @@ -658,6 +658,62 @@ 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_install = mock("install") + mock_install.expects(:run) + Bundler::CLI::Install.expects(:new).with({}).returns(mock_install) + 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_update = mock("update") + mock_update.expects(:run) + require "bundler/cli/update" + Bundler::CLI::Update.expects(:new).with( + { conservative: true }, + ["ruby-lsp", "debug", "prism"], + ).returns(mock_update) + RubyLsp::SetupBundler.new(dir, launcher: true).setup! + end + end + end + end + end + private def with_default_external_encoding(encoding, &block)