From 794a55a72b78acf4c5866f20ff39e6fcc759e439 Mon Sep 17 00:00:00 2001 From: Max Howell Date: Mon, 26 Oct 2009 18:13:38 +0000 Subject: [PATCH] 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. --- Library/Homebrew/formula_installer.rb | 79 +++++++++++++++++ Library/Homebrew/install.rb | 70 ++++++++------- bin/brew | 120 ++++++++------------------ 3 files changed, 149 insertions(+), 120 deletions(-) create mode 100644 Library/Homebrew/formula_installer.rb diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb new file mode 100644 index 0000000000..326d8ed513 --- /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 a27cd2e4f2..3c7ca6c90c 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 fc114514a0..b7e255d840 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