Skip to content

Commit

Permalink
Fix usage of Tab#installed_(on_request|as_dependency)
Browse files Browse the repository at this point in the history
These can return `true`, `false` or `nil` so adjust the signature to
note this and fix the call sites to ensure we don't accidentally pass
through `nil` values when we shouldn't.

While we're here, make a `TODO` to fix this bad API up in future.

Fixes #19076
  • Loading branch information
MikeMcQuaid committed Jan 13, 2025
1 parent 6aac197 commit 0940fb7
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 7 deletions.
4 changes: 2 additions & 2 deletions Library/Homebrew/cmd/leaves.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,12 @@ def run

sig { params(formula: Formula).returns(T::Boolean) }
def installed_on_request?(formula)
formula.any_installed_keg&.tab&.installed_on_request
formula.any_installed_keg&.tab&.installed_on_request == true
end

sig { params(formula: Formula).returns(T::Boolean) }
def installed_as_dependency?(formula)
formula.any_installed_keg&.tab&.installed_as_dependency
formula.any_installed_keg&.tab&.installed_as_dependency == true
end
end
end
Expand Down
8 changes: 7 additions & 1 deletion Library/Homebrew/formula_installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -800,12 +800,18 @@ def install_dependency(dep, inherited_options)
options |= inherited_options
options &= df.options

installed_on_request = if df.any_version_installed? && tab.present? && tab.installed_on_request

Check warning on line 803 in Library/Homebrew/formula_installer.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/formula_installer.rb#L803

Added line #L803 was not covered by tests
true
else
false
end

fi = FormulaInstaller.new(
df,
options:,
link_keg: keg_had_linked_keg && keg_was_linked,
installed_as_dependency: true,
installed_on_request: df.any_version_installed? && tab.present? && tab.installed_on_request,
installed_on_request:,
force_bottle: false,
include_test_formulae: @include_test_formulae,
build_from_source_formulae: @build_from_source_formulae,
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/reinstall.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ def self.reinstall_formula(
keg = Keg.new(formula.opt_prefix.resolved_path)
tab = keg.tab
link_keg = keg.linked?
installed_as_dependency = tab.installed_as_dependency
installed_on_request = tab.installed_on_request
installed_as_dependency = tab.installed_as_dependency == true
installed_on_request = tab.installed_on_request == true
build_bottle = tab.built_bottle?
backup keg
else
Expand Down
2 changes: 2 additions & 0 deletions Library/Homebrew/tab.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,13 @@ class AbstractTab
# Check whether the formula or cask was installed as a dependency.
#
# @api internal
sig { returns(T.nilable(T::Boolean)) } # TODO: change this to always return a boolean
attr_accessor :installed_as_dependency

# Check whether the formula or cask was installed on request.
#
# @api internal
sig { returns(T.nilable(T::Boolean)) } # TODO: change this to always return a boolean
attr_accessor :installed_on_request

attr_accessor :homebrew_version, :tabfile, :loaded_from_api, :time, :arch, :source, :built_on
Expand Down
4 changes: 2 additions & 2 deletions Library/Homebrew/upgrade.rb
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,8 @@ def self.upgrade_formulae(
if keg
tab = keg.tab
link_keg = keg.linked?
installed_as_dependency = tab.installed_as_dependency
installed_on_request = tab.installed_on_request
installed_as_dependency = tab.installed_as_dependency == true
installed_on_request = tab.installed_on_request == true

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

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/upgrade.rb#L135-L136

Added lines #L135 - L136 were not covered by tests
build_bottle = tab.built_bottle?
else
link_keg = nil
Expand Down

0 comments on commit 0940fb7

Please sign in to comment.