From added7729768fd8e47bb975defa133323809eb08 Mon Sep 17 00:00:00 2001 From: Vinicius Stock Date: Sat, 26 Oct 2024 14:19:29 -0400 Subject: [PATCH] Try timeout --- exe/ruby-lsp-launcher | 24 +++++++++---- .../requests/support/rubocop_runner.rb | 12 ++++++- .../scripts/compose_bundle_windows.rb | 1 - test/integration_test.rb | 35 +++++++++++-------- 4 files changed, 49 insertions(+), 23 deletions(-) diff --git a/exe/ruby-lsp-launcher b/exe/ruby-lsp-launcher index 2cdf97dbc..34b578a84 100755 --- a/exe/ruby-lsp-launcher +++ b/exe/ruby-lsp-launcher @@ -22,9 +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? + # 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( - ENV.to_hash, Gem.ruby, + *load_path, File.expand_path("../lib/ruby_lsp/scripts/compose_bundle_windows.rb", __dir__), raw_initialize, ) @@ -35,8 +41,12 @@ else 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 # We can't require `bundler/setup` because that file prematurely exits the process if setup fails. However, we can't @@ -61,11 +71,11 @@ rescue StandardError => e # 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 + $stderr.puts("Failed to set up composed Bundle\n#{e.full_message}") - # 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 + # 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__)) end error_path = File.join(".ruby-lsp", "install_error") diff --git a/lib/ruby_lsp/requests/support/rubocop_runner.rb b/lib/ruby_lsp/requests/support/rubocop_runner.rb index cacbc3cf1..238e933b3 100644 --- a/lib/ruby_lsp/requests/support/rubocop_runner.rb +++ b/lib/ruby_lsp/requests/support/rubocop_runner.rb @@ -1,16 +1,26 @@ # typed: strict # frozen_string_literal: true +# If there's no top level Gemfile, don't load RuboCop from a global installation +begin + Bundler.with_original_env { Bundler.default_gemfile } +rescue Bundler::GemfileNotFound + return +end + +# Ensure that RuboCop is available begin require "rubocop" rescue LoadError return end +# Ensure that RuboCop is at least version 1.4.0 begin gem("rubocop", ">= 1.4.0") rescue LoadError - raise StandardError, "Incompatible RuboCop version. Ruby LSP requires >= 1.4.0" + $stderr.puts "Incompatible RuboCop version. Ruby LSP requires >= 1.4.0" + return end if RuboCop.const_defined?(:LSP) # This condition will be removed when requiring RuboCop >= 1.61. 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/test/integration_test.rb b/test/integration_test.rb index 83fcd2562..1ed7753c1 100644 --- a/test/integration_test.rb +++ b/test/integration_test.rb @@ -140,9 +140,6 @@ 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")) - end - - Bundler.with_unbundled_env do launch(dir) end end @@ -151,14 +148,19 @@ def test_launch_mode_with_no_gemfile_and_bundle_path private def launch(workspace_path) - load_path = $-I.map do |p| - "-I#{File.expand_path(p)}" - end.join(" ") + 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( - ENV.to_hash, Gem.ruby, - load_path, + *load_path, File.join(@root, "exe", "ruby-lsp-launcher"), ) stdin.sync = true @@ -180,14 +182,19 @@ def launch(workspace_path) send_message(stdin, { id: 2, method: "shutdown" }) send_message(stdin, { method: "exit" }) - begin - Process.wait(wait_thr.pid) - rescue Errno::ECHILD - nil + # Wait until the process exits + wait_thr.join + + # 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 - wait_thr.join - refute_predicate(wait_thr, :alive?) 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"))