Skip to content

Commit

Permalink
Set BUNDLER_VERSION to avoid version mismatch restarts (#2658)
Browse files Browse the repository at this point in the history
Use locked Bundler version for composed bundle
  • Loading branch information
vinistock authored Oct 11, 2024
1 parent 9104549 commit 72f019a
Show file tree
Hide file tree
Showing 4 changed files with 90 additions and 13 deletions.
8 changes: 7 additions & 1 deletion exe/ruby-lsp
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,13 @@ if ENV["BUNDLE_GEMFILE"].nil?
exit(78)
end

exit exec(env, "bundle exec ruby-lsp #{original_args.join(" ")}")
base_bundle = if env["BUNDLER_VERSION"]
"bundle _#{env["BUNDLER_VERSION"]}_"
else
"bundle"
end

exit exec(env, "#{base_bundle} exec ruby-lsp #{original_args.join(" ")}")
end

require "ruby_lsp/load_sorbet"
Expand Down
38 changes: 30 additions & 8 deletions lib/ruby_lsp/setup_bundler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ 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)

@dependencies = T.let(load_dependencies, T::Hash[String, T.untyped])
dependencies, bundler_version = load_dependencies
@dependencies = T.let(dependencies, T::Hash[String, T.untyped])
@bundler_version = T.let(bundler_version, T.nilable(Gem::Version))
@rails_app = T.let(rails_app?, T::Boolean)
@retry = T.let(false, T::Boolean)
end
Expand Down Expand Up @@ -156,14 +158,15 @@ def write_custom_gemfile
@custom_gemfile.write(content) unless @custom_gemfile.exist? && @custom_gemfile.read == content
end

sig { returns(T::Hash[String, T.untyped]) }
sig { returns([T::Hash[String, T.untyped], T.nilable(Gem::Version)]) }
def load_dependencies
return {} unless @lockfile&.exist?
return [{}, nil] unless @lockfile&.exist?

# We need to parse the Gemfile.lock manually here. If we try to do `bundler/setup` to use something more
# convenient, we may end up with issues when the globally installed `ruby-lsp` version mismatches the one included
# in the `Gemfile`
dependencies = Bundler::LockfileParser.new(@lockfile.read).dependencies
lockfile_parser = Bundler::LockfileParser.new(@lockfile.read)
dependencies = lockfile_parser.dependencies

# When working on a gem, the `ruby-lsp` might be listed as a dependency in the gemspec. We need to make sure we
# check those as well or else we may get version mismatch errors. Notice that bundler allows more than one
Expand All @@ -172,7 +175,7 @@ def load_dependencies
dependencies.merge!(Bundler.load_gemspec(path).dependencies.to_h { |dep| [dep.name, dep] })
end

dependencies
[dependencies, lockfile_parser.bundler_version]
end

sig { params(bundle_gemfile: T.nilable(Pathname)).returns(T::Hash[String, String]) }
Expand All @@ -188,6 +191,16 @@ def run_bundle_install(bundle_gemfile = @gemfile)
env["BUNDLE_PATH"] = File.expand_path(env["BUNDLE_PATH"], @project_path)
end

# If there's a Bundler version locked, then we need to use that one to run bundle commands, so that the composed
# lockfile is also locked to the same version. This avoids Bundler restarts on version mismatches
base_bundle = if @bundler_version
env["BUNDLER_VERSION"] = @bundler_version.to_s
install_bundler_if_needed
"bundle _#{@bundler_version}_"
else
"bundle"
end

# If `ruby-lsp` and `debug` (and potentially `ruby-lsp-rails`) are already in the Gemfile, then we shouldn't try
# to upgrade them or else we'll produce undesired source control changes. If the custom bundle was just created
# and any of `ruby-lsp`, `ruby-lsp-rails` or `debug` weren't a part of the Gemfile, then we need to run `bundle
Expand All @@ -196,13 +209,13 @@ def run_bundle_install(bundle_gemfile = @gemfile)

# When not updating, we run `(bundle check || bundle install)`
# When updating, we run `((bundle check && bundle update ruby-lsp debug) || bundle install)`
command = +"(bundle check"
command = +"(#{base_bundle} check"

if should_bundle_update?
# If any of `ruby-lsp`, `ruby-lsp-rails` or `debug` are not in the Gemfile, try to update them to the latest
# version
command.prepend("(")
command << " && bundle update "
command << " && #{base_bundle} update "
command << "ruby-lsp " unless @dependencies["ruby-lsp"]
command << "debug " unless @dependencies["debug"]
command << "ruby-lsp-rails " if @rails_app && !@dependencies["ruby-lsp-rails"]
Expand All @@ -212,7 +225,7 @@ def run_bundle_install(bundle_gemfile = @gemfile)
@last_updated_path.write(Time.now.iso8601)
end

command << " || bundle install) "
command << " || #{base_bundle} install) "

# Redirect stdout to stderr to prevent going into an infinite loop. The extension might confuse stdout output with
# responses
Expand Down Expand Up @@ -259,6 +272,15 @@ def bundler_settings_as_env
end
end

sig { void }
def install_bundler_if_needed
# Try to find the bundler version specified in the lockfile in installed gems. If not found, install it
requirement = Gem::Requirement.new(@bundler_version.to_s)
return if Gem::Specification.any? { |s| s.name == "bundler" && requirement =~ s.version }

Gem.install("bundler", @bundler_version.to_s)
end

sig { returns(T::Boolean) }
def should_bundle_update?
# If `ruby-lsp`, `ruby-lsp-rails` and `debug` are in the Gemfile, then we shouldn't try to upgrade them or else it
Expand Down
8 changes: 8 additions & 0 deletions sorbet/rbi/shims/rubygems.rbi
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# typed: true

class Gem::Specification
class << self
sig { params(block: T.proc.params(spec: Gem::Specification).returns(T::Boolean)).returns(T::Boolean) }
def any?(&block); end
end
end
49 changes: 45 additions & 4 deletions test/setup_bundler_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ def test_does_not_copy_gemfile_lock_when_not_modified
Bundler.with_unbundled_env do
stub_bundle_with_env(
bundle_env(dir, ".ruby-lsp/Gemfile"),
"((bundle check && bundle update ruby-lsp debug) || bundle install) 1>&2",
/((bundle _[\d\.]+_ check && bundle _[\d\.]+_ update ruby-lsp debug) || bundle _[\d\.]+_ install) 1>&2/,
)

FileUtils.expects(:cp).never
Expand Down Expand Up @@ -593,6 +593,44 @@ def test_uses_correct_bundler_env_when_there_is_bundle_config
end
end

def test_sets_bundler_version_to_avoid_reloads
Dir.mktmpdir do |dir|
Dir.chdir(dir) do
# Write the main Gemfile and lockfile with valid versions
File.write(File.join(dir, "Gemfile"), <<~GEMFILE)
source "https://rubygems.org"
gem "stringio"
GEMFILE

lockfile_contents = <<~LOCKFILE
GEM
remote: https://rubygems.org/
specs:
stringio (3.1.0)
PLATFORMS
arm64-darwin-23
ruby
DEPENDENCIES
stringio
BUNDLED WITH
2.5.7
LOCKFILE
File.write(File.join(dir, "Gemfile.lock"), lockfile_contents)

Bundler.with_unbundled_env do
env = run_script(dir)
assert_equal("2.5.7", env["BUNDLER_VERSION"])
end

lockfile_parser = Bundler::LockfileParser.new(File.read(File.join(dir, ".ruby-lsp", "Gemfile.lock")))
assert_equal("2.5.7", lockfile_parser.bundler_version.to_s)
end
end
end

private

def with_default_external_encoding(encoding, &block)
Expand Down Expand Up @@ -635,10 +673,13 @@ def run_script(path = Dir.pwd, expected_path: nil, **options)

# This method needs to be called inside the `Bundler.with_unbundled_env` block IF the command you want to test is
# inside it.
def stub_bundle_with_env(env, command = "(bundle check || bundle install) 1>&2")
def stub_bundle_with_env(
env,
command = /(bundle check _[\d\.]+_ || bundle _[\d\.]+_ install) 1>&2/
)
Object.any_instance.expects(:system).with do |actual_env, actual_command|
actual_env.delete_if { |k, _v| k.start_with?("BUNDLE_PKGS") }
actual_env.all? { |k, v| env[k] == v } && actual_command == command
actual_env.delete_if { |k, _v| k.start_with?("BUNDLE_PKGS") || k == "BUNDLER_VERSION" }
actual_env.all? { |k, v| env[k] == v } && actual_command.match?(command)
end.returns(true)
end

Expand Down

0 comments on commit 72f019a

Please sign in to comment.