Clean up and improve build-error output and logs

All logs are now stored from each command executed in Formula.install.

Error output is truncated to five lines in an attempt to not overwhelm the user and to encourage users to read the error output and report the bug properly. Maybe we can get that figure up from 70% to 90%.

6 files changed, 51 insertions, 80 deletions

diff --git a/Library/Homebrew/build.rb b/Library/Homebrew/build.rb
index c6196827d..9d2b8e93b 100755
--- a/Library/Homebrew/build.rb
+++ b/Library/Homebrew/build.rb
@@ -128,7 +128,6 @@ def install f
       end
 
       interactive_shell f
-      nil
     else
       f.prefix.mkpath
       f.install
diff --git a/Library/Homebrew/exceptions.rb b/Library/Homebrew/exceptions.rb
index e955a99ba..b589e3456 100644
--- a/Library/Homebrew/exceptions.rb
+++ b/Library/Homebrew/exceptions.rb
@@ -95,54 +95,13 @@ class BuildError < Homebrew::InstallationError
   end
 
   def dump
-    e = self
-
-    require 'cmd/--config'
-    require 'cmd/--env'
-
-    e.backtrace[1] =~ %r{Library/Formula/(.+)\.rb:(\d+)}
-    formula_name = $1
-    error_line = $2
-
-    path = HOMEBREW_REPOSITORY/"Library/Formula/#{formula_name}.rb"
-    if path.symlink? and path.realpath.to_s =~ %r{^#{HOMEBREW_REPOSITORY}/Library/Taps/(\w+)-(\w+)/}
-      repo = "#$1/homebrew-#$2"
-      repo_path = path.realpath.relative_path_from(HOMEBREW_REPOSITORY/"Library/Taps/#$1-#$2").parent.to_s
-      issues_url = "https://github.com/#$1/homebrew-#$2/issues/new"
-    else
-      repo = "mxcl/master"
-      repo_path = "Library/Formula"
-      issues_url = ISSUES_URL
-    end
-
-    if ARGV.verbose?
-      ohai "Exit Status: #{e.exit_status}"
-      puts "https://github.com/#{repo}/blob/master/#{repo_path}/#{formula_name}.rb#L#{error_line}"
-    end
-    ohai "Build Environment"
-    Homebrew.dump_build_config
-    puts %["--use-clang" was specified] if ARGV.include? '--use-clang'
-    puts %["--use-llvm" was specified] if ARGV.include? '--use-llvm'
-    puts %["--use-gcc" was specified] if ARGV.include? '--use-gcc'
-    Homebrew.dump_build_env e.env
-    puts
-    onoe "#{e.to_s.strip} (#{formula_name}.rb:#{error_line})"
-    issues = GitHub.issues_for_formula formula_name
+    logs = "#{ENV['HOME']}/Library/Logs/Homebrew/#{formula}/"
     puts
-    if issues.empty?
-      puts "This link will help resolve the above errors:"
-      puts "    #{Tty.em}#{issues_url}#{Tty.reset}"
-    else
-      puts "These existing issues may help you:", *issues.map{ |s| "    #{Tty.em}#{s}#{Tty.reset}" }
-      puts "Otherwise, this may help you fix or report the issue:"
-      puts "    #{Tty.em}#{issues_url}#{Tty.reset}"
-    end
-    if e.was_running_configure?
-      puts "We saved the configure log:"
-      puts "    ~/Library/Logs/Homebrew/config.log"
-      puts "When you report the issue please paste the build output above and the config.log here:"
-      puts "    #{Tty.em}http://gist.github.com/#{Tty.reset}"
-    end
+    onoe "#{formula.name} did not build"
+    puts "Logs: #{logs}" unless Dir["#{logs}/*"].empty?
+    puts "Help: #{Tty.em}https://github.com/mxcl/homebrew/wiki/troubleshooting#{Tty.reset}"
+    issues = GitHub.issues_for_formula(formula.name)
+    puts *issues.map{ |s| "      #{Tty.em}#{s}#{Tty.reset}" } unless issues.empty?
   end
 end
 
diff --git a/Library/Homebrew/extend/fileutils.rb b/Library/Homebrew/extend/fileutils.rb
index 6a6d8585a..5531f6e6e 100644
--- a/Library/Homebrew/extend/fileutils.rb
+++ b/Library/Homebrew/extend/fileutils.rb
@@ -14,7 +14,7 @@ module FileUtils extend self
     # /tmp volume to the other volume. So we let the user override the tmp
     # prefix if they need to.
     tmp = ENV['HOMEBREW_TEMP'].chuzzle || '/tmp'
-    tempd = `/usr/bin/mktemp -d #{tmp}/brew-#{name}-#{version}-XXXX`.chuzzle
+    tempd = `/usr/bin/mktemp -d #{tmp}/#{name}-XXXX`.chuzzle
     raise "Failed to create sandbox" if tempd.nil?
     prevd = pwd
     cd tempd
diff --git a/Library/Homebrew/formula.rb b/Library/Homebrew/formula.rb
index bf784d0ce..78269c88d 100644
--- a/Library/Homebrew/formula.rb
+++ b/Library/Homebrew/formula.rb
@@ -223,25 +223,17 @@ class Formula
         # we allow formulas to do anything they want to the Ruby process
         # so load any deps before this point! And exit asap afterwards
         yield self
-      rescue Interrupt, RuntimeError, SystemCallError => e
-        puts if Interrupt === e # don't print next to the ^C
-        unless ARGV.debug?
-          %w(config.log CMakeCache.txt).select{|f| File.exist? f}.each do |f|
-            HOMEBREW_LOGS.install f
-            puts "#{f} was copied to #{HOMEBREW_LOGS}"
+      rescue RuntimeError, SystemCallError => e
+        if not ARGV.debug?
+          %w(config.log CMakeCache.txt).each do |fn|
+            (HOMEBREW_LOGS/name).install(fn) if File.file?(fn)
           end
           raise
         end
-        onoe e.inspect
-        puts e.backtrace
 
+        onoe e.inspect
+        puts e.backtrace unless e.kind_of? BuildError
         ohai "Rescuing build..."
-        if (e.was_running_configure? rescue false) and File.exist? 'config.log'
-          puts "It looks like an autotools configure failed."
-          puts "Gist 'config.log' and any error output when reporting an issue."
-          puts
-        end
-
         puts "When you exit this shell Homebrew will attempt to finalise the installation."
         puts "If nothing is installed or the shell exits with a non-zero error code,"
         puts "Homebrew will abort. The installation prefix is:"
@@ -529,23 +521,30 @@ protected
     if ARGV.verbose?
       safe_system cmd, *args
     else
+      @exec_count ||= 0
+      @exec_count += 1
+      logd = HOMEBREW_LOGS/name
+      logfn = "#{logd}/%02d.%s" % [@exec_count, File.basename(cmd).split(' ').first]
+      mkdir_p(logd)
+
       rd, wr = IO.pipe
       pid = fork do
+        ENV['VERBOSE'] = '1' # helps with many tool's logging outputs
         rd.close
         $stdout.reopen wr
         $stderr.reopen wr
         args.collect!{|arg| arg.to_s}
         exec(cmd, *args) rescue nil
+        puts "Failed to execute: #{cmd}"
         exit! 1 # never gets here unless exec threw or failed
       end
       wr.close
-      out = ''
-      out << rd.read until rd.eof?
+
+      f = File.open(logfn, 'w')
+      f.write(rd.read) until rd.eof?
+
       Process.wait
-      unless $?.success?
-        puts out
-        raise
-      end
+      raise unless $?.success?
     end
 
     removed_ENV_variables.each do |key, value|
@@ -553,7 +552,21 @@ protected
     end if removed_ENV_variables
 
   rescue
+    if f
+      f.flush
+      Kernel.system "/usr/bin/tail -n 5 #{logfn}"
+      require 'cmd/--config'
+      $f = f
+      def Homebrew.puts(*foo); $f.puts *foo end
+      f.puts
+      Homebrew.dump_build_config
+      class << Homebrew; undef :puts end
+    else
+      puts "No logs recorded :(" unless ARGV.verbose?
+    end
     raise BuildError.new(self, cmd, args, $?)
+  ensure
+    f.close if f
   end
 
 public
diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb
index d9cff505c..213704107 100644
--- a/Library/Homebrew/formula_installer.rb
+++ b/Library/Homebrew/formula_installer.rb
@@ -200,6 +200,8 @@ class FormulaInstaller
   end
 
   def build
+    FileUtils.rm Dir["#{HOMEBREW_LOGS}/#{f}/*"]
+
     @start_time = Time.now
 
     # 1. formulae can modify ENV, so we must ensure that each
@@ -236,7 +238,7 @@ class FormulaInstaller
       end
     end
 
-    ignore_interrupts do # the fork will receive the interrupt and marshall it back
+    ignore_interrupts(:quietly) do # the fork will receive the interrupt and marshall it back
       write.close
       Process.wait
       data = read.read
@@ -245,18 +247,14 @@ class FormulaInstaller
       raise "Suspicious installation failure" unless $?.success?
     end
 
-    # This is the installation receipt. The reason this comment is necessary
-    # is because some numpty decided to call the class Tab rather than
-    # the far more appropriate InstallationReceipt :P
-    Tab.for_install(f, args).write
+    raise "Empty installation" if Dir["#{f.prefix}/*"].empty?
+
+    Tab.for_install(f, args).write # INSTALL_RECEIPT.json
 
   rescue Exception => e
     ignore_interrupts do
       # any exceptions must leave us with nothing installed
-      if f.prefix.directory?
-        puts "One sec, just cleaning up..." if e.kind_of? Interrupt
-        f.prefix.rmtree
-      end
+      f.prefix.rmtree if f.prefix.directory?
       f.rack.rmdir_if_possible
     end
     raise
diff --git a/Library/Homebrew/utils.rb b/Library/Homebrew/utils.rb
index 0970f99fe..ff301bbcf 100644
--- a/Library/Homebrew/utils.rb
+++ b/Library/Homebrew/utils.rb
@@ -216,8 +216,10 @@ def inreplace path, before=nil, after=nil
   end
 end
 
-def ignore_interrupts
-  std_trap = trap("INT") { puts "One sec, just cleaning up" }
+def ignore_interrupts(opt = nil)
+  std_trap = trap("INT") do
+    puts "One sec, just cleaning up" unless opt = :quietly
+  end
   yield
 ensure
   trap("INT", std_trap)