Tweak use of with_env

- Use it in more places where it saves code
- Allow using symbolic keys for a cleaner interface
- Use `HOMEBREW_GEM_*` instead of `GEM_OLD_*`

5 files changed, 48 insertions, 68 deletions

diff --git a/Library/Homebrew/brew.sh b/Library/Homebrew/brew.sh
index b2859c927..241fa1c2e 100644
--- a/Library/Homebrew/brew.sh
+++ b/Library/Homebrew/brew.sh
@@ -70,8 +70,8 @@ then
 fi
 
 # Save value to use for installing gems
-export GEM_OLD_HOME="$GEM_HOME"
-export GEM_OLD_PATH="$GEM_PATH"
+export HOMEBREW_GEM_HOME="$GEM_HOME"
+export HOMEBREW_GEM_PATH="$GEM_PATH"
 
 # Users may have these set, pointing the system Ruby
 # at non-system gem paths
diff --git a/Library/Homebrew/extend/fileutils.rb b/Library/Homebrew/extend/fileutils.rb
index 34ef3869f..6286e48c0 100644
--- a/Library/Homebrew/extend/fileutils.rb
+++ b/Library/Homebrew/extend/fileutils.rb
@@ -114,7 +114,7 @@ module FileUtils
 
     if superenv?
       make_name = File.basename(make_path)
-      with_env "HOMEBREW_MAKE" => make_name do
+      with_env(HOMEBREW_MAKE: make_name) do
         system "make", *args
       end
     else
diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb
index 1765f20c1..607fdd5c4 100644
--- a/Library/Homebrew/formula.rb
+++ b/Library/Homebrew/formula.rb
@@ -1615,33 +1615,30 @@ class Formula
   # @private
   def run_test
     @prefix_returns_versioned_prefix = true
-    old_home = ENV["HOME"]
-    old_java_opts = ENV["_JAVA_OPTIONS"]
-    old_curl_home = ENV["CURL_HOME"]
-    old_tmpdir = ENV["TMPDIR"]
-    old_temp = ENV["TEMP"]
-    old_tmp = ENV["TMP"]
-    old_term = ENV["TERM"]
-    old_path = ENV["PATH"]
-    old_homebrew_path = ENV["HOMEBREW_PATH"]
-
-    ENV["CURL_HOME"] = old_curl_home || old_home
-    ENV["TMPDIR"] = ENV["TEMP"] = ENV["TMP"] = HOMEBREW_TEMP
-    ENV["TERM"] = "dumb"
-    ENV["PATH"] = PATH.new(old_path).append(HOMEBREW_PREFIX/"bin")
-    ENV["HOMEBREW_PATH"] = nil
-    ENV["_JAVA_OPTIONS"] = "#{old_java_opts} -Duser.home=#{HOMEBREW_CACHE}/java_cache"
+
+    test_env = {
+      CURL_HOME: ENV["CURL_HOME"] || ENV["HOME"],
+      TMPDIR: HOMEBREW_TEMP,
+      TEMP: HOMEBREW_TEMP,
+      TMP: HOMEBREW_TEMP,
+      TERM: "dumb",
+      PATH: PATH.new(ENV["PATH"]).append(HOMEBREW_PREFIX/"bin"),
+      HOMEBREW_PATH: nil,
+      _JAVA_OPTIONS: "#{ENV["_JAVA_OPTIONS"]} -Duser.home=#{HOMEBREW_CACHE}/java_cache",
+    }
 
     ENV.clear_sensitive_environment!
 
     mktemp("#{name}-test") do |staging|
       staging.retain! if ARGV.keep_tmp?
       @testpath = staging.tmpdir
-      ENV["HOME"] = @testpath
+      test_env[:HOME] = @testpath
       setup_home @testpath
       begin
         with_logging("test") do
-          test
+          with_env(test_env) do
+            test
+          end
         end
       rescue Exception # rubocop:disable Lint/RescueException
         staging.retain! if ARGV.debug?
@@ -1650,15 +1647,6 @@ class Formula
     end
   ensure
     @testpath = nil
-    ENV["HOME"] = old_home
-    ENV["_JAVA_OPTIONS"] = old_java_opts
-    ENV["CURL_HOME"] = old_curl_home
-    ENV["TMPDIR"] = old_tmpdir
-    ENV["TEMP"] = old_temp
-    ENV["TMP"] = old_tmp
-    ENV["TERM"] = old_term
-    ENV["PATH"] = old_path
-    ENV["HOMEBREW_PATH"] = old_homebrew_path
     @prefix_returns_versioned_prefix = false
   end
 
@@ -1891,32 +1879,27 @@ class Formula
       env_home = buildpath/".brew_home"
       mkdir_p env_home
 
-      old_home = ENV["HOME"]
-      old_java_opts = ENV["_JAVA_OPTIONS"]
-      old_curl_home = ENV["CURL_HOME"]
-      old_path = ENV["HOMEBREW_PATH"]
+      stage_env = {
+        HOMEBREW_PATH: nil,
+      }
 
       unless ARGV.interactive?
-        ENV["HOME"] = env_home
-        ENV["_JAVA_OPTIONS"] = "#{old_java_opts} -Duser.home=#{HOMEBREW_CACHE}/java_cache"
-        ENV["CURL_HOME"] = old_curl_home || old_home
+        stage_env[:HOME] = env_home
+        stage_env[:_JAVA_OPTIONS] =
+          "#{ENV["_JAVA_OPTIONS"]} -Duser.home=#{HOMEBREW_CACHE}/java_cache"
+        stage_env[:CURL_HOME] = ENV["CURL_HOME"] || ENV["HOME"]
       end
-      ENV["HOMEBREW_PATH"] = nil
 
       setup_home env_home
 
       ENV.clear_sensitive_environment!
 
       begin
-        yield staging
+        with_env(stage_env) do
+          yield staging
+        end
       ensure
         @buildpath = nil
-        unless ARGV.interactive?
-          ENV["HOME"] = old_home
-          ENV["_JAVA_OPTIONS"] = old_java_opts
-          ENV["CURL_HOME"] = old_curl_home
-        end
-        ENV["HOMEBREW_PATH"] = old_path
       end
     end
   end
diff --git a/Library/Homebrew/test/utils_spec.rb b/Library/Homebrew/test/utils_spec.rb
index 0c2ae5161..037a80b64 100644
--- a/Library/Homebrew/test/utils_spec.rb
+++ b/Library/Homebrew/test/utils_spec.rb
@@ -274,13 +274,13 @@ describe "globally-scoped helper methods" do
   describe "#with_env" do
     it "sets environment variables within the block" do
       expect(ENV["PATH"]).not_to eq("/bin")
-      with_env "PATH" => "/bin" do
+      with_env(PATH: "/bin") do
         expect(ENV["PATH"]).to eq("/bin")
       end
     end
 
     it "restores ENV after the block" do
-      with_env "PATH" => "/bin" do
+      with_env(PATH: "/bin") do
         expect(ENV["PATH"]).to eq("/bin")
       end
       expect(ENV["PATH"]).not_to eq("/bin")
@@ -288,7 +288,7 @@ describe "globally-scoped helper methods" do
 
     it "restores ENV if an exception is raised" do
       expect {
-        with_env "PATH" => "/bin" do
+        with_env(PATH: "/bin") do
           raise StandardError, "boom"
         end
       }.to raise_error(StandardError)
diff --git a/Library/Homebrew/utils.rb b/Library/Homebrew/utils.rb
index 2211ed75b..0c875a8ab 100644
--- a/Library/Homebrew/utils.rb
+++ b/Library/Homebrew/utils.rb
@@ -189,9 +189,11 @@ module Homebrew
 
   def install_gem_setup_path!(name, version = nil, executable = name)
     # Respect user's preferences for where gems should be installed.
-    ENV["GEM_HOME"] = ENV["GEM_OLD_HOME"].to_s
+    ENV["GEM_HOME"] = ENV["HOMEBREW_GEM_HOME"].to_s
     ENV["GEM_HOME"] = Gem.user_dir if ENV["GEM_HOME"].empty?
-    ENV["GEM_PATH"] = ENV["GEM_OLD_PATH"] unless ENV["GEM_OLD_PATH"].to_s.empty?
+    unless ENV["HOMEBREW_GEM_PATH"].to_s.empty?
+      ENV["GEM_PATH"] = ENV["HOMEBREW_GEM_PATH"]
+    end
 
     # Make rubygems notice env changes.
     Gem.clear_paths
@@ -263,31 +265,25 @@ module Homebrew
 end
 
 def with_system_path
-  old_path = ENV["PATH"]
-  ENV["PATH"] = PATH.new("/usr/bin", "/bin")
-  yield
-ensure
-  ENV["PATH"] = old_path
+  with_env(PATH: PATH.new("/usr/bin", "/bin")) do
+    yield
+  end
 end
 
 def with_homebrew_path
-  old_path = ENV["PATH"]
-  ENV["PATH"] = ENV["HOMEBREW_PATH"]
-  yield
-ensure
-  ENV["PATH"] = old_path
+  with_env(PATH: PATH.new(ENV["HOMEBREW_PATH"])) do
+    yield
+  end
 end
 
 def with_custom_locale(locale)
-  old_locale = ENV["LC_ALL"]
-  ENV["LC_ALL"] = locale
-  yield
-ensure
-  ENV["LC_ALL"] = old_locale
+  with_env(LC_ALL: locale) do
+    yield
+  end
 end
 
-def run_as_not_developer(&_block)
-  with_env "HOMEBREW_DEVELOPER" => nil do
+def run_as_not_developer
+  with_env(HOMEBREW_DEVELOPER: nil) do
     yield
   end
 end
@@ -536,7 +532,7 @@ end
 # Calls the given block with the passed environment variables
 # added to ENV, then restores ENV afterwards.
 # Example:
-# with_env "PATH" => "/bin" do
+# with_env(PATH: "/bin") do
 #   system "echo $PATH"
 # end
 #
@@ -547,6 +543,7 @@ def with_env(hash)
   old_values = {}
   begin
     hash.each do |key, value|
+      key = key.to_s
       old_values[key] = ENV.delete(key)
       ENV[key] = value
     end