Infer path to be added for requirements that search PATH

When a requirement is specified like:

  satisfy { which "foo" }

There is no reason that we should inject all of ENV.userpaths! into the
build environment. Instead, infer the directory to be added to PATH from
the Pathname that is returned.

This is another step towards condensing the "which program" requirements
down into a one-liner DSL element.

6 files changed, 42 insertions, 19 deletions

diff --git a/Library/Formula/rhash.rb b/Library/Formula/rhash.rb
index 9bea1082a..32de2c134 100644
--- a/Library/Formula/rhash.rb
+++ b/Library/Formula/rhash.rb
@@ -3,7 +3,7 @@ require 'formula'
 class LionOrNewer < Requirement
   fatal true
 
-  satisfy(:build_env => false) { MacOS.version >= :lion }
+  satisfy MacOS.version >= :lion
 
   def message
     "rhash requires `wcsdup` which isn't in the SDK before Lion."
diff --git a/Library/Formula/shocco.rb b/Library/Formula/shocco.rb
index 572667344..81029c85a 100644
--- a/Library/Formula/shocco.rb
+++ b/Library/Formula/shocco.rb
@@ -10,7 +10,6 @@ end
 
 class MarkdownProvider < Requirement
   fatal true
-  env :userpaths
 
   satisfy { which 'markdown' }
 
diff --git a/Library/Formula/signing-party.rb b/Library/Formula/signing-party.rb
index 33c201055..cea84fbc9 100644
--- a/Library/Formula/signing-party.rb
+++ b/Library/Formula/signing-party.rb
@@ -2,7 +2,6 @@ require 'formula'
 
 class GnupgInstalled < Requirement
   fatal true
-  env :userpaths
 
   satisfy { which('gpg') || which('gpg2') }
 
diff --git a/Library/Homebrew/dependencies.rb b/Library/Homebrew/dependencies.rb
index c60bd3686..4a4cd6d91 100644
--- a/Library/Homebrew/dependencies.rb
+++ b/Library/Homebrew/dependencies.rb
@@ -184,13 +184,12 @@ class Requirement
   # Overriding #satisfied? is deprepcated.
   # Pass a block or boolean to the satisfied DSL method instead.
   def satisfied?
-    self.class.satisfy.yielder do |proc|
+    result = self.class.satisfy.yielder do |proc|
       instance_eval(&proc)
     end
-  rescue NoMethodError
-    self.class.satisfy || false
-  rescue ArgumentError
-    false
+
+    infer_env_modification(result)
+    !!result
   end
 
   # Overriding #fatal? is deprecated.
@@ -217,29 +216,45 @@ class Requirement
     message.hash
   end
 
+  private
+
+  def infer_env_modification(o)
+    case o
+    when Pathname
+      self.class.env do
+        unless ENV["PATH"].split(":").include?(o.parent.to_s)
+          append("PATH", o.parent, ":")
+        end
+      end
+    end
+  end
+
   class << self
     def fatal(val=nil)
       val.nil? ? @fatal : @fatal = val
     end
 
     def satisfy(options={}, &block)
-      if block_given?
-        @satisfied ||= Requirement::Satisfier.new(options, &block)
-      else
-        @satisfied ||= options
-      end
+      @satisfied ||= Requirement::Satisfier.new(options, &block)
     end
   end
 
   class Satisfier
     def initialize(options={}, &block)
-      @options = { :build_env => true }
-      @options.merge!(options)
+      case options
+      when Hash
+        @options = { :build_env => true }
+        @options.merge!(options)
+      else
+        @satisfied = options
+      end
       @proc = block
     end
 
     def yielder
-      if @options[:build_env]
+      if instance_variable_defined?(:@satisfied)
+        @satisfied
+      elsif @options[:build_env]
         require 'superenv'
         ENV.with_build_environment do
           ENV.userpaths!
diff --git a/Library/Homebrew/requirements.rb b/Library/Homebrew/requirements.rb
index 6bf082619..892de0341 100644
--- a/Library/Homebrew/requirements.rb
+++ b/Library/Homebrew/requirements.rb
@@ -216,7 +216,6 @@ end
 
 class MysqlInstalled < Requirement
   fatal true
-  env :userpaths
 
   satisfy { which 'mysql_config' }
 
@@ -238,7 +237,6 @@ end
 
 class PostgresqlInstalled < Requirement
   fatal true
-  env :userpaths
 
   satisfy { which 'pg_config' }
 
@@ -257,7 +255,6 @@ end
 
 class TeXInstalled < Requirement
   fatal true
-  env :userpaths
 
   satisfy { which('tex') || which('latex') }
 
diff --git a/Library/Homebrew/test/test_requirement.rb b/Library/Homebrew/test/test_requirement.rb
index 0319e9e25..e8803ed92 100644
--- a/Library/Homebrew/test/test_requirement.rb
+++ b/Library/Homebrew/test/test_requirement.rb
@@ -74,4 +74,17 @@ class RequirementTests < Test::Unit::TestCase
 
     assert req.satisfied?
   end
+
+  def test_infers_path_from_satisfy_result
+    which_path = Pathname.new("/foo/bar/baz")
+    req = Class.new(Requirement) do
+      satisfy { which_path }
+    end.new
+
+    ENV.expects(:with_build_environment).yields.returns(which_path)
+    ENV.expects(:userpaths!)
+    ENV.expects(:append).with("PATH", which_path.parent, ":")
+
+    req.modify_build_environment
+  end
 end