Skip to content

Commit

Permalink
Prevent passing version to bin/bundle (#2765)
Browse files Browse the repository at this point in the history
Prevent passing version to bin/bundle
  • Loading branch information
vinistock authored Oct 24, 2024
1 parent 5ac10ef commit 2f2f87d
Show file tree
Hide file tree
Showing 4 changed files with 116 additions and 33 deletions.
2 changes: 1 addition & 1 deletion exe/ruby-lsp
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ if ENV["BUNDLE_GEMFILE"].nil?
"bundle"
end

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

$LOAD_PATH.unshift(File.expand_path("../lib", __dir__))
Expand Down
38 changes: 29 additions & 9 deletions lib/ruby_lsp/setup_bundler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -186,15 +186,7 @@ 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
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 custom bundle was just created
Expand Down Expand Up @@ -326,5 +318,33 @@ 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
end
end
1 change: 1 addition & 0 deletions project-words
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ binread
Bizt
Bizw
bufnr
binstub
byteslice
codepoint
codepoints
Expand Down
108 changes: 85 additions & 23 deletions test/integration_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,48 +4,110 @@
require "test_helper"

class IntegrationTest < Minitest::Test
def setup
def test_ruby_lsp_doctor_works
skip("CI only") unless ENV["CI"]
end

def test_ruby_lsp_doctor_works
in_isolation do
system("bundle exec ruby-lsp --doctor")
assert_equal(0, $CHILD_STATUS)
end
end

def test_ruby_lsp_check_works
skip("CI only") unless ENV["CI"]

in_isolation do
system("bundle exec ruby-lsp-check")
assert_equal(0, $CHILD_STATUS)
end
end

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

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
end
end

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

FileUtils.mkdir(File.join(dir, "bin"))
FileUtils.touch(File.join(dir, "bin", "bundle"))

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
end
end

private

def in_isolation(&block)
gem_path = Bundler.root
def in_temp_dir
Dir.mktmpdir do |dir|
Dir.chdir(dir) do
File.write(File.join(dir, "Gemfile"), <<~GEMFILE)
source "https://rubygems.org"
gem "ruby-lsp", path: "#{gem_path}"
GEMFILE

Bundler.with_unbundled_env do
capture_subprocess_io do
system("bundle install")

# Only do this after `bundle install` as to not change the lockfile
File.write(File.join(dir, "Gemfile"), <<~GEMFILE, mode: "a+")
# This causes ruby-lsp to run in its own directory without
# all the supplementary gems like rubocop
Dir.chdir("#{gem_path}")
GEMFILE

yield
end
yield(dir)
end
end
end

def in_isolation(&block)
gem_path = Bundler.root
in_temp_dir do |dir|
File.write(File.join(dir, "Gemfile"), <<~GEMFILE)
source "https://rubygems.org"
gem "ruby-lsp", path: "#{gem_path}"
GEMFILE

Bundler.with_unbundled_env do
capture_subprocess_io do
system("bundle install")

# Only do this after `bundle install` as to not change the lockfile
File.write(File.join(dir, "Gemfile"), <<~GEMFILE, mode: "a+")
# This causes ruby-lsp to run in its own directory without
# all the supplementary gems like rubocop
Dir.chdir("#{gem_path}")
GEMFILE

yield
end
end
end
Expand Down

0 comments on commit 2f2f87d

Please sign in to comment.