From a217b03952fca6df22ecc62523d9c85883aef856 Mon Sep 17 00:00:00 2001 From: Max Howell Date: Tue, 11 Sep 2012 20:59:59 -0400 Subject: [PATCH] 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%. --- Library/ENV/4.3/cc | 7 ++-- Library/Homebrew/build.rb | 1 - Library/Homebrew/exceptions.rb | 53 +++------------------------ Library/Homebrew/extend/fileutils.rb | 2 +- Library/Homebrew/formula.rb | 53 +++++++++++++++++---------- Library/Homebrew/formula_installer.rb | 16 ++++---- Library/Homebrew/utils.rb | 6 ++- 7 files changed, 55 insertions(+), 83 deletions(-) diff --git a/Library/ENV/4.3/cc b/Library/ENV/4.3/cc index 37ebc0400d..b027c789aa 100755 --- a/Library/ENV/4.3/cc +++ b/Library/ENV/4.3/cc @@ -168,9 +168,10 @@ class Cmd dels = @args - args adds = args - @args dups = dels & args - puts "brew: Superenv removed: #{dels*' '}" unless dels.empty? - puts "brew: Superenv deduped: #{dels}" unless dups.empty? - puts "brew: Superenv added: #{adds*' '}" unless adds.empty? + + STDERR.puts "brew: superenv removed: #{dels*' '}" unless dels.empty? + STDERR.puts "brew: superenv deduped: #{dels}" unless dups.empty? + STDERR.puts "brew: superenv added: #{adds*' '}" unless adds.empty? end end diff --git a/Library/Homebrew/build.rb b/Library/Homebrew/build.rb index c6196827da..9d2b8e93b5 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 e955a99ba3..b589e3456a 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 + logs = "#{ENV['HOME']}/Library/Logs/Homebrew/#{formula}/" puts - onoe "#{e.to_s.strip} (#{formula_name}.rb:#{error_line})" - issues = GitHub.issues_for_formula formula_name - 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 6a6d8585a9..5531f6e6e3 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 bf784d0ce5..78269c88db 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 - + 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 d9cff505c1..213704107b 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 0970f99fee..ff301bbcf2 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)