brew install improvements

Couldn't make this atomic, apologies.

Fixes a few things, like deps failing to build not aborting the install.
--force now works properly again.

Overall more robust code. I went back over it all and gave it a lot of
thought.

Cleaner separation of logic. Less code in brew, now the only code there is
ARGV handling, and basic sanity checks.

Not extending ARGV or ENV in global now as that would propagate to other tools
or utilities you may write.

3 files changed, 149 insertions, 120 deletions

diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb
new file mode 100644
index 000000000..326d8ed51
--- /dev/null
+++ b/Library/Homebrew/formula_installer.rb
@@ -0,0 +1,79 @@
+require 'beer_events'
+require 'formula'
+require 'set'
+
+def ignore_interrupts
+  std_trap = trap("INT") {}
+  yield
+ensure
+  trap("INT", std_trap)
+end
+
+class FormulaInstaller
+  @@attempted = Set.new
+
+  def initialize
+    @install_deps = true
+  end
+
+  attr_writer :install_deps
+
+  def expand_deps f
+    deps = []
+    f.deps.collect do |dep|
+      dep = Formula.factory dep
+      deps += expand_deps dep
+      deps << dep
+    end
+    deps
+  end
+
+  def install f    
+    expand_deps(f).each do |dep|
+      begin
+        install_private dep unless dep.installed?
+      rescue
+        #TODO continue if this is an optional dep
+        raise
+      end
+    end if @install_deps
+    
+    install_private f
+  end
+  
+  private
+
+  def install_private f
+    return if @@attempted.include? f.name
+    @@attempted << f.name
+
+    # 1. formulae can modify ENV, so we must ensure that each
+    #    installation has a pristine ENV when it starts, forking now is 
+    #    the easiest way to do this
+    # 2. formulae have access to __END__ the only way to allow this is
+    #    to make the formula script the executed script
+    read, write = IO.pipe
+    # 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
+
+    watch_out_for_spill do
+      fork do
+        begin
+          read.close
+          exec '/usr/bin/ruby', '-I', File.dirname(__FILE__), '-rinstall', f.path, '--', *ARGV.options
+        rescue => e
+          Marshal.dump(e, write)
+          write.close
+          exit! 1
+        end
+      end
+      ignore_interrupts do
+        write.close
+        Process.wait
+        data = read.read
+        raise Marshal.load(data) unless data.nil? or data.empty?
+        raise "Suspicious installation failure" unless $?.success?
+      end
+    end
+  end
+end
diff --git a/Library/Homebrew/install.rb b/Library/Homebrew/install.rb
index a27cd2e4f..3c7ca6c90 100755
--- a/Library/Homebrew/install.rb
+++ b/Library/Homebrew/install.rb
@@ -3,19 +3,9 @@ BREW_FILE = `which brew`.strip
 
 require 'global'
 
-require 'brew.h'
-require 'extend/ENV'
-require 'fileutils'
-require 'formula'
-require 'hardware'
-require 'keg'
-
-
-show_summary_heading = false
-
 def text_for_keg_only_formula f
   if f.keg_only? == :provided_by_osx
-    rationale = "This because the formula is already provided by OS X."
+    rationale = "This is because the formula is already provided by OS X."
   elsif f.keg_only?.kind_of? String
     rationale = "The formula provides the following rationale:\n\n#{f.keg_only?.chomp}"
   else
@@ -29,16 +19,41 @@ Generally there are no consequences of this for you, however if you build your
 own software and it requires this formula, you may want to run this command to
 link it into the Homebrew prefix:
 
-     brew link #{f.name}
-EOS
+    brew link #{f.name}
+  EOS
 end
 
+# I like this little at all, but see no alternative seeing as the formula
+# rb file has to be the running script to allow it to use __END__ and DATA
+at_exit do
+  begin
+    require 'extend/ENV'
+    require 'fileutils'
+    require 'hardware'
+    require 'keg'
+    require 'brew.h.rb'
+
+    ENV.extend(HomebrewEnvExtension)
+    ENV.setup_build_environment
+
+    install(Formula.factory($0))
+  rescue Exception => e
+    if ENV['HOMEBREW_ERROR_PIPE']
+      pipe = IO.new(ENV['HOMEBREW_ERROR_PIPE'].to_i, 'w')
+      Marshal.dump(e, pipe)
+      pipe.close
+      exit! 1
+    else
+      onoe e
+      puts e.backtrace
+      exit! 2
+    end
+  end
+end
 
 def install f
-  # we deliberately only do this when install is run, although it may be the wrong decision…
-  ENV.extend(HomebrewEnvExtension)
-  ENV.setup_build_environment
-  
+  show_summary_heading = false
+
   f.deps.each do |dep|
     dep = Formula.factory dep
     if dep.keg_only?
@@ -51,7 +66,7 @@ def install f
 
   if ARGV.verbose?
     ohai "Build Environment"
-    %w[PATH CFLAGS LDFLAGS CPPFLAGS MAKEFLAGS CC CXX MACOSX_DEPLOYMENT_TARGET].each do |env|
+    %w[PATH CFLAGS LDFLAGS CPPFLAGS MAKEFLAGS CC CXX MACOSX_DEPLOYMENT_TARGET PKG_CONFIG_PATH].each do |env|
       puts "#{env}: #{ENV[env]}" unless ENV[env].to_s.empty?
     end
   end
@@ -93,7 +108,7 @@ def install f
   end
 
   ohai 'Finishing up' if ARGV.verbose?
-  
+
   begin
     clean f
   rescue Exception => e
@@ -138,21 +153,4 @@ def install f
   print "#{f.prefix}: #{f.prefix.abv}"
   print ", built in #{pretty_duration build_time}" if build_time
   puts
-
-rescue Exception => e
-  if ENV['HOMEBREW_ERROR_PIPE']
-    pipe = IO.new(ENV['HOMEBREW_ERROR_PIPE'].to_i, 'w')
-    Marshal.dump(e, pipe)
-    pipe.close
-    exit! 1
-  else
-    onoe e
-    puts e.backtrace
-    exit! 2
-  end
 end
-
-
-# I like this little at all, but see no alternative seeing as the formula
-# rb file has to be the running script to allow it to use __END__ and DATA
-at_exit { install(Formula.factory($0)) }
diff --git a/bin/brew b/bin/brew
index fc114514a..b7e255d84 100755
--- a/bin/brew
+++ b/bin/brew
@@ -47,9 +47,6 @@ end
 if MACOS_VERSION < 10.5
   abort "Homebrew requires Leopard or higher, but you could fork it and fix that..."
 end
-case Hardware.cpu_type when :ppc, :dunno
-  abort "Sorry, Homebrew does not support your computer's CPU architecture."
-end
 
 def dump_config
   puts <<-EOS
@@ -76,7 +73,7 @@ end
 begin
   require 'brew.h'
 
-  case ARGV.shift
+  case arg = ARGV.shift
   when '--prefix'
     puts HOMEBREW_PREFIX
   when '--config'
@@ -124,86 +121,6 @@ begin
         exec_editor *ARGV.formulae.collect {|f| f.path}
       end
 
-    when 'install'
-      if ARGV.named_empty?
-        puts "You must specify a formula. Search for available formulae with `brew search'."
-        exit 0
-      end
-      
-      # The Cellar lives under the repository
-      raise "Cannot write to #{HOMEBREW_REPOSITORY}" unless HOMEBREW_REPOSITORY.writable?
-      # The Prefix is where we symlink under
-      raise "Cannot write to #{HOMEBREW_PREFIX}" unless HOMEBREW_PREFIX.writable?
-
-      if ARGV.interactive? and ARGV.formulae.length > 1
-        # the reason for this is interactive mode is a little tricky to do 
-        # with more than one formula, AND I can't think of a time where you'd
-        # want to do it anyway. If someone comes up with a legitimate use for
-        # this we will adapt the code. "But I might want it!" is not a
-        # legitimate use!
-        raise "Interactive mode can only be used with one formula argument"
-      end
-
-      warn_about_macports_or_fink # keep warning before dependency resolution
-
-      unless ARGV.force?
-        formulae = ARGV.formulae.reject do |f|
-          if f.installed?
-            message = "Formula already installed: #{f.prefix}"
-            if ARGV.formulae.length > 1
-              opoo message
-            else
-              puts message # if only one is being installed a warning looks severe
-            end
-            true
-          end
-        end
-        exit 0 if formulae.empty?
-      else
-        formulae = ARGV.formulae
-      end
-
-      unless ARGV.include? '--ignore-dependencies'
-        deps = []
-        formulae.each { |f| deps += expand_deps f }
-        formulae = deps.reject { |f| f.installed? }
-      
-        if formulae.length > 1 and ARGV.interactive?
-          # because current code is a mess
-          raise "Please install the formula's dependencies before entering interactive mode"
-        end
-      end
-
-      require 'set'
-      done = Set.new
-
-      require 'beer_events'
-      watch_out_for_spill do
-        formulae.each do |f|
-          next if done.include? f.class
-          done << f.class
-
-          # 1. formulae can modify ENV, so we must ensure that each
-          #    installation has a pristine ENV when it starts, forking now is 
-          #    the easiest way to do this
-          # 2. formulae have access to __END__ the only way to allow this is
-          #    to make the formula script the executed script
-        read, write = IO.pipe
-        # 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
-
-        if not fork
-          read.close
-          exec '/usr/bin/ruby', '-I', homebrew_rubylib_path, '-rinstall', f.path, '--', *ARGV.options
-        else
-          write.close
-          data = read.read
-          raise Marshal.load(data) unless data.nil? or data.empty?
-          Process.wait
-        end
-      end
-    end
-
     when 'up', 'update'
       require 'update'
       updater = RefreshBrew.new
@@ -262,6 +179,41 @@ begin
         ARGV.named.each {|name| info name}
       end
 
+  when 'install'
+    require 'formula_installer'
+
+    ############################################################ sanity checks
+    case Hardware.cpu_type when :ppc, :dunno
+      abort "Sorry, Homebrew does not support your computer's CPU architecture."
+    end
+
+    raise "Cannot write to #{HOMEBREW_CELLAR}" unless HOMEBREW_CELLAR.writable?
+    raise "Cannot write to #{HOMEBREW_PREFIX}" unless HOMEBREW_PREFIX.writable?
+
+    ################################################################# warnings
+    if MACOS_VERSION >= 10.6
+      `/Developer/usr/bin/llvm-gcc-4.2 -v 2>&1` =~ /LLVM build (\d{4,})/
+      opoo "You should upgrade to Xcode 3.2.1" if $1.to_i < 2206
+    end
+
+    if macports_or_fink_installed?
+      opoo "It appears you have Macports or Fink installed"
+      puts "Although, unlikely, this can break builds or cause obscure runtime issues."
+      puts "If you experience problems try uninstalling these tools."
+    end
+
+    ################################################################# install!
+    installer = FormulaInstaller.new
+    installer.install_deps = !ARGV.include?('--ignore-dependencies')
+
+    ARGV.formulae.each do |f|
+      if not f.installed? or ARGV.force?
+        installer.install f
+      else
+        puts "Formula already installed: #{f.prefix}"
+      end
+    end
+
   when 'log'
     Dir.chdir HOMEBREW_PREFIX
     exec "git", "log", ARGV.formulae.first.path, *ARGV.options