Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

install, upgrade: check if formula can be installed before fetching #12024

Merged
merged 3 commits into from
Sep 12, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion Library/Homebrew/formula_installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,12 @@ def verify_deps_exist
raise
end

def check_install_sanity
def check_installation_already_attempted
raise FormulaInstallationAlreadyAttemptedError, formula if self.class.attempted.include?(formula)
end

def check_install_sanity
check_installation_already_attempted

if force_bottle? && !pour_bottle?
raise CannotInstallFormulaError, "--force-bottle passed but #{formula.full_name} has no bottle!"
Expand Down
74 changes: 30 additions & 44 deletions Library/Homebrew/install.rb
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,28 @@ def install_formula?(
Or to force-install it, run:
brew install #{f} --force
EOS
elsif f.linked?
message = "#{f.name} #{f.linked_version} is already installed"
if f.outdated? && !head
unless Homebrew::EnvConfig.no_install_upgrade?
puts "#{message} but outdated"
return true
end

onoe <<~EOS
#{message}
To upgrade to #{f.pkg_version}, run:
brew upgrade #{f.full_name}
EOS
elsif only_dependencies
return true
else
onoe <<~EOS
#{message}
To install #{f.pkg_version}, first run:
brew unlink #{f.name}
EOS
end
else
# If none of the above is true and the formula is linked, then
# FormulaInstaller will handle this case.
Expand Down Expand Up @@ -272,69 +294,33 @@ def install_formulae(
)

begin
fi.prelude
fi.fetch
fi
rescue CannotInstallFormulaError => e
ofail e.message
rescue UnsatisfiedRequirements, DownloadError, ChecksumMismatchError => e
ofail "#{f}: #{e}"
nil
end
end.compact

formula_installers.each do |fi|
install_formula(fi, only_deps: only_deps)
install_formula(fi)
Cleanup.install_formula_clean!(fi.formula)
end
end

def install_formula(formula_installer, only_deps: false)
def install_formula(formula_installer)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if any logic can be shared with the upgrade.rb version? Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added Upgrade.install_formula

f = formula_installer.formula

formula_installer.prelude
formula_installer.check_installation_already_attempted
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain a bit more about this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above ⬆️


f.print_tap_action

if f.linked_keg.directory?
if Homebrew::EnvConfig.no_install_upgrade?
message = <<~EOS
#{f.name} #{f.linked_version} is already installed
EOS
message += if f.outdated? && !f.head?
<<~EOS
To upgrade to #{f.pkg_version}, run:
brew upgrade #{f.full_name}
EOS
else
<<~EOS
To install #{f.pkg_version}, first run:
brew unlink #{f.name}
EOS
end
raise CannotInstallFormulaError, message unless only_deps
elsif f.outdated? && !f.head?
puts "#{f.name} #{f.linked_version} is installed but outdated"
kegs = Upgrade.outdated_kegs(f)
linked_kegs = kegs.select(&:linked?)
Upgrade.print_upgrade_message(f, formula_installer.options)
end
end
upgrade = f.linked? && f.outdated? && !f.head? && !Homebrew::EnvConfig.no_install_upgrade?

kegs.each(&:unlink) if kegs.present?

formula_installer.install
formula_installer.finish
rescue FormulaInstallationAlreadyAttemptedError
# We already attempted to install f as part of the dependency tree of
# another formula. In that case, don't generate an error, just move on.
nil
rescue CannotInstallFormulaError => e
ofail e.message
ensure
# Re-link kegs if upgrade fails
begin
linked_kegs.each(&:link) if linked_kegs.present? && !f.latest_version_installed?
rescue
nil
end
Upgrade.install_formula(formula_installer, upgrade: upgrade)
end
private_class_method :install_formula
end
Expand Down
41 changes: 26 additions & 15 deletions Library/Homebrew/upgrade.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,13 @@ def upgrade_formulae(
quiet: quiet,
verbose: verbose,
)
fi.fetch unless dry_run
unless dry_run
fi.prelude
fi.fetch
end
fi
rescue CannotInstallFormulaError => e
ofail e
rescue UnsatisfiedRequirements, DownloadError => e
ofail "#{formula}: #{e}"
nil
Expand Down Expand Up @@ -159,44 +164,50 @@ def create_formula_installer(
def upgrade_formula(formula_installer, dry_run: false, verbose: false)
formula = formula_installer.formula

kegs = outdated_kegs(formula)
linked_kegs = kegs.select(&:linked?)

if dry_run
print_dry_run_dependencies(formula, formula_installer.compute_dependencies)
return
end

formula_installer.prelude
formula_installer.check_installation_already_attempted
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain a bit more about this change?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since prelude already gets invoked before fetch, here we just need to check for FormulaInstallationAlreadyAttemptedError. In a previous draft, I made this a boolean method since the error was always being rescued—except in Cask::Installer (which is probably a bug).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here we just need to check for FormulaInstallationAlreadyAttemptedError

Thanks. Sorry, why do we need to check this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that that error gets raised when we've already attempted to install the formula as part of the dependency tree of another formula. #12018 most likely makes this unnecessary for brew upgrade, but there's still brew install, brew reinstall, and Cask::Installer that need to be updated.


install_formula(formula_installer, upgrade: true)
rescue BuildError => e
e.dump(verbose: verbose)
puts
Homebrew.failed = true
end
private_class_method :upgrade_formula

def install_formula(formula_installer, upgrade:)
formula = formula_installer.formula

print_upgrade_message(formula, formula_installer.options)
if upgrade
print_upgrade_message(formula, formula_installer.options)

kegs = outdated_kegs(formula)
linked_kegs = kegs.select(&:linked?)
end

# first we unlink the currently active keg for this formula otherwise it is
# possible for the existing build to interfere with the build we are about to
# do! Seriously, it happens!
kegs.each(&:unlink)
kegs.each(&:unlink) if kegs.present?

formula_installer.install
formula_installer.finish
rescue FormulaInstallationAlreadyAttemptedError
# We already attempted to upgrade f as part of the dependency tree of
# another formula. In that case, don't generate an error, just move on.
nil
rescue CannotInstallFormulaError => e
ofail e
rescue BuildError => e
e.dump(verbose: verbose)
puts
Homebrew.failed = true
ensure
# restore previous installation state if build failed
begin
linked_kegs.each(&:link) unless formula.latest_version_installed?
linked_kegs.each(&:link) if linked_kegs.present? && !f.latest_version_installed?
rescue
nil
end
end
private_class_method :upgrade_formula

def check_broken_dependents(installed_formulae)
CacheStoreDatabase.use(:linkage) do |db|
Expand Down