Skip to content

Commit

Permalink
Use original Bundler binstub from RubyGems to avoid version argument …
Browse files Browse the repository at this point in the history
…issues (#2987)

### Motivation

Closes #2862 and #2731 (comment)

Clearly, adding the Bundler version to the command causes trouble for people using binstubs in certain configurations.

I think I found a way to continue specifying the version, which guarantees that RubyGems will eagerly activate the right specification, while at the same time avoiding those issues.

### Implementation

The idea is to invoke the original Bundler binstub generated by RubyGems directly. That way, even if there's a custom binstub, we avoid invoking it and use the default one (which accepts the version).

### Automated Tests

Managed to write a test that fails before this fix.

### Manual tests

1. Create an executable file `bin/bundle`
2. Add this code to the executable
```ruby
#!/usr/bin/env ruby
raise "This should not be called"
```
3. Add the bin directory to your PATH temporary: `export PATH ./bin:$PATH`
4. Verify that booting the LSP does not fail `exe/ruby-lsp`
  • Loading branch information
vinistock authored Jan 6, 2025
1 parent 8792872 commit 5ae8e77
Show file tree
Hide file tree
Showing 4 changed files with 87 additions and 77 deletions.
11 changes: 6 additions & 5 deletions exe/ruby-lsp
Original file line number Diff line number Diff line change
Expand Up @@ -78,13 +78,14 @@ if ENV["BUNDLE_GEMFILE"].nil?
exit(78)
end

base_bundle = if env["BUNDLER_VERSION"]
"bundle _#{env["BUNDLER_VERSION"]}_"
else
"bundle"
bundler_path = File.join(Gem.default_bindir, "bundle")
base_command = (File.exist?(bundler_path) ? "#{Gem.ruby} #{bundler_path}" : "bundle").dup

if env["BUNDLER_VERSION"]
base_command << " _#{env["BUNDLER_VERSION"]}_"
end

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

$LOAD_PATH.unshift(File.expand_path("../lib", __dir__))
Expand Down
51 changes: 14 additions & 37 deletions lib/ruby_lsp/setup_bundler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -202,15 +202,15 @@ def run_bundle_install(bundle_gemfile = @gemfile)
env["BUNDLE_PATH"] = File.expand_path(env["BUNDLE_PATH"], @project_path)
end

return run_bundle_install_through_command(env) unless @launcher

# This same check happens conditionally when running through the command. For invoking the CLI directly, it's
# important that we ensure the Bundler version is set to avoid restarts
# Set the specific Bundler version used by the main app. This avoids issues with Bundler restarts, which clean the
# environment and lead to the `ruby-lsp` executable not being found
if @bundler_version
env["BUNDLER_VERSION"] = @bundler_version.to_s
install_bundler_if_needed
end

return run_bundle_install_through_command(env) unless @launcher

begin
run_bundle_install_directly(env)
# If no error occurred, then clear previous errors
Expand Down Expand Up @@ -272,8 +272,6 @@ def run_bundle_install_directly(env, force_install: false)

sig { params(env: T::Hash[String, String]).returns(T::Hash[String, String]) }
def run_bundle_install_through_command(env)
base_bundle = base_bundle_command(env)

# 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 composed 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 @@ -282,13 +280,20 @@ def run_bundle_install_through_command(env)

# When not updating, we run `(bundle check || bundle install)`
# When updating, we run `((bundle check && bundle update ruby-lsp debug) || bundle install)`
command = +"(#{base_bundle} check"
bundler_path = File.join(Gem.default_bindir, "bundle")
base_command = (File.exist?(bundler_path) ? "#{Gem.ruby} #{bundler_path}" : "bundle").dup

if env["BUNDLER_VERSION"]
base_command << " _#{env["BUNDLER_VERSION"]}_"
end

command = +"(#{base_command} 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 << " && #{base_bundle} update "
command << " && #{base_command} 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 @@ -298,7 +303,7 @@ def run_bundle_install_through_command(env)
@last_updated_path.write(Time.now.iso8601)
end

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

# Redirect stdout to stderr to prevent going into an infinite loop. The extension might confuse stdout output with
# responses
Expand Down Expand Up @@ -408,34 +413,6 @@ def rails_app?
/class .* < (::)?Rails::Application/.match?(application_contents)
end

# Returns the base bundle command we should use for this project, which will be:
# - `bundle` if there's no locked Bundler version and no `bin/bundle` binstub in the $PATH
# - `bundle _<version>_` if there's a locked Bundler version
# - `bin/bundle` if there's a `bin/bundle` binstub in the $PATH
sig { params(env: T::Hash[String, String]).returns(String) }
def base_bundle_command(env)
path_parts = if Gem.win_platform?
ENV["Path"] || ENV["PATH"] || ENV["path"] || ""
else
ENV["PATH"] || ""
end.split(File::PATH_SEPARATOR)

bin_dir = File.expand_path("bin", @project_path)
bundle_binstub = File.join(@project_path, "bin", "bundle")

if File.exist?(bundle_binstub) && path_parts.any? { |path| File.expand_path(path, @project_path) == bin_dir }
return bundle_binstub
end

if @bundler_version
env["BUNDLER_VERSION"] = @bundler_version.to_s
install_bundler_if_needed
return "bundle _#{@bundler_version}_"
end

"bundle"
end

sig { void }
def patch_thor_to_print_progress_to_stderr!
return unless defined?(Bundler::Thor::Shell::Basic)
Expand Down
1 change: 1 addition & 0 deletions project-words
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ bindir
binmode
binread
binstub
binstubs
Bizt
Bizw
bufnr
Expand Down
101 changes: 66 additions & 35 deletions test/integration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,59 +27,89 @@ def test_ruby_lsp_check_works
end
end

def test_adds_bundler_version_as_part_of_exec_command
def test_uses_same_bundler_version_as_main_app
in_temp_dir do |dir|
File.write(File.join(dir, "Gemfile"), <<~GEMFILE)
File.write(File.join(dir, "Gemfile"), <<~RUBY)
source "https://rubygems.org"
gem "ruby-lsp", path: "#{Bundler.root}"
GEMFILE
gem "stringio"
RUBY

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
capture_subprocess_io do
system("bundle install")

Object.any_instance.expects(:exec).with do |env, command|
env.key?("BUNDLE_GEMFILE") &&
env.key?("BUNDLER_VERSION") &&
/bundle _[\d\.]+_ exec ruby-lsp/.match?(command)
end.once.raises(StandardError.new("stop"))

# We raise intentionally to avoid continuing running the executable
assert_raises(StandardError) do
load(Gem.bin_path("ruby-lsp", "ruby-lsp"))
end
end
end

Bundler.with_unbundled_env do
launch(dir, "ruby-lsp")
end

assert_match(/BUNDLED WITH\n\s*2.5.7/, File.read(File.join(dir, ".ruby-lsp", "Gemfile.lock")))
end
end

def test_avoids_bundler_version_if_local_bin_is_in_path
def test_does_not_use_custom_binstubs_if_they_are_in_the_path
in_temp_dir do |dir|
File.write(File.join(dir, "Gemfile"), <<~GEMFILE)
File.write(File.join(dir, "Gemfile"), <<~RUBY)
source "https://rubygems.org"
gem "ruby-lsp", path: "#{Bundler.root}"
GEMFILE
gem "stringio"
RUBY

FileUtils.mkdir(File.join(dir, "bin"))
FileUtils.touch(File.join(dir, "bin", "bundle"))
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
capture_subprocess_io do
system("bundle install")

Object.any_instance.expects(:exec).with do |env, command|
env.key?("BUNDLE_GEMFILE") &&
!env.key?("BUNDLER_VERSION") &&
"bundle exec ruby-lsp" == command
end.once.raises(StandardError.new("stop"))

ENV["PATH"] = "./bin#{File::PATH_SEPARATOR}#{ENV["PATH"]}"
# We raise intentionally to avoid continuing running the executable
assert_raises(StandardError) do
load(Gem.bin_path("ruby-lsp", "ruby-lsp"))
end
end
end

bin_path = File.join(dir, "bin")
FileUtils.mkdir(bin_path)
File.write(File.join(bin_path, "bundle"), <<~RUBY)
#!/usr/bin/env ruby
raise "This should not be called"
RUBY
FileUtils.chmod(0o755, File.join(bin_path, "bundle"))

Bundler.with_unbundled_env do
launch(dir, "ruby-lsp", { "PATH" => "#{bin_path}#{File::PATH_SEPARATOR}#{ENV["PATH"]}" })
end

assert_match(/BUNDLED WITH\n\s*2.5.7/, File.read(File.join(dir, ".ruby-lsp", "Gemfile.lock")))
end
end

Expand Down Expand Up @@ -163,7 +193,7 @@ def test_composed_bundle_includes_debug

private

def launch(workspace_path)
def launch(workspace_path, exec = "ruby-lsp-launcher", extra_env = {})
specification = Gem::Specification.find_by_name("ruby-lsp")
paths = [specification.full_gem_path]
paths.concat(specification.dependencies.map { |dep| dep.to_spec.full_gem_path })
Expand All @@ -175,9 +205,10 @@ def launch(workspace_path)
end.uniq.flatten

stdin, stdout, stderr, wait_thr = T.unsafe(Open3).popen3(
extra_env,
Gem.ruby,
*load_path,
File.join(@root, "exe", "ruby-lsp-launcher"),
File.join(@root, "exe", exec),
)
stdin.sync = true
stdin.binmode
Expand Down

0 comments on commit 5ae8e77

Please sign in to comment.