Skip to content

Commit

Permalink
Revamp installed_on_request handling
Browse files Browse the repository at this point in the history
- `reinstall` and `upgrade` no longer mark as installed on request,
  with or without names specified, but preserve the version from the
  tab instead
- default `install_on_request` to `false` rather than `true`
- only set installed in request in a tab if it's missing rather than
  false

Co-authored-by: Michael Cho <[email protected]>
  • Loading branch information
MikeMcQuaid and cho-m committed Nov 19, 2024
1 parent ad2e330 commit 4d4531c
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 28 deletions.
2 changes: 0 additions & 2 deletions Library/Homebrew/cmd/reinstall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,6 @@ def run
Homebrew::Reinstall.reinstall_formula(
formula,
flags: args.flags_only,
installed_on_request: args.named.present?,
force_bottle: args.force_bottle?,
build_from_source_formulae: args.build_from_source_formulae,
interactive: args.interactive?,
Expand All @@ -156,7 +155,6 @@ def run
Upgrade.check_installed_dependents(
formulae,
flags: args.flags_only,
installed_on_request: args.named.present?,
force_bottle: args.force_bottle?,
build_from_source_formulae: args.build_from_source_formulae,
interactive: args.interactive?,
Expand Down
2 changes: 0 additions & 2 deletions Library/Homebrew/cmd/upgrade.rb
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ def upgrade_outdated_formulae(formulae)
formulae_to_install,
flags: args.flags_only,
dry_run: args.dry_run?,
installed_on_request: args.named.present?,
force_bottle: args.force_bottle?,
build_from_source_formulae: args.build_from_source_formulae,
interactive: args.interactive?,
Expand All @@ -237,7 +236,6 @@ def upgrade_outdated_formulae(formulae)
formulae_to_install,
flags: args.flags_only,
dry_run: args.dry_run?,
installed_on_request: args.named.present?,
force_bottle: args.force_bottle?,
build_from_source_formulae: args.build_from_source_formulae,
interactive: args.interactive?,
Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/formula_installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ def initialize(
formula,
link_keg: false,
installed_as_dependency: false,
installed_on_request: true,
installed_on_request: false,
show_header: false,
build_bottle: false,
skip_post_install: false,
Expand Down
2 changes: 2 additions & 0 deletions Library/Homebrew/install.rb
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,8 @@ def install_formulae(
formula_installer = FormulaInstaller.new(
formula,
options: build_options.used_options,
installed_on_request: true,
installed_as_dependency: false,
build_bottle:,
force_bottle:,
bottle_arch:,
Expand Down
22 changes: 14 additions & 8 deletions Library/Homebrew/reinstall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ module Reinstall
def self.reinstall_formula(
formula,
flags:,
installed_on_request: false,
force_bottle: false,
build_from_source_formulae: [],
interactive: false,
Expand All @@ -25,9 +24,16 @@ def self.reinstall_formula(
if formula.opt_prefix.directory?
keg = Keg.new(formula.opt_prefix.resolved_path)
tab = keg.tab
keg_had_linked_opt = true
keg_was_linked = keg.linked?
link_keg = keg.linked?
installed_as_dependency = tab.installed_as_dependency
installed_on_request = tab.installed_on_request
build_bottle = tab.built_bottle?
backup keg
else
link_keg = nil
installed_as_dependency = false
installed_on_request = true
build_bottle = false

Check warning on line 36 in Library/Homebrew/reinstall.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/reinstall.rb#L34-L36

Added lines #L34 - L36 were not covered by tests
end

build_options = BuildOptions.new(Options.create(flags), formula.options)
Expand All @@ -39,10 +45,10 @@ def self.reinstall_formula(
formula,
**{
options:,
link_keg: keg_had_linked_opt ? keg_was_linked : nil,
installed_as_dependency: tab&.installed_as_dependency,
installed_on_request: installed_on_request || tab&.installed_on_request,
build_bottle: tab&.built_bottle?,
link_keg:,
installed_as_dependency:,
installed_on_request:,
build_bottle:,
force_bottle:,
build_from_source_formulae:,
git:,
Expand All @@ -65,7 +71,7 @@ def self.reinstall_formula(
rescue FormulaInstallationAlreadyAttemptedError
nil
rescue Exception # rubocop:disable Lint/RescueException
ignore_interrupts { restore_backup(keg, keg_was_linked, verbose:) }
ignore_interrupts { restore_backup(keg, link_keg, verbose:) }

Check warning on line 74 in Library/Homebrew/reinstall.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/reinstall.rb#L74

Added line #L74 was not covered by tests
raise
else
begin
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ class #{Formulary.class_s(name)} < Formula

def install_test_formula(name, content = nil, build_bottle: false)
setup_test_formula(name, content)
fi = FormulaInstaller.new(Formula[name], build_bottle:)
fi = FormulaInstaller.new(Formula[name], build_bottle:, installed_on_request: true)
fi.prelude
fi.fetch
fi.install
Expand Down
32 changes: 18 additions & 14 deletions Library/Homebrew/upgrade.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ def self.upgrade_formulae(
formulae_to_install,
flags:,
dry_run: false,
installed_on_request: false,
force_bottle: false,
build_from_source_formulae: [],
dependents: false,
Expand Down Expand Up @@ -55,7 +54,6 @@ def self.upgrade_formulae(
fi = create_formula_installer(
formula,
flags:,
installed_on_request:,
force_bottle:,
build_from_source_formulae:,
interactive:,
Expand Down Expand Up @@ -114,7 +112,6 @@ def self.upgrade_formulae(
private_class_method def self.create_formula_installer(
formula,
flags:,
installed_on_request: false,
force_bottle: false,
build_from_source_formulae: [],
interactive: false,
Expand All @@ -126,15 +123,23 @@ def self.upgrade_formulae(
quiet: false,
verbose: false
)
if formula.opt_prefix.directory?
keg = Keg.new(formula.opt_prefix.resolved_path)
keg_had_linked_opt = true
keg_was_linked = keg.linked?
keg = if formula.optlinked?
Keg.new(formula.opt_prefix.resolved_path)
else
formula.installed_kegs.find(&:optlinked?)
end

if formula.opt_prefix.directory?
keg = Keg.new(formula.opt_prefix.resolved_path)
if keg
tab = keg.tab
link_keg = keg.linked?
installed_as_dependency = tab.installed_as_dependency
installed_on_request = tab.installed_on_request
build_bottle = tab.built_bottle?

Check warning on line 137 in Library/Homebrew/upgrade.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/upgrade.rb#L134-L137

Added lines #L134 - L137 were not covered by tests
else
link_keg = nil
installed_as_dependency = false
installed_on_request = true
build_bottle = false
end

build_options = BuildOptions.new(Options.create(flags), formula.options)
Expand All @@ -146,10 +151,10 @@ def self.upgrade_formulae(
formula,
**{
options:,
link_keg: keg_had_linked_opt ? keg_was_linked : nil,
installed_as_dependency: tab&.installed_as_dependency,
installed_on_request: installed_on_request || tab&.installed_on_request,
build_bottle: tab&.built_bottle?,
link_keg:,
installed_as_dependency:,
installed_on_request:,
build_bottle:,
force_bottle:,
build_from_source_formulae:,
interactive:,
Expand Down Expand Up @@ -338,7 +343,6 @@ def self.check_installed_dependents(
upgrade_formulae(
upgradeable_dependents,
flags:,
installed_on_request:,
force_bottle:,
build_from_source_formulae:,
dependents: true,
Expand Down

0 comments on commit 4d4531c

Please sign in to comment.