Skip to content

Commit 5a36d78

Browse files
authored
Retry composing bundle if it's modified during setup (#2890)
### Motivation If someone switches branches while we're setting up the composed bundle and the switch results in gemfile/lockfile changes, we often fail to boot because the resulting bundle is invalid. ### Implementation I think we can reduce the number of failures by checking if the contents of the gemfile or lockfile have been changed after we finished running bundle install. If that's the case, we can re-build the composed bundle from scratch and retry a single time, which should be able to fix common issues. ### Automated Tests Added a test that verifies the new behaviour.
1 parent fc2d91c commit 5a36d78

File tree

2 files changed

+60
-5
lines changed

2 files changed

+60
-5
lines changed

lib/ruby_lsp/setup_bundler.rb

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ def initialize(project_path, **options)
4545
)
4646
@lockfile = T.let(@gemfile ? Bundler.default_lockfile : nil, T.nilable(Pathname))
4747

48+
@gemfile_hash = T.let(@gemfile ? Digest::SHA256.hexdigest(@gemfile.read) : nil, T.nilable(String))
49+
@lockfile_hash = T.let(@lockfile&.exist? ? Digest::SHA256.hexdigest(@lockfile.read) : nil, T.nilable(String))
50+
4851
@gemfile_name = T.let(@gemfile&.basename&.to_s || "Gemfile", String)
4952

5053
# Custom bundle paths
@@ -91,10 +94,8 @@ def setup!
9194
return run_bundle_install(@custom_gemfile)
9295
end
9396

94-
lockfile_contents = @lockfile.read
95-
current_lockfile_hash = Digest::SHA256.hexdigest(lockfile_contents)
96-
97-
if @custom_lockfile.exist? && @lockfile_hash_path.exist? && @lockfile_hash_path.read == current_lockfile_hash
97+
if @lockfile_hash && @custom_lockfile.exist? && @lockfile_hash_path.exist? &&
98+
@lockfile_hash_path.read == @lockfile_hash
9899
$stderr.puts(
99100
"Ruby LSP> Skipping composed bundle setup since #{@custom_lockfile} already exists and is up to date",
100101
)
@@ -103,7 +104,7 @@ def setup!
103104

104105
FileUtils.cp(@lockfile.to_s, @custom_lockfile.to_s)
105106
correct_relative_remote_paths
106-
@lockfile_hash_path.write(current_lockfile_hash)
107+
@lockfile_hash_path.write(@lockfile_hash)
107108
run_bundle_install(@custom_gemfile)
108109
end
109110

@@ -214,6 +215,23 @@ def run_bundle_install(bundle_gemfile = @gemfile)
214215
@error_path.write(Marshal.dump(e))
215216
end
216217

218+
# If either the Gemfile or the lockfile have been modified during the process of setting up the bundle, retry
219+
# composing the bundle from scratch
220+
221+
if @gemfile && @lockfile
222+
current_gemfile_hash = Digest::SHA256.hexdigest(@gemfile.read)
223+
current_lockfile_hash = Digest::SHA256.hexdigest(@lockfile.read)
224+
225+
if !@retry && (current_gemfile_hash != @gemfile_hash || current_lockfile_hash != @lockfile_hash)
226+
@gemfile_hash = current_gemfile_hash
227+
@lockfile_hash = current_lockfile_hash
228+
@retry = true
229+
@custom_dir.rmtree
230+
$stderr.puts("Ruby LSP> Bundle was modified during setup. Retrying from scratch...")
231+
return setup!
232+
end
233+
end
234+
217235
env
218236
end
219237

test/setup_bundler_test.rb

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -786,6 +786,43 @@ def test_succeeds_when_using_ssh_git_sources_instead_of_https
786786
end
787787
end
788788

789+
def test_is_resilient_to_gemfile_changes_in_the_middle_of_setup
790+
Dir.mktmpdir do |dir|
791+
Dir.chdir(dir) do
792+
File.write(File.join(dir, "Gemfile"), <<~GEMFILE)
793+
source "https://rubygems.org"
794+
gem "rdoc"
795+
GEMFILE
796+
797+
Bundler.with_unbundled_env do
798+
capture_subprocess_io do
799+
# Run bundle install to generate the lockfile
800+
system("bundle install")
801+
802+
# This section simulates the bundle being modified during the composed bundle setup. We initialize the
803+
# composed bundle first to eagerly calculate the gemfile and lockfile hashes. Then we modify the Gemfile
804+
# afterwards and trigger the setup.
805+
#
806+
# This type of scenario may happen if someone switches branches in the middle of running bundle install. By
807+
# the time we finish, the bundle may be in a different state and we need to recover from that
808+
composed_bundle = RubyLsp::SetupBundler.new(dir, launcher: true)
809+
810+
File.write(File.join(dir, "Gemfile"), <<~GEMFILE)
811+
source "https://rubygems.org"
812+
gem "rdoc"
813+
gem "irb"
814+
GEMFILE
815+
system("bundle install")
816+
817+
composed_bundle.setup!
818+
end
819+
820+
assert_match("irb", File.read(".ruby-lsp/Gemfile.lock"))
821+
end
822+
end
823+
end
824+
end
825+
789826
private
790827

791828
def with_default_external_encoding(encoding, &block)

0 commit comments

Comments
 (0)