Move server update after launching server (#3555)

vinistock · web-flow · commit c50572cbf5a1 · 2025-06-26T14:30:19.000Z
Update server after launching
diff --git a/lib/ruby_lsp/server.rb b/lib/ruby_lsp/server.rb
@@ -362,6 +362,7 @@ def run_initialized
 
       perform_initial_indexing
       check_formatter_is_available
+      update_server if @global_state.enabled_feature?(:launcher)
     end
 
     #: (Hash[Symbol, untyped] message) -> void
@@ -1406,8 +1407,38 @@ def compose_bundle(message)
 
       # 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
+      launch_bundle_compose("Recomposing the bundle ahead of restart") do |stderr, status|
+        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\n#{stderr}"),
+          )
+        end
+      end
+    end
+
+    #: -> void
+    def update_server
+      return unless File.exist?(File.join(@global_state.workspace_path, ".ruby-lsp", "needs_update"))
+
+      launch_bundle_compose("Trying to update server") do |stderr, status|
+        if status&.exitstatus == 0
+          send_log_message("Successfully updated the server")
+        else
+          send_log_message("Failed to update server\n#{stderr}", type: Constant::MessageType::ERROR)
+        end
+      end
+    end
+
+    #: (String) { (IO, Process::Status?) -> void } -> Thread
+    def launch_bundle_compose(log, &block)
       Thread.new do
-        send_log_message("Recomposing the bundle ahead of restart")
+        send_log_message(log)
 
         _stdout, stderr, status = Bundler.with_unbundled_env do
           Open3.capture3(
@@ -1422,17 +1453,7 @@ def compose_bundle(message)
           )
         end
 
-        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\n#{stderr}"),
-          )
-        end
+        block.call(stderr, status)
       end
     end
 
diff --git a/lib/ruby_lsp/setup_bundler.rb b/lib/ruby_lsp/setup_bundler.rb
@@ -65,6 +65,7 @@ def initialize(project_path, **options)
       @bundler_version = bundler_version #: Gem::Version?
       @rails_app = rails_app? #: bool
       @retry = false #: bool
+      @needs_update_path = @custom_dir + "needs_update" #: Pathname
     end
 
     # Sets up the composed bundle and returns the `BUNDLE_GEMFILE`, `BUNDLE_PATH` and `BUNDLE_APP_CONFIG` that should be
@@ -260,31 +261,60 @@ def run_bundle_install(bundle_gemfile = @gemfile)
     #: (Hash[String, String] env, ?force_install: bool) -> Hash[String, String]
     def run_bundle_install_directly(env, force_install: false)
       RubyVM::YJIT.enable if defined?(RubyVM::YJIT.enable)
+      return update(env) if @needs_update_path.exist?
 
       # The ENV can only be merged after checking if an update is required because we depend on the original value of
       # ENV["BUNDLE_GEMFILE"], which gets overridden after the merge
-      should_update = should_bundle_update?
-      ENV #: as untyped
-        .merge!(env)
+      FileUtils.touch(@needs_update_path) if should_bundle_update?
+      ENV.merge!(env)
 
-      unless should_update && !force_install
-        Bundler::CLI::Install.new({ "no-cache" => true }).run
-        correct_relative_remote_paths if @custom_lockfile.exist?
-        return env
+      $stderr.puts("Ruby LSP> Checking if the composed bundle is satisfied...")
+      missing_gems = bundle_check
+
+      unless missing_gems.empty?
+        $stderr.puts(<<~MESSAGE)
+          Ruby LSP> Running bundle install because the following gems are not installed:
+          #{missing_gems.map { |g| "#{g.name}: #{g.version}" }.join("\n")}
+        MESSAGE
+
+        bundle_install
       end
 
+      $stderr.puts("Ruby LSP> Bundle already satisfied")
+      env
+    rescue => e
+      $stderr.puts("Ruby LSP> Running bundle install because #{e.message}")
+      bundle_install
+      env
+    end
+
+    # Essentially the same as bundle check, but simplified
+    #: -> Array[Gem::Specification]
+    def bundle_check
+      definition = Bundler.definition
+      definition.validate_runtime!
+      definition.check!
+      definition.missing_specs
+    end
+
+    #: -> void
+    def bundle_install
+      Bundler::CLI::Install.new({ "no-cache" => true }).run
+      correct_relative_remote_paths if @custom_lockfile.exist?
+    end
+
+    #: (Hash[String, String]) -> Hash[String, String]
+    def update(env)
       # 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?
+      @needs_update_path.delete
       @last_updated_path.write(Time.now.iso8601)
       env
-    rescue Bundler::GemNotFound, Bundler::GitError
-      # If a gem is not installed, skip the upgrade and try to install it with a single retry
-      @retry ? env : run_bundle_install_directly(env, force_install: true)
     end
 
     #: (Hash[String, String] env) -> Hash[String, String]
diff --git a/test/setup_bundler_test.rb b/test/setup_bundler_test.rb
@@ -694,7 +694,10 @@ def test_invoke_cli_calls_bundler_directly_for_install
             mock_install = mock("install")
             mock_install.expects(:run)
             Bundler::CLI::Install.expects(:new).with({ "no-cache" => true }).returns(mock_install)
-            RubyLsp::SetupBundler.new(dir, launcher: true).setup!
+
+            compose = RubyLsp::SetupBundler.new(dir, launcher: true)
+            compose.expects(:bundle_check).raises(StandardError, "missing gems")
+            compose.setup!
           end
         end
       end
@@ -728,6 +731,8 @@ def test_invoke_cli_calls_bundler_directly_for_update
               { conservative: true },
               ["ruby-lsp", "debug", "prism"],
             ).returns(mock_update)
+
+            FileUtils.touch(File.join(dir, ".ruby-lsp", "needs_update"))
             RubyLsp::SetupBundler.new(dir, launcher: true).setup!
           end
         end
@@ -750,7 +755,9 @@ def test_progress_is_printed_to_stderr
           end
 
           stdout, stderr = capture_subprocess_io do
-            RubyLsp::SetupBundler.new(dir, launcher: true).setup!
+            compose = RubyLsp::SetupBundler.new(dir, launcher: true)
+            compose.expects(:bundle_check).raises(StandardError, "missing gems")
+            compose.setup!
           end
 
           assert_match(/Bundle complete! [\d]+ Gemfile dependencies, [\d]+ gems now installed/, stderr)
@@ -844,41 +851,6 @@ def test_is_resilient_to_gemfile_changes_in_the_middle_of_setup
     end
   end
 
-  def test_update_does_not_fail_if_gems_are_uninstalled
-    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
-            system("bundle install")
-            run_script(dir)
-
-            mock_update = mock("update")
-            mock_update.expects(:run).raises(Bundler::GemNotFound.new("rdoc"))
-            require "bundler/cli/update"
-            Bundler::CLI::Update.expects(:new).with(
-              { conservative: true },
-              ["ruby-lsp", "debug", "prism"],
-            ).returns(mock_update)
-
-            mock_install = mock("install")
-            mock_install.expects(:run)
-            require "bundler/cli/install"
-            Bundler::CLI::Install.expects(:new).with({ "no-cache" => true }).returns(mock_install)
-
-            RubyLsp::SetupBundler.new(dir, launcher: true).setup!
-          end
-        end
-
-        refute_path_exists(File.join(".ruby-lsp", "install_error"))
-      end
-    end
-  end
-
   def test_only_returns_environment_if_bundle_was_composed_ahead_of_time
     Dir.mktmpdir do |dir|
       Dir.chdir(dir) do
