Auto merge of #4303 - njam:original_env, r=segiddins

Introduce "Bundler.clean_env"

Via #4232
Which should return the environment as it was *before* bundler modified it.

Should this use "original_env" or "clean_env" or both?
> At one time, the difference was that one went up one layer of env, while the other removed all Bundler values. I'm not sure if that's still true today.

You can assign this ticket to me if you want.

lib/bundler.rb

@@ -200,31 +200,41 @@ def settings
       @settings = Settings.new(Pathname.new(".bundle").expand_path)
     end
 
-    def with_original_env
-      bundled_env = ENV.to_hash
-      ENV.replace(ORIGINAL_ENV)
-      yield
-    ensure
-      ENV.replace(bundled_env.to_hash)
+    # @return [Hash] Environment present before Bundler was activated
+    def original_env
+      ORIGINAL_ENV.clone
     end
 
-    def with_clean_env
-      with_original_env do
-        ENV["MANPATH"] = ENV["BUNDLE_ORIG_MANPATH"]
-        ENV.delete_if {|k, _| k[0, 7] == "BUNDLE_" }
+    # @deprecated Use `original_env` instead
+    # @return [Hash] Environment with all bundler-related variables removed
+    def clean_env
+      env = original_env
 
-        if ENV.key?("RUBYOPT")
-          ENV["RUBYOPT"] = ENV["RUBYOPT"].sub "-rbundler/setup", ""
-        end
+      if env.key?("BUNDLE_ORIG_MANPATH")
+        env["MANPATH"] = env["BUNDLE_ORIG_MANPATH"]
+      end
 
-        if ENV.key?("RUBYLIB")
-          rubylib = ENV["RUBYLIB"].split(File::PATH_SEPARATOR)
-          rubylib.delete(File.expand_path("..", __FILE__))
-          ENV["RUBYLIB"] = rubylib.join(File::PATH_SEPARATOR)
-        end
+      env.delete_if {|k, _| k[0, 7] == "BUNDLE_" }
 
-        yield
+      if env.key?("RUBYOPT")
+        env["RUBYOPT"] = env["RUBYOPT"].sub "-rbundler/setup", ""
       end
+
+      if env.key?("RUBYLIB")
+        rubylib = env["RUBYLIB"].split(File::PATH_SEPARATOR)
+        rubylib.delete(File.expand_path("..", __FILE__))
+        env["RUBYLIB"] = rubylib.join(File::PATH_SEPARATOR)
+      end
+
+      env
+    end
+
+    def with_original_env
+      with_env(original_env) { yield }
+    end
+
+    def with_clean_env
+      with_env(clean_env) { yield }
     end
 
     def clean_system(*args)
@@ -430,5 +440,14 @@ def upgrade_lockfile
       Bundler.ui.warn "Detected Gemfile.lock generated by 0.9, deleting..."
       lockfile.rmtree
     end
+
+    # @param env [Hash]
+    def with_env(env)
+      backup = ENV.to_hash
+      ENV.replace(env)
+      yield
+    ensure
+      ENV.replace(backup)
+    end
   end
 end

spec/runtime/with_clean_env_spec.rb

@@ -2,107 +2,96 @@
 require "spec_helper"
 
 describe "Bundler.with_env helpers" do
-  shared_examples_for "Bundler.with_*_env" do
-    it "should reset and restore the environment" do
-      gem_path = ENV["GEM_PATH"]
-      path = ENV["PATH"]
-
-      Bundler.with_clean_env do
-        expect(`echo $GEM_PATH`.strip).not_to eq(gem_path)
-        expect(`echo $PATH`.strip).not_to eq(path)
-      end
-
-      expect(ENV["GEM_PATH"]).to eq(gem_path)
-      expect(ENV["PATH"]).to eq(path)
-    end
-  end
-
-  around do |example|
-    env = Bundler::ORIGINAL_ENV.dup
-    Bundler::ORIGINAL_ENV["BUNDLE_PATH"] = "./Gemfile"
-    example.run
-    Bundler::ORIGINAL_ENV.replace env
-  end
-
-  describe "Bundler.with_clean_env" do
-    it_should_behave_like "Bundler.with_*_env"
-
-    it "should keep the original GEM_PATH even in sub processes" do
+  describe "Bundler.original_env" do
+    before do
       gemfile ""
       bundle "install --path vendor/bundle"
+    end
 
-      code = "Bundler.with_clean_env do;" \
-             "  print ENV['GEM_PATH'] != '';" \
-             "end"
+    it "should return the PATH present before bundle was activated" do
+      code = "print Bundler.original_env['PATH']"
+      path = `getconf PATH`.strip + ":/foo"
+      with_path_as(path) do
+        result = bundle("exec ruby -e #{code.inspect}")
+        expect(result).to eq(path)
+      end
+    end
 
-      result = bundle "exec ruby -e #{code.inspect}"
-      expect(result).to eq("true")
+    it "should return the GEM_PATH present before bundle was activated" do
+      code = "print Bundler.original_env['GEM_PATH']"
+      gem_path = ENV["GEM_PATH"] + ":/foo"
+      with_gem_path_as(gem_path) do
+        result = bundle("exec ruby -e #{code.inspect}")
+        expect(result).to eq(gem_path)
+      end
     end
+  end
 
-    it "should keep the original PATH even in sub processes" do
+  describe "Bundler.clean_env" do
+    before do
       gemfile ""
       bundle "install --path vendor/bundle"
-
-      code = "Bundler.with_clean_env do;" \
-             "  print ENV['PATH'] != '';" \
-             "end"
-
-      result = bundle "exec ruby -e #{code.inspect}"
-      expect(result).to eq("true")
     end
 
-    it "should not pass any bundler environment variables" do
-      Bundler.with_clean_env do
-        expect(`echo $BUNDLE_PATH`.strip).not_to eq("./Gemfile")
-      end
+    it "should delete BUNDLE_PATH" do
+      code = "print Bundler.clean_env.has_key?('BUNDLE_PATH')"
+      ENV["BUNDLE_PATH"] = "./foo"
+      result = bundle("exec ruby -e #{code.inspect}")
+      expect(result).to eq("false")
     end
 
-    it "should not pass RUBYOPT changes" do
-      Bundler::ORIGINAL_ENV["RUBYOPT"] = " -rbundler/setup"
-
-      Bundler.with_clean_env do
-        expect(`echo $RUBYOPT`.strip).not_to include "-rbundler/setup"
-      end
-
-      expect(Bundler::ORIGINAL_ENV["RUBYOPT"]).to eq(" -rbundler/setup")
+    it "should remove '-rbundler/setup' from RUBYOPT" do
+      code = "print Bundler.clean_env['RUBYOPT']"
+      ENV["RUBYOPT"] = "-W2 -rbundler/setup"
+      result = bundle("exec ruby -e #{code.inspect}")
+      expect(result).not_to include("-rbundler/setup")
     end
 
-    it "cleans RUBYLIB" do
-      lib_path = File.expand_path("../../../lib", __FILE__)
-      Bundler::ORIGINAL_ENV["RUBYLIB"] = lib_path
-
-      Bundler.with_clean_env do
-        expect(`echo $RUBYLIB`.strip).not_to include(lib_path)
-      end
+    it "should clean up RUBYLIB" do
+      code = "print Bundler.clean_env['RUBYLIB']"
+      ENV["RUBYLIB"] = File.expand_path("../../../lib", __FILE__) + File::PATH_SEPARATOR + "/foo"
+      result = bundle("exec ruby -e #{code.inspect}")
+      expect(result).to eq("/foo")
     end
 
-    it "should not change ORIGINAL_ENV" do
-      expect(Bundler::ORIGINAL_ENV["BUNDLE_PATH"]).to eq("./Gemfile")
+    it "should restore the original MANPATH" do
+      code = "print Bundler.clean_env['MANPATH']"
+      ENV["MANPATH"] = "/foo"
+      ENV["BUNDLE_ORIG_MANPATH"] = "/foo-original"
+      result = bundle("exec ruby -e #{code.inspect}")
+      expect(result).to eq("/foo-original")
     end
   end
 
   describe "Bundler.with_original_env" do
-    it_should_behave_like "Bundler.with_*_env"
+    it "should set ENV to original_env in the block" do
+      expected = Bundler.original_env
+      actual = Bundler.with_original_env { ENV.to_hash }
+      expect(actual).to eq(expected)
+    end
 
-    it "should pass bundler environment variables set before Bundler was run" do
+    it "should restore the environment after execution" do
       Bundler.with_original_env do
-        expect(`echo $BUNDLE_PATH`.strip).to eq("./Gemfile")
+        ENV["FOO"] = "hello"
       end
-    end
 
-    it "should preserve the PATH environment variable" do
-      gemfile ""
-      bundle "install --path vendor/bundle"
+      expect(ENV).not_to have_key("FOO")
+    end
+  end
 
-      code = "Bundler.with_original_env do;" \
-             "  print ENV['PATH'];" \
-             "end"
+  describe "Bundler.with_clean_env" do
+    it "should set ENV to clean_env in the block" do
+      expected = Bundler.clean_env
+      actual = Bundler.with_clean_env { ENV.to_hash }
+      expect(actual).to eq(expected)
+    end
 
-      path = `getconf PATH`.strip
-      with_path_as(path) do
-        result = bundle("exec ruby -e #{code.inspect}")
-        expect(result).to eq(path)
+    it "should restore the environment after execution" do
+      Bundler.with_clean_env do
+        ENV["FOO"] = "hello"
       end
+
+      expect(ENV).not_to have_key("FOO")
     end
   end