Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set BUNDLER_VERSION to avoid version mismatch restarts #2658

Merged
merged 1 commit into from
Oct 11, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading