More robust implementation of ENV.with_build_environment

Library/Homebrew/extend/ENV.rb

@@ -18,6 +18,17 @@ module EnvActivation
       extend(Stdenv)
     end
   end
+
+  def with_build_environment
+    old_env = to_hash.dup
+    tmp_env = to_hash.dup.extend(EnvActivation)
+    tmp_env.activate_extensions!
+    tmp_env.setup_build_environment
+    replace(tmp_env)
+    yield
+  ensure
+    replace(old_env)
+  end
 end
 
 ENV.extend(EnvActivation)

Library/Homebrew/extend/ENV/shared.rb

@@ -70,14 +70,6 @@ module SharedEnvExtension
     prepend 'PATH', HOMEBREW_PREFIX/'bin', ':'
   end
 
-  def with_build_environment
-    old_env = to_hash
-    setup_build_environment
-    yield
-  ensure
-    replace(old_env)
-  end
-
   def fortran
     # superenv removes these PATHs, but this option needs them
     # TODO fix better, probably by making a super-fc

Library/Homebrew/test/test_ENV.rb

@@ -1,42 +1,59 @@
 require 'testing_env'
+require 'extend/ENV'
 
 class EnvironmentTests < Test::Unit::TestCase
+  def setup
+    @env = {}.extend(EnvActivation)
+    @env.activate_extensions!
+  end
+
   def test_ENV_options
-    ENV.gcc_4_0
+    @env.gcc_4_0
-    ENV.O3
+    @env.O3
-    ENV.minimal_optimization
+    @env.minimal_optimization
-    ENV.no_optimization
+    @env.no_optimization
-    ENV.libxml2
+    @env.libxml2
-    ENV.enable_warnings
+    @env.enable_warnings
-    assert !ENV.cc.empty?
+    assert !@env.cc.empty?
-    assert !ENV.cxx.empty?
+    assert !@env.cxx.empty?
   end
 
   def test_switching_compilers
-    ENV.llvm
+    @env.llvm
-    ENV.clang
+    @env.clang
-    assert_nil ENV['LD']
+    assert_nil @env['LD']
-    assert_equal ENV['OBJC'], ENV['CC']
+    assert_equal @env['OBJC'], @env['CC']
   end
 
   def test_with_build_environment_restores_env
-    before = ENV.to_hash
+    before = @env.dup
-    ENV.with_build_environment do
+    @env.with_build_environment do
-      ENV['foo'] = 'bar'
+      @env['foo'] = 'bar'
     end
-    assert_nil ENV['foo']
+    assert_nil @env['foo']
-    assert_equal before, ENV.to_hash
+    assert_equal before, @env
   end
 
   def test_with_build_environment_ensures_env_restored
-    ENV.expects(:replace).with(ENV.to_hash)
+    before = @env.dup
     begin
-      ENV.with_build_environment { raise Exception }
+      @env.with_build_environment do
+        @env['foo'] = 'bar'
+        raise Exception
+      end
     rescue Exception
     end
+    assert_nil @env['foo']
+    assert_equal before, @env
   end
 
   def test_with_build_environment_returns_block_value
-    assert_equal 1, ENV.with_build_environment { 1 }
+    assert_equal 1, @env.with_build_environment { 1 }
+  end
+
+  def test_with_build_environment_does_not_mutate_interface
+    expected = @env.methods
+    @env.with_build_environment { assert_equal expected, @env.methods }
+    assert_equal expected, @env.methods
   end
 end