Browse files

reinstall cleanup

  • Loading branch information...
1 parent 4b320ee commit c54f879fbf53c7742d252c2534868538488a2fa9 @samueljohn samueljohn committed Sep 5, 2013
Showing with 18 additions and 46 deletions.
  1. +3 −8 Library/Homebrew/cmd/install.rb
  2. +15 −38 Library/Homebrew/cmd/reinstall.rb
View
11 Library/Homebrew/cmd/install.rb
@@ -19,13 +19,7 @@ def install
end unless ARGV.force?
perform_preinstall_checks
- ARGV.formulae.each do |f|
- begin
- install_formula(f)
- rescue CannotInstallFormulaError => e
- ofail e.message
- end
- end
+ ARGV.formulae.each { |f| install_formula(f) }
end
def check_ppc
@@ -86,6 +80,7 @@ def install_formula f
# another formula. In that case, don't generate an error, just move on.
rescue FormulaAlreadyInstalledError => e
opoo e.message
- # Ignore CannotInstallFormulaError and let caller handle it.
+ rescue CannotInstallFormulaError => e
+ ofail e.message
@manphiz
manphiz added a note Sep 5, 2013

@samueljohn The reason I need CannotInstallFormulaError pass on this exception is so that the caller know there is something wrong and should handle it. In install command it doesn't matter. But in reinstall, it should restore the "backup" keg. But without the exception it doesn't know and will just remove the backup.

A little example: cloog depends on isl. Now I unlink isl and try brew reinstall cloog, CannotInstallFormulaError raised but swallowed here, and reinstall code never gets an exception and proceed to remove cloog backup.

I'm reverting this file to make it work for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
end
end
View
53 Library/Homebrew/cmd/reinstall.rb
@@ -21,61 +21,38 @@ def reinstall
canonical_name = Formula.canonical_name(name)
formula = Formula.factory(canonical_name)
- if not formula.installed?
- if force_new_install?
- oh1 "Force installing new formula: #{name}"
- self.install_formula formula
- next
- else
- raise <<-EOS.undent
- #{formula} is not installed. Please install it first or use
- "--force-new-install" flag.
- EOS
- end
- end
-
- linked_keg_ref = HOMEBREW_REPOSITORY/'opt'/canonical_name
- keg = Keg.new(linked_keg_ref.realpath)
-
begin
oh1 "Reinstalling #{name} #{ARGV.options_only*' '}"
- quarantine keg
+ opt_link = HOMEBREW_PREFIX/'opt'/canonical_name
+ if opt_link.exist?
+ keg = Keg.new(opt_link.realpath)
+ backup keg
+ end
self.install_formula formula
rescue Exception => e
ofail e.message unless e.message.empty?
- restore_quarantine keg, formula
- raise 'Reinstallation abort.'
+ restore_backup keg, formula
+ raise 'Reinstall failed.'
else
- remove_quarantine keg
+ backup_path(keg).rmtree if backup_path(keg).exist?
end
end
end
- def force_new_install?
- ARGV.include? '--force-new-install'
- end
-
- def quarantine keg
+ def backup keg
keg.unlink
-
- path = Pathname.new(keg.to_s)
- path.rename quarantine_path(path)
+ keg.rename backup_path(keg)
end
- def restore_quarantine keg, formula
- path = Pathname.new(quarantine_path(keg))
+ def restore_backup keg, formula
+ path = backup_path(keg)
if path.directory?
- path.rename keg.to_s
+ path.rename keg
keg.link unless formula.keg_only?
end
end
- def remove_quarantine keg
- path = Pathname.new(quarantine_path(keg))
- path.rmtree
- end
-
- def quarantine_path path
- path.to_s + '.reinstall'
+ def backup_path path
+ Pathname.new "#{path}.reinstall"
end
end

0 comments on commit c54f879

Please sign in to comment.