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

Pass environment as a hash to system or exec calls #841

Merged
merged 1 commit into from
Jul 31, 2023
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
6 changes: 3 additions & 3 deletions exe/ruby-lsp
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ if ENV["BUNDLE_GEMFILE"].nil? && File.exist?("Gemfile.lock")
# SetupBundler to install the gems
path = Bundler.settings["path"]

command = +"BUNDLE_GEMFILE=#{bundle_gemfile} bundle exec ruby-lsp #{ARGV.join(" ")}"
command.prepend("BUNDLE_PATH=#{File.expand_path(path, Dir.pwd)} ") if path
exit exec(command)
env = { "BUNDLE_GEMFILE" => bundle_gemfile }
env["BUNDLE_PATH"] = File.expand_path(path, Dir.pwd) if path
exit exec(env, "bundle exec ruby-lsp #{ARGV.join(" ")}")
end

require "sorbet-runtime"
Expand Down
10 changes: 6 additions & 4 deletions lib/ruby_lsp/setup_bundler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,18 @@ def run_bundle_install(bundle_gemfile = nil)
# want to install it in the top level `vendor` and not `.ruby-lsp/vendor`
path = Bundler.settings["path"]

command = +""
# Use the absolute `BUNDLE_PATH` to prevent accidentally creating unwanted folders under `.ruby-lsp`
command << "BUNDLE_PATH=#{File.expand_path(path, Dir.pwd)} " if path
command << "BUNDLE_GEMFILE=#{bundle_gemfile} " if bundle_gemfile
env = {}
env["BUNDLE_GEMFILE"] = bundle_gemfile if bundle_gemfile
env["BUNDLE_PATH"] = File.expand_path(path, Dir.pwd) if path

# If both `ruby-lsp` and `debug` 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 either `ruby-lsp` or `debug`
# weren't a part of the Gemfile, then we need to run `bundle install` for the first time to generate the
# Gemfile.lock with them included or else Bundler will complain that they're missing. We can only update if the
# custom `.ruby-lsp/Gemfile.lock` already exists and includes both gems
command = +""

if (@dependencies["ruby-lsp"] && @dependencies["debug"]) ||
@custom_bundle_dependencies["ruby-lsp"].nil? || @custom_bundle_dependencies["debug"].nil?
# Install gems using the custom bundle
Expand All @@ -147,7 +149,7 @@ def run_bundle_install(bundle_gemfile = nil)

# Add bundle update
warn("Ruby LSP> Running bundle install for the custom bundle. This may take a while...")
system(command)
system(env, command)
end
end
end
25 changes: 12 additions & 13 deletions test/setup_bundler_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,20 @@

class SetupBundlerTest < Minitest::Test
def test_does_nothing_when_running_in_the_ruby_lsp
Object.any_instance.expects(:system).with(bundle_install_command)
Object.any_instance.expects(:system).with(bundle_env, "bundle install 1>&2")
run_script("/some/path/ruby-lsp")
refute_path_exists(".ruby-lsp")
end

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

def test_removes_ruby_lsp_folder_if_both_gems_were_added_to_the_bundle
Object.any_instance.expects(:system).with(bundle_install_command)
Object.any_instance.expects(:system).with(bundle_env, "bundle install 1>&2")
Bundler::LockfileParser.any_instance.expects(:dependencies).returns({ "ruby-lsp" => true, "debug" => true })
FileUtils.mkdir(".ruby-lsp")
run_script
Expand All @@ -29,7 +29,7 @@ def test_removes_ruby_lsp_folder_if_both_gems_were_added_to_the_bundle
end

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

Expand All @@ -43,7 +43,7 @@ def test_creates_custom_bundle
end

def test_copies_gemfile_lock_when_modified
Object.any_instance.expects(:system).with(bundle_install_command(".ruby-lsp/Gemfile"))
Object.any_instance.expects(:system).with(bundle_env(".ruby-lsp/Gemfile"), "bundle install 1>&2")
Bundler::LockfileParser.any_instance.expects(:dependencies).returns({}).twice
FileUtils.mkdir(".ruby-lsp")
FileUtils.touch(".ruby-lsp/Gemfile.lock")
Expand All @@ -59,7 +59,7 @@ def test_copies_gemfile_lock_when_modified
end

def test_does_not_copy_gemfile_lock_when_not_modified
Object.any_instance.expects(:system).with(bundle_install_command(".ruby-lsp/Gemfile"))
Object.any_instance.expects(:system).with(bundle_env(".ruby-lsp/Gemfile"), "bundle install 1>&2")
Bundler::LockfileParser.any_instance.expects(:dependencies).returns({}).twice
FileUtils.mkdir(".ruby-lsp")
FileUtils.cp("Gemfile.lock", ".ruby-lsp/Gemfile.lock")
Expand All @@ -71,7 +71,7 @@ def test_does_not_copy_gemfile_lock_when_not_modified

def test_uses_absolute_bundle_path_for_bundle_install
Bundler.settings.set_global("path", "vendor/bundle")
Object.any_instance.expects(:system).with(bundle_install_command(".ruby-lsp/Gemfile"))
Object.any_instance.expects(:system).with(bundle_env(".ruby-lsp/Gemfile"), "bundle install 1>&2")
Bundler::LockfileParser.any_instance.expects(:dependencies).returns({})
run_script
ensure
Expand All @@ -88,13 +88,12 @@ def run_script(path = "/fake/project/path")
RubyLsp::SetupBundler.new(path).setup!
end

def bundle_install_command(bundle_gemfile = nil)
def bundle_env(bundle_gemfile = nil)
path = Bundler.settings["path"]

command = +""
command << "BUNDLE_PATH=#{File.expand_path(path, Dir.pwd)} " if path
command << "BUNDLE_GEMFILE=#{bundle_gemfile} " if bundle_gemfile
command << "bundle install "
command << "1>&2"
env = {}
env["BUNDLE_PATH"] = File.expand_path(path, Dir.pwd) if path
env["BUNDLE_GEMFILE"] = bundle_gemfile if bundle_gemfile
env
end
end