Skip to content

Commit

Permalink
Attempt to re-generate custom bundle once on bundle failure (#1852)
Browse files Browse the repository at this point in the history
  • Loading branch information
@vinistock
vinistock authored Apr 8, 2024
1 parent 5d59a71 commit 687bb2c
Show file tree
Hide file tree
Showing 2 changed files with 105 additions and 17 deletions.
14 changes: 13 additions & 1 deletion lib/ruby_lsp/setup_bundler.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ class SetupBundler
extend T::Sig

class BundleNotLocked < StandardError; end
class BundleInstallFailure < StandardError; end

FOUR_HOURS = T.let(4 * 60 * 60, Integer)

Expand Down Expand Up @@ -49,6 +50,7 @@ def initialize(project_path, **options)
@last_updated_path = T.let(@custom_dir + "last_updated", Pathname)

@dependencies = T.let(load_dependencies, T::Hash[String, T.untyped])
@retry = T.let(false, T::Boolean)
end

# Sets up the custom bundle and returns the `BUNDLE_GEMFILE`, `BUNDLE_PATH` and `BUNDLE_APP_CONFIG` that should be
Expand Down Expand Up @@ -224,7 +226,17 @@ def run_bundle_install(bundle_gemfile = @gemfile)
# Add bundle update
$stderr.puts("Ruby LSP> Running bundle install for the custom bundle. This may take a while...")
$stderr.puts("Ruby LSP> Command: #{command}")
system(env, command)

# Try to run the bundle install or update command. If that fails, it normally means that the custom lockfile is in
# a bad state that no longer reflects the top level one. In that case, we can remove the whole directory, try
# another time and give up if it fails again
if !system(env, command) && !@retry && @custom_dir.exist?
@retry = true
@custom_dir.rmtree
$stderr.puts("Ruby LSP> Running bundle install failed. Trying to re-generate the custom bundle from scratch")
return setup!
end

[bundle_gemfile.to_s, expanded_path, env["BUNDLE_APP_CONFIG"]]
end

Expand Down
108 changes: 92 additions & 16 deletions test/setup_bundler_test.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@

class SetupBundlerTest < Minitest::Test
def test_does_nothing_if_both_ruby_lsp_and_debug_are_in_the_bundle
Object.any_instance.expects(:system).with(bundle_env, "(bundle check || bundle install) 1>&2")
Object.any_instance.expects(:system).with(bundle_env, "(bundle check || bundle install) 1>&2").returns(true)
Bundler::LockfileParser.any_instance.expects(:dependencies).returns({ "ruby-lsp" => true, "debug" => true })
run_script
refute_path_exists(".ruby-lsp")
end

def test_does_nothing_if_both_ruby_lsp_and_debug_are_in_the_bundle2
Object.any_instance.expects(:system).with(bundle_env, "(bundle check || bundle install) 1>&2")
Object.any_instance.expects(:system).with(bundle_env, "(bundle check || bundle install) 1>&2").returns(true)
Bundler::LockfileParser.any_instance.expects(:dependencies).returns({
"ruby-lsp" => true,
"rails" => true,
Expand All @@ -35,7 +35,7 @@ def test_removes_ruby_lsp_folder_if_both_gems_were_added_to_the_bundle
end

def test_in_a_rails_app_does_nothing_if_ruby_lsp_and_ruby_lsp_rails_and_debug_are_in_the_bundle
Object.any_instance.expects(:system).with(bundle_env, "(bundle check || bundle install) 1>&2")
Object.any_instance.expects(:system).with(bundle_env, "(bundle check || bundle install) 1>&2").returns(true)
Bundler::LockfileParser.any_instance.expects(:dependencies)
.returns({ "ruby-lsp" => true, "ruby-lsp-rails" => true, "debug" => true })
run_script
Expand All @@ -45,7 +45,7 @@ def test_in_a_rails_app_does_nothing_if_ruby_lsp_and_ruby_lsp_rails_and_debug_ar
end

def test_in_a_rails_app_removes_ruby_lsp_folder_if_all_gems_were_added_to_the_bundle
Object.any_instance.expects(:system).with(bundle_env, "(bundle check || bundle install) 1>&2")
Object.any_instance.expects(:system).with(bundle_env, "(bundle check || bundle install) 1>&2").returns(true)
Bundler::LockfileParser.any_instance.expects(:dependencies)
.returns({ "ruby-lsp" => true, "ruby-lsp-rails" => true, "debug" => true })
FileUtils.mkdir(".ruby-lsp")
Expand All @@ -56,7 +56,10 @@ def test_in_a_rails_app_removes_ruby_lsp_folder_if_all_gems_were_added_to_the_bu
end

def test_creates_custom_bundle
Object.any_instance.expects(:system).with(bundle_env(".ruby-lsp/Gemfile"), "(bundle check || bundle install) 1>&2")
Object.any_instance.expects(:system).with(
bundle_env(".ruby-lsp/Gemfile"),
"(bundle check || bundle install) 1>&2",
).returns(true)
Bundler::LockfileParser.any_instance.expects(:dependencies).returns({}).at_least_once
run_script

Expand All @@ -72,7 +75,10 @@ def test_creates_custom_bundle
end

def test_creates_custom_bundle_for_a_rails_app
Object.any_instance.expects(:system).with(bundle_env(".ruby-lsp/Gemfile"), "(bundle check || bundle install) 1>&2")
Object.any_instance.expects(:system).with(
bundle_env(".ruby-lsp/Gemfile"),
"(bundle check || bundle install) 1>&2",
).returns(true)
Bundler::LockfileParser.any_instance.expects(:dependencies).returns({ "rails" => true }).at_least_once
run_script

Expand Down Expand Up @@ -128,7 +134,7 @@ def test_changing_lockfile_causes_custom_bundle_to_be_rebuilt
Object.any_instance.expects(:system).with(
bundle_env(".ruby-lsp/Gemfile"),
"(bundle check || bundle install) 1>&2",
)
).returns(true)
Bundler.with_unbundled_env do
run_script
end
Expand Down Expand Up @@ -160,7 +166,7 @@ def test_does_not_copy_gemfile_lock_when_not_modified
Object.any_instance.expects(:system).with(
bundle_env(".ruby-lsp/Gemfile"),
"((bundle check && bundle update ruby-lsp debug) || bundle install) 1>&2",
)
).returns(true)

FileUtils.expects(:cp).never

Expand Down Expand Up @@ -197,7 +203,7 @@ def test_does_only_updates_every_4_hours
Object.any_instance.expects(:system).with(
bundle_env(".ruby-lsp/Gemfile"),
"(bundle check || bundle install) 1>&2",
)
).returns(true)

Bundler.with_unbundled_env do
# Run the script again without having the lockfile modified
Expand All @@ -210,7 +216,10 @@ def test_does_only_updates_every_4_hours

def test_uses_absolute_bundle_path_for_bundle_install
Bundler.settings.set_global("path", "vendor/bundle")
Object.any_instance.expects(:system).with(bundle_env(".ruby-lsp/Gemfile"), "(bundle check || bundle install) 1>&2")
Object.any_instance.expects(:system).with(
bundle_env(".ruby-lsp/Gemfile"),
"(bundle check || bundle install) 1>&2",
).returns(true)
Bundler::LockfileParser.any_instance.expects(:dependencies).returns({}).at_least_once
run_script(expected_path: File.expand_path("vendor/bundle", Dir.pwd))
ensure
Expand All @@ -227,7 +236,7 @@ def test_creates_custom_bundle_if_no_gemfile
Object.any_instance.expects(:system).with(
bundle_env(bundle_gemfile.to_s),
"(bundle check || bundle install) 1>&2",
)
).returns(true)

Bundler.with_unbundled_env do
run_script
Expand Down Expand Up @@ -289,7 +298,7 @@ def test_does_nothing_if_both_ruby_lsp_and_debug_are_gemspec_dependencies
FileUtils.touch(File.join(dir, "Gemfile.lock"))

Bundler.with_unbundled_env do
Object.any_instance.expects(:system).with(bundle_env, "(bundle check || bundle install) 1>&2")
Object.any_instance.expects(:system).with(bundle_env, "(bundle check || bundle install) 1>&2").returns(true)
Bundler::LockfileParser.any_instance.expects(:dependencies).returns({})
run_script
end
Expand All @@ -306,7 +315,7 @@ def test_creates_custom_bundle_with_specified_branch
Object.any_instance.expects(:system).with(
bundle_env(bundle_gemfile.to_s),
"(bundle check || bundle install) 1>&2",
)
).returns(true)

Bundler.with_unbundled_env do
run_script(branch: "test-branch")
Expand Down Expand Up @@ -342,7 +351,7 @@ def test_install_prerelease_versions_if_experimental_is_true
Object.any_instance.expects(:system).with(
bundle_env(".ruby-lsp/Gemfile"),
"((bundle check && bundle update ruby-lsp debug --pre) || bundle install) 1>&2",
)
).returns(true)

Bundler.with_unbundled_env do
run_script(experimental: true)
Expand All @@ -361,7 +370,7 @@ def test_returns_bundle_app_config_if_there_is_local_config
Object.any_instance.expects(:system).with(
bundle_env(bundle_gemfile.to_s),
"(bundle check || bundle install) 1>&2",
)
).returns(true)

run_script
end
Expand Down Expand Up @@ -490,7 +499,7 @@ def test_ruby_lsp_rails_is_automatically_included_in_rails_apps
Object.any_instance.expects(:system).with(
bundle_env(".ruby-lsp/Gemfile"),
"(bundle check || bundle install) 1>&2",
)
).returns(true)
Bundler.with_unbundled_env do
run_script
end
Expand All @@ -501,6 +510,73 @@ def test_ruby_lsp_rails_is_automatically_included_in_rails_apps
end
end

def test_recovers_from_stale_lockfiles
Dir.mktmpdir do |dir|
custom_dir = File.join(dir, ".ruby-lsp")
FileUtils.mkdir_p(custom_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)

# Write the lockfile hash based on the valid file
File.write(File.join(custom_dir, "main_lockfile_hash"), Digest::SHA256.hexdigest(lockfile_contents))

# Write the custom bundle's lockfile using a fake version that doesn't exist to force bundle install to fail
File.write(File.join(custom_dir, "Gemfile"), <<~GEMFILE)
source "https://rubygems.org"
gem "stringio"
GEMFILE
File.write(File.join(custom_dir, "Gemfile.lock"), <<~LOCKFILE)
GEM
remote: https://rubygems.org/
specs:
stringio (999.1.555)
PLATFORMS
arm64-darwin-23
ruby
DEPENDENCIES
stringio
BUNDLED WITH
2.5.7
LOCKFILE

Bundler.with_unbundled_env do
run_script
end

# Verify that the script recovered and re-generated the custom bundle from scratch
assert_path_exists(".ruby-lsp/Gemfile")
assert_path_exists(".ruby-lsp/Gemfile.lock")
refute_match("999.1.555", File.read(".ruby-lsp/Gemfile.lock"))
end
end
end

private

# This method runs the script and then immediately unloads it. This allows us to make assertions against the effects
Expand Down

0 comments on commit 687bb2c

Please sign in to comment.