From 3725f771de576ca7324fd566678e3416e270a8a7 Mon Sep 17 00:00:00 2001
From: Jack Nagel <jacknagel@gmail.com>
Date: Tue, 22 Jan 2013 14:33:33 -0600
Subject: [PATCH] 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.
---
 Library/Homebrew/dependencies.rb          | 41 ++++++++++++++++-------
 Library/Homebrew/requirements.rb          |  3 --
 Library/Homebrew/test/test_requirement.rb | 13 +++++++
 3 files changed, 41 insertions(+), 16 deletions(-)

diff --git a/Library/Homebrew/dependencies.rb b/Library/Homebrew/dependencies.rb
index c60bd36860..4a4cd6d91f 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 6bf0826197..892de03416 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 0319e9e252..e8803ed92e 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