brew test, install, update-test: add --keep-tmp option

Also enables sandbox for --interactive and --debug use of install
and test, using automatic retention.

Closes #66.

Signed-off-by: Andrew Janke <andrew@apjanke.net>

15 files changed, 143 insertions, 83 deletions

diff --git a/Library/Homebrew/build.rb b/Library/Homebrew/build.rb
index d898b84e0..1c9584719 100644
--- a/Library/Homebrew/build.rb
+++ b/Library/Homebrew/build.rb
@@ -105,7 +105,8 @@ class Build
 
     formula.extend(Debrew::Formula) if ARGV.debug?
 
-    formula.brew do
+    formula.brew do |_formula, staging|
+      staging.retain! if ARGV.keep_tmp?
       formula.patch
 
       if ARGV.git?
diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb
index 7e1edc237..22be53ad0 100644
--- a/Library/Homebrew/cmd/install.rb
+++ b/Library/Homebrew/cmd/install.rb
@@ -1,4 +1,4 @@
-#:  * `install` [`--debug`] [`--env=`<std>|<super>] [`--ignore-dependencies`] [`--only-dependencies`] [`--cc=`<compiler>] [`--build-from-source`|`--force-bottle`] [`--devel`|`--HEAD`] <formula>:
+#:  * `install` [`--debug`] [`--env=`<std>|<super>] [`--ignore-dependencies`] [`--only-dependencies`] [`--cc=`<compiler>] [`--build-from-source`|`--force-bottle`] [`--devel`|`--HEAD`] [`--keep-tmp`] <formula>:
 #:    Install <formula>.
 #:
 #:    <formula> is usually the name of the formula to install, but it can be specified
@@ -35,6 +35,9 @@
 #:    If `--HEAD` is passed, and <formula> defines it, install the HEAD version,
 #:    aka master, trunk, unstable.
 #:
+#:    If `--keep-tmp` is passed, the temporary files created for the test are
+#:    not deleted.
+#:
 #:    To install a newer version of HEAD use
 #:    `brew rm <foo> && brew install --HEAD <foo>`.
 #:
diff --git a/Library/Homebrew/cmd/postinstall.rb b/Library/Homebrew/cmd/postinstall.rb
index 5987fda16..95bd3f8ef 100644
--- a/Library/Homebrew/cmd/postinstall.rb
+++ b/Library/Homebrew/cmd/postinstall.rb
@@ -22,15 +22,11 @@ module Homebrew
     end
 
     if Sandbox.available? && ARGV.sandbox?
-      if Sandbox.auto_disable?
-        Sandbox.print_autodisable_warning
-      else
-        Sandbox.print_sandbox_message
-      end
+      Sandbox.print_sandbox_message
     end
 
     Utils.safe_fork do
-      if Sandbox.available? && ARGV.sandbox? && !Sandbox.auto_disable?
+      if Sandbox.available? && ARGV.sandbox?
         sandbox = Sandbox.new
         formula.logs.mkpath
         sandbox.record_log(formula.logs/"sandbox.postinstall.log")
diff --git a/Library/Homebrew/cmd/test-bot.rb b/Library/Homebrew/cmd/test-bot.rb
index 85c75d0ab..414558b2c 100644
--- a/Library/Homebrew/cmd/test-bot.rb
+++ b/Library/Homebrew/cmd/test-bot.rb
@@ -21,6 +21,7 @@
 # --verbose:       Print test step output in realtime. Has the side effect of passing output
 #                  as raw bytes instead of re-encoding in UTF-8.
 # --fast:          Don't install any packages, but run e.g. audit anyway.
+# --keep-tmp:      Keep temporary files written by main installs and tests that are run.
 #
 # --ci-master:           Shortcut for Homebrew master branch CI options.
 # --ci-pr:               Shortcut for Homebrew pull request CI options.
@@ -532,7 +533,12 @@ module Homebrew
       end
       test "brew", "fetch", "--retry", *fetch_args
       test "brew", "uninstall", "--force", formula_name if formula.installed?
-      install_args = ["--verbose"]
+
+      # shared_*_args are applied to both the main and --devel spec
+      shared_install_args = ["--verbose"]
+      shared_install_args << "--keep-tmp" if ARGV.keep_tmp?
+      # install_args is just for the main (stable, or devel if in a devel-only tap) spec
+      install_args = []
       install_args << "--build-bottle" if !ARGV.include?("--fast") && !ARGV.include?("--no-bottle") && !formula.bottle_disabled?
       install_args << "--HEAD" if ARGV.include? "--HEAD"
 
@@ -548,6 +554,7 @@ module Homebrew
         formula_bottled = formula.bottled?
       end
 
+      install_args.concat(shared_install_args)
       install_args << formula_name
       # Don't care about e.g. bottle failures for dependencies.
       install_passed = false
@@ -582,7 +589,9 @@ module Homebrew
             test "brew", "install", bottle_filename
           end
         end
-        test "brew", "test", "--verbose", formula_name if formula.test_defined?
+        shared_test_args = ["--verbose"]
+        shared_test_args << "--keep-tmp" if ARGV.keep_tmp?
+        test "brew", "test", formula_name, *shared_test_args if formula.test_defined?
         testable_dependents.each do |dependent|
           unless dependent.installed?
             test "brew", "fetch", "--retry", dependent.name
@@ -607,11 +616,13 @@ module Homebrew
          && !ARGV.include?("--HEAD") && !ARGV.include?("--fast") \
          && satisfied_requirements?(formula, :devel)
         test "brew", "fetch", "--retry", "--devel", *fetch_args
-        run_as_not_developer { test "brew", "install", "--devel", "--verbose", formula_name }
+        run_as_not_developer do
+          test "brew", "install", "--devel", formula_name, *shared_install_args
+        end
         devel_install_passed = steps.last.passed?
         test "brew", "audit", "--devel", *audit_args
         if devel_install_passed
-          test "brew", "test", "--devel", "--verbose", formula_name if formula.test_defined?
+          test "brew", "test", "--devel", formula_name, *shared_test_args if formula.test_defined?
           test "brew", "uninstall", "--devel", "--force", formula_name
         end
       end
diff --git a/Library/Homebrew/cmd/test.rb b/Library/Homebrew/cmd/test.rb
index ffb9c169d..0218e1c80 100644
--- a/Library/Homebrew/cmd/test.rb
+++ b/Library/Homebrew/cmd/test.rb
@@ -1,4 +1,4 @@
-#:  * `test` [`--devel`|`--HEAD`] [`--debug`] <formula>:
+#:  * `test` [`--devel`|`--HEAD`] [`--debug`] [`--keep-tmp`] <formula>:
 #:    A few formulae provide a test method. `brew test` <formula> runs this
 #:    test method. There is no standard output or return code, but it should
 #:    generally indicate to the user if something is wrong with the installed
@@ -10,6 +10,9 @@
 #:    If `--debug` is passed and the test fails, an interactive debugger will be
 #:    launched with access to IRB or a shell inside the temporary test directory.
 #:
+#:    If `--keep-tmp` is passed, the temporary files created for the test are
+#:    not deleted.
+#:
 #:    Example: `brew install jruby && brew test jruby`
 
 require "extend/ENV"
@@ -55,15 +58,11 @@ module Homebrew
         end
 
         if Sandbox.available? && !ARGV.no_sandbox?
-          if Sandbox.auto_disable?
-            Sandbox.print_autodisable_warning
-          else
-            Sandbox.print_sandbox_message
-          end
+          Sandbox.print_sandbox_message
         end
 
         Utils.safe_fork do
-          if Sandbox.available? && !ARGV.no_sandbox? && !Sandbox.auto_disable?
+          if Sandbox.available? && !ARGV.no_sandbox?
             sandbox = Sandbox.new
             f.logs.mkpath
             sandbox.record_log(f.logs/"sandbox.test.log")
diff --git a/Library/Homebrew/dev-cmd/update-test.rb b/Library/Homebrew/dev-cmd/update-test.rb
index edf4c542a..621c41526 100644
--- a/Library/Homebrew/dev-cmd/update-test.rb
+++ b/Library/Homebrew/dev-cmd/update-test.rb
@@ -5,6 +5,8 @@ module Homebrew
   #    brew update-test --commit=<sha1> # using <sha1> as start commit
   #    brew update-test --before=<date> # using commit at <date> as start commit
   #
+  # Options:
+  #   --keep-tmp      Retain temporary directory containing the new clone
   def update_test
     cd HOMEBREW_REPOSITORY
     start_sha1 = if commit = ARGV.value("commit")
@@ -19,7 +21,8 @@ module Homebrew
     puts "Start commit: #{start_sha1}"
     puts "End   commit: #{end_sha1}"
 
-    mktemp do
+    mktemp("update-test") do |staging|
+      staging.retain! if ARGV.keep_tmp?
       curdir = Pathname.new(Dir.pwd)
 
       oh1 "Setup test environment..."
diff --git a/Library/Homebrew/download_strategy.rb b/Library/Homebrew/download_strategy.rb
index 3532eeb6a..53abf6c40 100644
--- a/Library/Homebrew/download_strategy.rb
+++ b/Library/Homebrew/download_strategy.rb
@@ -17,7 +17,9 @@ class AbstractDownloadStrategy
   def fetch
   end
 
-  # Unpack {#cached_location} into the current working directory.
+  # Unpack {#cached_location} into the current working directory, and possibly
+  # chdir into the newly-unpacked directory.
+  # Unlike {Resource#stage}, this does not take a block.
   def stage
   end
 
diff --git a/Library/Homebrew/extend/ARGV.rb b/Library/Homebrew/extend/ARGV.rb
index 65be4bcf1..eec2172a2 100644
--- a/Library/Homebrew/extend/ARGV.rb
+++ b/Library/Homebrew/extend/ARGV.rb
@@ -120,6 +120,10 @@ module HomebrewArgvExtension
     include?("--dry-run") || switch?("n")
   end
 
+  def keep_tmp?
+    include? "--keep-tmp"
+  end
+
   def git?
     flag? "--git"
   end
diff --git a/Library/Homebrew/extend/fileutils.rb b/Library/Homebrew/extend/fileutils.rb
index ee6735e27..af17d4eff 100644
--- a/Library/Homebrew/extend/fileutils.rb
+++ b/Library/Homebrew/extend/fileutils.rb
@@ -6,42 +6,83 @@ require "etc"
 # @see http://ruby-doc.org/stdlib-1.8.7/libdoc/fileutils/rdoc/FileUtils.html Ruby's FileUtils API
 module FileUtils
   # Create a temporary directory then yield. When the block returns,
-  # recursively delete the temporary directory.
-  def mktemp(prefix = name)
-    prev = pwd
-    tmp  = Dir.mktmpdir(prefix, HOMEBREW_TEMP)
-
-    # Make sure files inside the temporary directory have the same group as the
-    # brew instance.
-    #
-    # Reference from `man 2 open`
-    # > When a new file is created, it is given the group of the directory which
-    # contains it.
-    group_id = if HOMEBREW_BREW_FILE.grpowned?
-      HOMEBREW_BREW_FILE.stat.gid
-    else
-      Process.gid
+  # recursively delete the temporary directory. Passing opts[:retain]
+  # or calling `do |staging| ... staging.retain!` in the block will skip
+  # the deletion and retain the temporary directory's contents.
+  def mktemp(prefix = name, opts = {})
+    Mktemp.new(prefix, opts).run do |staging|
+      yield staging
     end
-    begin
-      # group_id.to_s makes OS X 10.6.7 (ruby-1.8.7-p174) and earlier happy.
-      chown(nil, group_id.to_s, tmp)
-    rescue Errno::EPERM
-      opoo "Failed setting group \"#{Etc.getgrgid(group_id).name}\" on #{tmp}"
+  end
+
+  module_function :mktemp
+
+  # Performs mktemp's functionality, and tracks the results.
+  # Each instance is only intended to be used once.
+  class Mktemp
+    include FileUtils
+
+    # Path to the tmpdir used in this run, as a Pathname.
+    attr_reader :tmpdir
+
+    def initialize(prefix = name, opts = {})
+      @prefix = prefix
+      @retain = opts[:retain]
+      @quiet = false
+    end
+
+    # Instructs this Mktemp to retain the staged files
+    def retain!
+      @retain = true
+    end
+
+    # True if the staged temporary files should be retained
+    def retain?
+      @retain
     end
 
-    begin
-      cd(tmp)
+    # Instructs this Mktemp to not emit messages when retention is triggered
+    def quiet!
+      @quiet = true
+    end
+
+    def to_s
+      "[Mktemp: #{tmpdir} retain=#{@retain} quiet=#{@quiet}]"
+    end
 
+    def run
+      @tmpdir = Pathname.new(Dir.mktmpdir("#{@prefix}-", HOMEBREW_TEMP))
+
+      # Make sure files inside the temporary directory have the same group as the
+      # brew instance.
+      #
+      # Reference from `man 2 open`
+      # > When a new file is created, it is given the group of the directory which
+      # contains it.
+      group_id = if HOMEBREW_BREW_FILE.grpowned?
+                   HOMEBREW_BREW_FILE.stat.gid
+                 else
+                   Process.gid
+                 end
       begin
-        yield
+        # group_id.to_s makes OS X 10.6.7 (ruby-1.8.7-p174) and earlier happy.
+        chown(nil, group_id.to_s, tmpdir)
+      rescue Errno::EPERM
+        opoo "Failed setting group \"#{Etc.getgrgid(group_id).name}\" on #{tmp}"
+      end
+
+      begin
+        Dir.chdir(tmpdir) { yield self }
       ensure
-        cd(prev)
+        ignore_interrupts { rm_rf(tmpdir) } unless retain?
       end
     ensure
-      ignore_interrupts { rm_rf(tmp) }
+      if retain? && !@tmpdir.nil? && !@quiet
+        ohai "Kept temporary files"
+        puts "Temporary files retained at #{@tmpdir}"
+      end
     end
   end
-  module_function :mktemp
 
   # @private
   alias_method :old_mkdir, :mkdir
diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb
index b9cdc5486..4f5c56634 100644
--- a/Library/Homebrew/formula.rb
+++ b/Library/Homebrew/formula.rb
@@ -920,14 +920,19 @@ class Formula
     end
   end
 
-  # yields self with current working directory set to the uncompressed tarball
+  # yields |self,staging| with current working directory set to the uncompressed tarball
+  # where staging is a Mktemp staging context
   # @private
   def brew
-    stage do
+    stage do |staging|
+      staging.retain! if ARGV.keep_tmp?
       prepare_patches
 
       begin
-        yield self
+        yield self, staging
+      rescue StandardError
+        staging.retain! if ARGV.interactive? || ARGV.debug?
+        raise
       ensure
         cp Dir["config.log", "CMakeCache.txt"], logs
       end
@@ -1320,11 +1325,17 @@ class Formula
   def run_test
     old_home = ENV["HOME"]
     build, self.build = self.build, Tab.for_formula(self)
-    mktemp do
-      @testpath = Pathname.pwd
+    mktemp("#{name}-test") do |staging|
+      staging.retain! if ARGV.keep_tmp?
+      @testpath = staging.tmpdir
       ENV["HOME"] = @testpath
       setup_home @testpath
-      test
+      begin
+        test
+      rescue Exception
+        staging.retain! if ARGV.debug?
+        raise
+      end
     end
   ensure
     @testpath = nil
@@ -1537,7 +1548,7 @@ class Formula
   end
 
   def stage
-    active_spec.stage do
+    active_spec.stage do |_resource, staging|
       @source_modified_time = active_spec.source_modified_time
       @buildpath = Pathname.pwd
       env_home = buildpath/".brew_home"
@@ -1547,7 +1558,7 @@ class Formula
       setup_home env_home
 
       begin
-        yield
+        yield staging
       ensure
         @buildpath = nil
         ENV["HOME"] = old_home
diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb
index da63b7b32..3ce7117b0 100644
--- a/Library/Homebrew/formula_installer.rb
+++ b/Library/Homebrew/formula_installer.rb
@@ -523,6 +523,7 @@ class FormulaInstaller
     args << "--debug" if debug?
     args << "--cc=#{ARGV.cc}" if ARGV.cc
     args << "--default-fortran-flags" if ARGV.include? "--default-fortran-flags"
+    args << "--keep-tmp" if ARGV.keep_tmp?
 
     if ARGV.env
       args << "--env=#{ARGV.env}"
@@ -567,18 +568,14 @@ class FormulaInstaller
     ].concat(build_argv)
 
     if Sandbox.available? && ARGV.sandbox?
-      if Sandbox.auto_disable?
-        Sandbox.print_autodisable_warning
-      else
-        Sandbox.print_sandbox_message
-      end
+      Sandbox.print_sandbox_message
     end
 
     Utils.safe_fork do
       # Invalidate the current sudo timestamp in case a build script calls sudo
       system "/usr/bin/sudo", "-k"
 
-      if Sandbox.available? && ARGV.sandbox? && !Sandbox.auto_disable?
+      if Sandbox.available? && ARGV.sandbox?
         sandbox = Sandbox.new
         formula.logs.mkpath
         sandbox.record_log(formula.logs/"sandbox.build.log")
diff --git a/Library/Homebrew/resource.rb b/Library/Homebrew/resource.rb
index a15788fb3..5077ac149 100644
--- a/Library/Homebrew/resource.rb
+++ b/Library/Homebrew/resource.rb
@@ -72,6 +72,10 @@ class Resource
     downloader.clear_cache
   end
 
+  # Verifies download and unpacks it
+  # The block may call `|resource,staging| staging.retain!` to retain the staging
+  # directory. Subclasses that override stage should implement the tmp
+  # dir using FileUtils.mktemp so that works with all subtypes.
   def stage(target = nil, &block)
     unless target || block
       raise ArgumentError, "target directory or block is required"
@@ -81,15 +85,16 @@ class Resource
     unpack(target, &block)
   end
 
-  # If a target is given, unpack there; else unpack to a temp folder
-  # If block is given, yield to that block
-  # A target or a block must be given, but not both
+  # If a target is given, unpack there; else unpack to a temp folder.
+  # If block is given, yield to that block with |self, staging|, where staging
+  # is a staging context that responds to retain!().
+  # A target or a block must be given, but not both.
   def unpack(target = nil)
-    mktemp(download_name) do
+    mktemp(download_name) do |staging|
       downloader.stage
       @source_modified_time = downloader.source_modified_time
       if block_given?
-        yield self
+        yield self, staging
       elsif target
         target = Pathname.new(target) unless target.is_a? Pathname
         target.install Dir["*"]
diff --git a/Library/Homebrew/sandbox.rb b/Library/Homebrew/sandbox.rb
index e847744ad..501a43e4d 100644
--- a/Library/Homebrew/sandbox.rb
+++ b/Library/Homebrew/sandbox.rb
@@ -8,18 +8,6 @@ class Sandbox
     OS.mac? && File.executable?(SANDBOX_EXEC)
   end
 
-  # there are times the sandbox cannot be used.
-  def self.auto_disable?
-    @auto_disable ||= ARGV.interactive? || ARGV.debug?
-  end
-
-  def self.print_autodisable_warning
-    unless @printed_autodisable_warning
-      opoo "The sandbox cannot be used in debug or interactive mode."
-      @printed_autodisable_warning = true
-    end
-  end
-
   def self.print_sandbox_message
     unless @printed_sandbox_message
       ohai "Using the sandbox"
diff --git a/Library/Homebrew/test/test_patching.rb b/Library/Homebrew/test/test_patching.rb
index 8a18125ef..a87741807 100644
--- a/Library/Homebrew/test/test_patching.rb
+++ b/Library/Homebrew/test/test_patching.rb
@@ -122,7 +122,7 @@ class PatchingTests < Homebrew::TestCase
             url PATCH_URL_A
             sha256 PATCH_A_SHA256
           end
-        end.brew(&:patch)
+        end.brew { |f, _staging| f.patch }
       end
     end
   end
@@ -136,7 +136,7 @@ class PatchingTests < Homebrew::TestCase
             sha256 TESTBALL_PATCHES_SHA256
             apply APPLY_A
           end
-        end.brew(&:patch)
+        end.brew { |f, _staging| f.patch }
       end
     end
   end
@@ -234,7 +234,7 @@ class PatchingTests < Homebrew::TestCase
             sha256 TESTBALL_PATCHES_SHA256
             apply "patches/#{APPLY_A}"
           end
-        end.brew(&:patch)
+        end.brew { |f, _staging| f.patch }
       end
     end
   end
diff --git a/Library/Homebrew/utils.rb b/Library/Homebrew/utils.rb
index 86bc270c8..a6920a336 100644
--- a/Library/Homebrew/utils.rb
+++ b/Library/Homebrew/utils.rb
@@ -172,8 +172,7 @@ def interactive_shell(f = nil)
   if $?.success?
     return
   elsif $?.exited?
-    puts "Aborting due to non-zero exit status"
-    exit $?.exitstatus
+    raise "Aborted due to non-zero exit status (#{$?.exitstatus})"
   else
     raise $?.inspect
   end