From 5131fcda14bf436390cd966b8d75bc648173e16e Mon Sep 17 00:00:00 2001
From: Reto Kaiser <reto@cargomedia.ch>
Date: Sat, 13 Feb 2016 16:55:54 +0100
Subject: [PATCH] Refactor path/environment preserver, make it always preserve
the PATH
---
lib/bundler.rb | 8 +--
lib/bundler/environment_preserver.rb | 35 +++++++++++
lib/bundler/path_preserver.rb | 12 ----
spec/bundler/cli_spec.rb | 2 +-
spec/bundler/environment_preserver_spec.rb | 68 ++++++++++++++++++++++
spec/bundler/path_preserver_spec.rb | 54 -----------------
spec/commands/exec_spec.rb | 7 ++-
spec/commands/help_spec.rb | 50 ++++++++--------
spec/install/gemfile/git_spec.rb | 4 +-
spec/runtime/with_clean_env_spec.rb | 15 +++++
spec/spec_helper.rb | 25 +-------
spec/support/helpers.rb | 38 ++++++------
12 files changed, 176 insertions(+), 142 deletions(-)
create mode 100644 lib/bundler/environment_preserver.rb
delete mode 100644 lib/bundler/path_preserver.rb
create mode 100644 spec/bundler/environment_preserver_spec.rb
delete mode 100644 spec/bundler/path_preserver_spec.rb
diff --git a/lib/bundler.rb b/lib/bundler.rb
index b0f166040f8..ca896094e09 100644
--- a/lib/bundler.rb
+++ b/lib/bundler.rb
@@ -3,7 +3,7 @@
require "pathname"
require "rbconfig"
require "thread"
-require "bundler/path_preserver"
+require "bundler/environment_preserver"
require "bundler/gem_remote_fetcher"
require "bundler/rubygems_ext"
require "bundler/rubygems_integration"
@@ -13,9 +13,9 @@
require "bundler/errors"
module Bundler
- PathPreserver.preserve_path_in_environment("PATH", ENV)
- PathPreserver.preserve_path_in_environment("GEM_PATH", ENV)
- ORIGINAL_ENV = ENV.to_hash
+ environment_preserver = EnvironmentPreserver.new(ENV, %w(PATH GEM_PATH))
+ ORIGINAL_ENV = environment_preserver.restore
+ ENV.replace(environment_preserver.backup)
SUDO_MUTEX = Mutex.new
autoload :Definition, "bundler/definition"
diff --git a/lib/bundler/environment_preserver.rb b/lib/bundler/environment_preserver.rb
new file mode 100644
index 00000000000..0dd2c1ed61e
--- /dev/null
+++ b/lib/bundler/environment_preserver.rb
@@ -0,0 +1,35 @@
+# frozen_string_literal: true
+module Bundler
+ class EnvironmentPreserver
+ # @param env [ENV]
+ # @param keys [Array<String>]
+ def initialize(env, keys)
+ @original = env.to_hash
+ @keys = keys
+ @prefix = "BUNDLE_ORIG_"
+ end
+
+ # @return [Hash]
+ def backup
+ env = restore.clone
+ @keys.each do |key|
+ value = env[key]
+ env[@prefix + key] = value unless value.nil? || value.empty?
+ end
+ env
+ end
+
+ # @return [Hash]
+ def restore
+ env = @original.clone
+ @keys.each do |key|
+ value_original = env[@prefix + key]
+ unless value_original.nil? || value_original.empty?
+ env[key] = value_original
+ env.delete(@prefix + key)
+ end
+ end
+ env
+ end
+ end
+end
diff --git a/lib/bundler/path_preserver.rb b/lib/bundler/path_preserver.rb
deleted file mode 100644
index f5701cd67b6..00000000000
--- a/lib/bundler/path_preserver.rb
+++ /dev/null
@@ -1,12 +0,0 @@
-# frozen_string_literal: true
-module Bundler
- module PathPreserver
- def self.preserve_path_in_environment(env_var, env)
- original_env_var = "_ORIGINAL_#{env_var}"
- original_path = env[original_env_var]
- path = env[env_var]
- env[original_env_var] = path if original_path.nil? || original_path.empty?
- env[env_var] = original_path if path.nil? || path.empty?
- end
- end
-end
diff --git a/spec/bundler/cli_spec.rb b/spec/bundler/cli_spec.rb
index b48af7c81a7..19c6f8277bc 100644
--- a/spec/bundler/cli_spec.rb
+++ b/spec/bundler/cli_spec.rb
@@ -18,7 +18,7 @@
f.puts "#!/usr/bin/env ruby\nputs 'Hello, world'\n"
end
- with_path_as(tmp) do
+ with_path_added(tmp) do
bundle "testtasks"
end
diff --git a/spec/bundler/environment_preserver_spec.rb b/spec/bundler/environment_preserver_spec.rb
new file mode 100644
index 00000000000..b3e7019a75a
--- /dev/null
+++ b/spec/bundler/environment_preserver_spec.rb
@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+require "spec_helper"
+
+describe Bundler::EnvironmentPreserver do
+ let(:preserver) { described_class.new(env, ["foo"]) }
+
+ describe "#backup" do
+ let(:env) { { "foo" => "my-foo", "bar" => "my-bar" } }
+ subject { preserver.backup }
+
+ it "should create backup entries" do
+ expect(subject["BUNDLE_ORIG_foo"]).to eq("my-foo")
+ end
+
+ it "should keep the original entry" do
+ expect(subject["foo"]).to eq("my-foo")
+ end
+
+ it "should not create backup entries for unspecified keys" do
+ expect(subject.key?("BUNDLE_ORIG_bar")).to eq(false)
+ end
+
+ it "should not affect the original env" do
+ subject
+ expect(env.keys.sort).to eq(%w(bar foo))
+ end
+
+ context "when a key is empty" do
+ let(:env) { { "foo" => "" } }
+
+ it "should not create backup entries" do
+ expect(subject.key?("BUNDLE_ORIG_foo")).to eq(false)
+ end
+ end
+ end
+
+ describe "#restore" do
+ subject { preserver.restore }
+
+ context "when an original key is set" do
+ let(:env) { { "foo" => "my-foo", "BUNDLE_ORIG_foo" => "orig-foo" } }
+
+ it "should restore the original value" do
+ expect(subject["foo"]).to eq("orig-foo")
+ end
+
+ it "should delete the backup value" do
+ expect(subject.key?("BUNDLE_ORIG_foo")).to eq(false)
+ end
+ end
+
+ context "when no original key is set" do
+ let(:env) { { "foo" => "my-foo" } }
+
+ it "should keep the current value" do
+ expect(subject["foo"]).to eq("my-foo")
+ end
+ end
+
+ context "when the original key is empty" do
+ let(:env) { { "foo" => "my-foo", "BUNDLE_ORIG_foo" => "" } }
+
+ it "should keep the current value" do
+ expect(subject["foo"]).to eq("my-foo")
+ end
+ end
+ end
+end
diff --git a/spec/bundler/path_preserver_spec.rb b/spec/bundler/path_preserver_spec.rb
deleted file mode 100644
index 78fda9aa408..00000000000
--- a/spec/bundler/path_preserver_spec.rb
+++ /dev/null
@@ -1,54 +0,0 @@
-# frozen_string_literal: true
-require "spec_helper"
-
-describe Bundler::PathPreserver do
- describe "#preserve_path_in_environment" do
- subject { described_class.preserve_path_in_environment(env_var, env) }
-
- context "env_var is PATH" do
- let(:env_var) { "PATH" }
- let(:path) { "/path/here" }
- let(:original_path) { "/original/path/here" }
-
- context "when _ORIGINAL_PATH in env is nil" do
- let(:env) { { "ORIGINAL_PATH" => nil, "PATH" => path } }
-
- it "should set _ORIGINAL_PATH of env to value of PATH from env" do
- expect(env["_ORIGINAL_PATH"]).to be_nil
- subject
- expect(env["_ORIGINAL_PATH"]).to eq("/path/here")
- end
- end
-
- context "when original_path in env is empty" do
- let(:env) { { "_ORIGINAL_PATH" => "", "PATH" => path } }
-
- it "should set _ORIGINAL_PATH of env to value of PATH from env" do
- expect(env["_ORIGINAL_PATH"]).to be_empty
- subject
- expect(env["_ORIGINAL_PATH"]).to eq("/path/here")
- end
- end
-
- context "when path in env is nil" do
- let(:env) { { "_ORIGINAL_PATH" => original_path, "PATH" => nil } }
-
- it "should set PATH of env to value of _ORIGINAL_PATH from env" do
- expect(env["PATH"]).to be_nil
- subject
- expect(env["PATH"]).to eq("/original/path/here")
- end
- end
-
- context "when path in env is empty" do
- let(:env) { { "_ORIGINAL_PATH" => original_path, "PATH" => "" } }
-
- it "should set PATH of env to value of _ORIGINAL_PATH from env" do
- expect(env["PATH"]).to be_empty
- subject
- expect(env["PATH"]).to eq("/original/path/here")
- end
- end
- end
- end
-end
diff --git a/spec/commands/exec_spec.rb b/spec/commands/exec_spec.rb
index ee766cdb5ff..16e853bb1d9 100644
--- a/spec/commands/exec_spec.rb
+++ b/spec/commands/exec_spec.rb
@@ -103,8 +103,9 @@
f.puts "echo foobar"
end
File.chmod(0744, "--verbose")
- ENV["PATH"] = "."
- bundle "exec -- --verbose"
+ with_path_as(".") do
+ bundle "exec -- --verbose"
+ end
expect(out).to eq("foobar")
end
@@ -358,7 +359,7 @@ def bin_path(a,b,c)
Bundler.rubygems.extend(Monkey)
G
bundle "install --deployment"
- bundle "exec ruby -e '`bundler -v`; puts $?.success?'"
+ bundle "exec ruby -e '`../../exe/bundler -v`; puts $?.success?'"
expect(out).to match("true")
end
end
diff --git a/spec/commands/help_spec.rb b/spec/commands/help_spec.rb
index 704b99a0e83..4e35ba3e6ab 100644
--- a/spec/commands/help_spec.rb
+++ b/spec/commands/help_spec.rb
@@ -13,23 +13,23 @@
end
it "uses mann when available" do
- fake_man!
-
- bundle "help gemfile"
+ with_fake_man do
+ bundle "help gemfile"
+ end
expect(out).to eq(%(["#{root}/lib/bundler/man/gemfile.5"]))
end
it "prefixes bundle commands with bundle- when finding the groff files" do
- fake_man!
-
- bundle "help install"
+ with_fake_man do
+ bundle "help install"
+ end
expect(out).to eq(%(["#{root}/lib/bundler/man/bundle-install"]))
end
it "simply outputs the txt file when there is no man on the path" do
- kill_path!
-
- bundle "help install", :expect_err => true
+ with_path_as("") do
+ bundle "help install", :expect_err => true
+ end
expect(out).to match(/BUNDLE-INSTALL/)
end
@@ -43,7 +43,7 @@
f.puts "#!/usr/bin/env ruby\nputs ARGV.join(' ')\n"
end
- with_path_as(tmp) do
+ with_path_added(tmp) do
bundle "help testtasks"
end
@@ -52,37 +52,37 @@
end
it "is called when the --help flag is used after the command" do
- fake_man!
-
- bundle "install --help"
+ with_fake_man do
+ bundle "install --help"
+ end
expect(out).to eq(%(["#{root}/lib/bundler/man/bundle-install"]))
end
it "is called when the --help flag is used before the command" do
- fake_man!
-
- bundle "--help install"
+ with_fake_man do
+ bundle "--help install"
+ end
expect(out).to eq(%(["#{root}/lib/bundler/man/bundle-install"]))
end
it "is called when the -h flag is used before the command" do
- fake_man!
-
- bundle "-h install"
+ with_fake_man do
+ bundle "-h install"
+ end
expect(out).to eq(%(["#{root}/lib/bundler/man/bundle-install"]))
end
it "is called when the -h flag is used after the command" do
- fake_man!
-
- bundle "install -h"
+ with_fake_man do
+ bundle "install -h"
+ end
expect(out).to eq(%(["#{root}/lib/bundler/man/bundle-install"]))
end
it "has helpful output when using --help flag for a non-existent command" do
- fake_man!
-
- bundle "instill -h", :expect_err => true
+ with_fake_man do
+ bundle "instill -h", :expect_err => true
+ end
expect(err).to include('Could not find command "instill -h --no-color".')
end
end
diff --git a/spec/install/gemfile/git_spec.rb b/spec/install/gemfile/git_spec.rb
index ecd64625406..c5c70c2c124 100644
--- a/spec/install/gemfile/git_spec.rb
+++ b/spec/install/gemfile/git_spec.rb
@@ -977,7 +977,9 @@
end
G
- bundle "update", :env => { "PATH" => "", "_ORIGINAL_PATH" => "" }
+ with_path_as("") do
+ bundle "update"
+ end
expect(out).to include("You need to install git to be able to use gems from git repositories. For help installing git, please refer to GitHub's tutorial at https://help.github.com/articles/set-up-git")
end
diff --git a/spec/runtime/with_clean_env_spec.rb b/spec/runtime/with_clean_env_spec.rb
index 92607efc76f..6bb0d21e786 100644
--- a/spec/runtime/with_clean_env_spec.rb
+++ b/spec/runtime/with_clean_env_spec.rb
@@ -89,6 +89,21 @@
expect(`echo $BUNDLE_PATH`.strip).to eq("./Gemfile")
end
end
+
+ it "should preserve the PATH environment variable" do
+ gemfile ""
+ bundle "install --path vendor/bundle"
+
+ code = "Bundler.with_original_env do;" \
+ " print ENV['PATH'];" \
+ "end"
+
+ path = `getconf PATH`.strip
+ with_path_as(path) do
+ result = bundle("exec ruby -e #{code.inspect}")
+ expect(result).to eq(path)
+ end
+ end
end
describe "Bundler.clean_system" do
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index bea836aae6c..c9359012fd0 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -80,13 +80,8 @@
config.filter_run :focused => true unless ENV["CI"]
config.run_all_when_everything_filtered = true
- original_wd = Dir.pwd
- original_path = ENV["PATH"]
- original_gem_home = ENV["GEM_HOME"]
- original_ruby_opt = ENV["RUBYOPT"]
- original_ruby_lib = ENV["RUBYLIB"]
- original_git_dir = ENV["GIT_DIR"]
- original_git_work_tree = ENV["GIT_WORK_TREE"]
+ original_wd = Dir.pwd
+ original_env = ENV.to_hash
def pending_jruby_shebang_fix
pending "JRuby executables do not have a proper shebang" if RUBY_PLATFORM == "java"
@@ -110,20 +105,6 @@ def pending_jruby_shebang_fix
puts @out if defined?(@out) && example.exception
Dir.chdir(original_wd)
- # Reset ENV
- ENV["PATH"] = original_path
- ENV["GEM_HOME"] = original_gem_home
- ENV["GEM_PATH"] = original_gem_home
- ENV["RUBYOPT"] = original_ruby_opt
- ENV["RUBYLIB"] = original_ruby_lib
- ENV["GIT_DIR"] = original_git_dir
- ENV["GIT_WORK_TREE"] = original_git_work_tree
- ENV["BUNDLE_PATH"] = nil
- ENV["BUNDLE_GEMFILE"] = nil
- ENV["BUNDLE_FROZEN"] = nil
- ENV["BUNDLE_APP_CONFIG"] = nil
- ENV["BUNDLER_TEST"] = nil
- ENV["BUNDLER_SPEC_PLATFORM"] = nil
- ENV["BUNDLER_SPEC_VERSION"] = nil
+ ENV.replace(original_env)
end
end
diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb
index 8bcae2a45af..60c1c6f42f0 100644
--- a/spec/support/helpers.rb
+++ b/spec/support/helpers.rb
@@ -230,22 +230,28 @@ def install_gems(*gems)
alias_method :install_gem, :install_gems
def with_gem_path_as(path)
- gem_home = ENV["GEM_HOME"]
- gem_path = ENV["GEM_PATH"]
+ backup = ENV.to_hash
ENV["GEM_HOME"] = path.to_s
ENV["GEM_PATH"] = path.to_s
+ ENV["BUNDLE_ORIG_GEM_PATH"] = nil
yield
ensure
- ENV["GEM_HOME"] = gem_home
- ENV["GEM_PATH"] = gem_path
+ ENV.replace(backup)
end
def with_path_as(path)
- old_path = ENV["PATH"]
- ENV["PATH"] = "#{path}:#{ENV["PATH"]}"
+ backup = ENV.to_hash
+ ENV["PATH"] = path.to_s
+ ENV["BUNDLE_ORIG_PATH"] = nil
yield
ensure
- ENV["PATH"] = old_path
+ ENV.replace(backup)
+ end
+
+ def with_path_added(path)
+ with_path_as(path.to_s + ":" + ENV["PATH"]) do
+ yield
+ end
end
def break_git!
@@ -257,17 +263,12 @@ def break_git!
ENV["PATH"] = "#{tmp("broken_path")}:#{ENV["PATH"]}"
end
- def fake_man!
+ def with_fake_man
FileUtils.mkdir_p(tmp("fake_man"))
File.open(tmp("fake_man/man"), "w", 0755) do |f|
f.puts "#!/usr/bin/env ruby\nputs ARGV.inspect\n"
end
-
- ENV["PATH"] = "#{tmp("fake_man")}:#{ENV["PATH"]}"
- end
-
- def kill_path!
- ENV["PATH"] = ""
+ with_path_added(tmp("fake_man")) { yield }
end
def system_gems(*gems)
@@ -278,20 +279,17 @@ def system_gems(*gems)
Gem.clear_paths
- gem_home = ENV["GEM_HOME"]
- gem_path = ENV["GEM_PATH"]
- path = ENV["PATH"]
+ env_backup = ENV.to_hash
ENV["GEM_HOME"] = system_gem_path.to_s
ENV["GEM_PATH"] = system_gem_path.to_s
+ ENV["BUNDLE_ORIG_GEM_PATH"] = nil
install_gems(*gems)
return unless block_given?
begin
yield
ensure
- ENV["GEM_HOME"] = gem_home
- ENV["GEM_PATH"] = gem_path
- ENV["PATH"] = path
+ ENV.replace(env_backup)
end
end