Re-work ARGV filtering to properly handle --HEAD

Previously, stripping arguments like `--HEAD` for dependencies failed because
that flag affects the installation prefix encoded into formula objects. The
previous implementation of `ARGV` filtering tried to contain all changes to a
single method call before the `FormulaInstaller` forks. This update spreads
things out a bit:

  - The Homebrew `ARGV` extension adds a new method, `filter_for_dependencies`
    which strips flags like `--HEAD`, yields to a block, then restores the
    original contents of ARGV.

  - The `explicitly_requested?` test, which returns true or false depending on
    if a formula object is a member of `ARGV.formulae`, is now a method of
    `Formula` objects.

  - `FormulaInstaller` objects now execute the installation of dependencies
    inside an `ARGV.filter_for_dependencies` block if the dependency was
    `explicitly_requested?`.

Fixes #8668.
Closes #7724.

4 files changed, 59 insertions, 52 deletions

diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb
index 7142a5fcb..2848986d4 100644
--- a/Library/Homebrew/cmd/install.rb
+++ b/Library/Homebrew/cmd/install.rb
@@ -84,6 +84,10 @@ module Homebrew extend self
     unless formulae.empty?
       perform_preinstall_checks
       formulae.each do |f|
+        # Check formula status and skip if necessary---a formula passed on the
+        # command line may have been installed to satisfy a dependency.
+        next if f.installed?
+
         begin
           fi = FormulaInstaller.new(f)
           fi.install
diff --git a/Library/Homebrew/extend/ARGV.rb b/Library/Homebrew/extend/ARGV.rb
index e46c0b038..9a86e86ec 100644
--- a/Library/Homebrew/extend/ARGV.rb
+++ b/Library/Homebrew/extend/ARGV.rb
@@ -96,6 +96,24 @@ module HomebrewArgvExtension
     Homebrew.help_s
   end
 
+  def filter_for_dependencies
+    # Clears some flags that affect installation, yields to a block, then
+    # restores to original state.
+    old_args = clone
+
+    %w[
+      --debug -d
+      --fresh
+      --interactive -i
+      --verbose -v
+      --HEAD
+    ].each {|flag| delete flag}
+
+    yield
+
+    replace old_args
+  end
+
   private
 
   def downcased_unique_named
diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb
index bf930f529..9c63a35ff 100644
--- a/Library/Homebrew/formula.rb
+++ b/Library/Homebrew/formula.rb
@@ -148,6 +148,16 @@ class Formula
     return false
   end
 
+  def explicitly_requested?
+
+    # `ARGV.formulae` will throw an exception if it comes up with an empty
+    # list.
+    #
+    # FIXME: `ARGV.formulae` shouldn't be throwing exceptions, see issue #8823
+   return false if ARGV.named.empty?
+   ARGV.formulae.include? self
+  end
+
   def installed_prefix
     head_prefix = HOMEBREW_CELLAR+@name+'HEAD'
     if @version == 'HEAD' || head_prefix.directory?
diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb
index 61b2b608c..bd7c672c3 100644
--- a/Library/Homebrew/formula_installer.rb
+++ b/Library/Homebrew/formula_installer.rb
@@ -28,15 +28,17 @@ class FormulaInstaller
       needed_deps = f.recursive_deps.reject{ |d| d.installed? }
       unless needed_deps.empty?
         needed_deps.each do |dep|
-          fi = FormulaInstaller.new(dep)
-          fi.ignore_deps = true
-          fi.show_header = false
-          oh1 "Installing #{f} dependency: #{dep}"
-          fi.install
-          fi.caveats
-          fi.finish
+          if dep.explicitly_requested?
+            install_dependency dep
+          else
+            ARGV.filter_for_dependencies do
+              # Re-create the formula object so that args like `--HEAD` won't
+              # affect properties like the installation prefix.
+              dep = Formula.factory dep.name
+              install_dependency dep
+            end
+          end
         end
-
         # now show header as all the deps stuff has clouded the original issue
         show_header = true
       end
@@ -58,6 +60,16 @@ class FormulaInstaller
     raise "Nothing was installed to #{f.prefix}" unless f.installed?
   end
 
+  def install_dependency dep
+    fi = FormulaInstaller.new dep
+    fi.ignore_deps = true
+    fi.show_header = false
+    oh1 "Installing #{f} dependency: #{dep}"
+    fi.install
+    fi.caveats
+    fi.finish
+  end
+
   def caveats
     if f.caveats
       ohai "Caveats", f.caveats
@@ -105,7 +117,13 @@ class FormulaInstaller
     # I'm guessing this is not a good way to do this, but I'm no UNIX guru
     ENV['HOMEBREW_ERROR_PIPE'] = write.to_i.to_s
 
-    args = filtered_args
+    args = ARGV.clone
+    unless args.include? '--fresh'
+      previous_install = Tab.for_formula f
+      args.concat previous_install.used_options
+      args.uniq! # Just in case some dupes were added
+    end
+
     fork do
       begin
         read.close
@@ -239,49 +257,6 @@ class FormulaInstaller
       @show_summary_heading = true
     end
   end
-
-  private
-
-  # This method gives us a chance to pre-process command line arguments before the
-  # installer forks and `Formula.install` kicks in.
-  def filtered_args
-    # Returns true if the formula attached to this installer was explicitly
-    # passed on the command line by the user as opposed to being automatically
-    # added to satisfy a dependency.
-    def explicitly_requested?
-      # `ARGV.formulae` will throw an exception if it comes up with an empty
-      # list.
-      #
-      # FIXME:
-      # `ARGV.formulae` probably shouldn't be throwing exceptions, it should be
-      # the caller's responsibility to check `ARGV.formulae.empty?`.
-      return false if ARGV.named.empty?
-      ARGV.formulae.include? f
-    end
-
-    args = ARGV.clone
-
-    # FIXME: Also need to remove `--HEAD`, however there is a problem in that
-    # the properties of formula objects, such as `prefix`, are influenced by
-    # `--HEAD` and get set when the object is created.
-    #
-    # See issue #8668
-    %w[
-      --debug -d
-      --fresh
-      --interactive -i
-      --verbose -v
-    ].each {|flag| args.delete flag} unless explicitly_requested?
-
-    unless args.include? '--fresh'
-      previous_install = Tab.for_formula f
-      args.concat previous_install.used_options
-    end
-
-    args.uniq! # Just in case some dupes slipped by
-
-    return args
-  end
 end