Skip to content

Commit

Permalink
Store and use revision, where possible, in tab runtime dependencies.
Browse files Browse the repository at this point in the history
Let's start storing `revision` and `pkg_version` for tab runtime
dependencies and use them when available.

When the `revision` is not available, use a conservative approach to
deciding whether dependencies need to be upgrade.

Co-authored-by: Mike McQuaid <mike@mikemcquaid.com>
  • Loading branch information
Bo98 and MikeMcQuaid committed Nov 10, 2023
1 parent a808e30 commit 19f27f9
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 19 deletions.
29 changes: 23 additions & 6 deletions Library/Homebrew/dependency.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ def to_formula
formula
end

def installed?(minimum_version: nil)
sig { params(minimum_version: T.nilable(Version), minimum_revision: T.nilable(Integer)).returns(T::Boolean) }
def installed?(minimum_version: nil, minimum_revision: nil)
formula = begin
to_formula
rescue FormulaUnavailableError
Expand All @@ -55,14 +56,29 @@ def installed?(minimum_version: nil)
return false unless formula

if minimum_version.present?
formula.any_version_installed? && (formula.any_installed_version.version >= minimum_version)
installed_version = formula.any_installed_version
return false unless installed_version

# Tabs prior to 4.1.18 did not have revision or pkg_version fields.
# As a result, we have to be more conversative when we do not have
# a minimum revision from the tab and assume that if the formula has a
# the same version and a non-zero revision that it needs upgraded.
if minimum_revision.present?

Check warning on line 66 in Library/Homebrew/dependency.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/dependency.rb#L66

Added line #L66 was not covered by tests
minimum_pkg_version = PkgVersion.new(minimum_version, minimum_revision)
installed_version >= minimum_pkg_version

Check warning on line 68 in Library/Homebrew/dependency.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/dependency.rb#L68

Added line #L68 was not covered by tests
elsif installed_version.version == minimum_version
formula.revision.zero?
else
installed_version.version > minimum_version
end
else
formula.latest_version_installed?
end
end

def satisfied?(inherited_options = [], minimum_version: nil)
installed?(minimum_version: minimum_version) && missing_options(inherited_options).empty?
def satisfied?(inherited_options = [], minimum_version: nil, minimum_revision: nil)
installed?(minimum_version: minimum_version, minimum_revision: minimum_revision) &&
missing_options(inherited_options).empty?
end

def missing_options(inherited_options)
Expand Down Expand Up @@ -248,8 +264,9 @@ def hash
[name, tags, bounds].hash
end

def installed?(minimum_version: nil)
use_macos_install? || super(minimum_version: minimum_version)
sig { params(minimum_version: T.nilable(Version), minimum_revision: T.nilable(Integer)).returns(T::Boolean) }
def installed?(minimum_version: nil, minimum_revision: nil)
use_macos_install? || super(minimum_version: minimum_version, minimum_revision: minimum_revision)

Check warning on line 269 in Library/Homebrew/dependency.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/dependency.rb#L269

Added line #L269 was not covered by tests
end

sig { returns(T::Boolean) }
Expand Down
7 changes: 5 additions & 2 deletions Library/Homebrew/formula_installer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -616,11 +616,14 @@ def expand_dependencies_for_formula(formula, inherited_options)
keep_build_test ||= dep.build? && !install_bottle_for?(dependent, build) &&
(formula.head? || !dependent.latest_version_installed?)

bottle_runtime_version = @bottle_tab_runtime_dependencies.dig(dep.name, "version")
bottle_runtime_version = @bottle_tab_runtime_dependencies.dig(dep.name, "version").presence

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

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/formula_installer.rb#L619

Added line #L619 was not covered by tests
bottle_runtime_version = Version.new(bottle_runtime_version) if bottle_runtime_version
bottle_runtime_revision = @bottle_tab_runtime_dependencies.dig(dep.name, "revision")

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

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/formula_installer.rb#L621

Added line #L621 was not covered by tests

if dep.prune_from_option?(build) || ((dep.build? || dep.test?) && !keep_build_test)
Dependency.prune
elsif dep.satisfied?(inherited_options[dep.name], minimum_version: bottle_runtime_version)
elsif dep.satisfied?(inherited_options[dep.name], minimum_version: bottle_runtime_version,
minimum_revision: bottle_runtime_revision)
Dependency.skip
end
end
Expand Down
2 changes: 2 additions & 0 deletions Library/Homebrew/tab.rb
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,8 @@ def self.runtime_deps_hash(formula, deps)
{
"full_name" => f.full_name,
"version" => f.version.to_s,
"revision" => f.revision,
"pkg_version" => f.pkg_version.to_s,
"declared_directly" => formula.deps.include?(dep),
}
end
Expand Down
10 changes: 7 additions & 3 deletions Library/Homebrew/test/tab_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@
tab.homebrew_version = "1.1.6"
tab.runtime_dependencies = runtime_deps_hash
expect(tab.runtime_dependencies).to eql(
[{ "full_name" => "foo", "version" => "1.0", "declared_directly" => false }],
[{ "full_name" => "foo", "version" => "1.0", "revision" => 0, "pkg_version" => "1.0",
"declared_directly" => false }],
)
end

Expand Down Expand Up @@ -255,6 +256,7 @@
tap = Tap.new("user", "repo")
from_tap = formula("from_tap", path: tap.path/"Formula/from_tap.rb") do
url "from_tap-1.0"
revision 1
end
stub_formula_loader from_tap

Expand All @@ -266,8 +268,10 @@
tab = described_class.create(f, compiler, stdlib)

runtime_dependencies = [
{ "full_name" => "bar", "version" => "2.0", "declared_directly" => true },
{ "full_name" => "user/repo/from_tap", "version" => "1.0", "declared_directly" => true },
{ "full_name" => "bar", "version" => "2.0", "revision" => 0, "pkg_version" => "2.0",
"declared_directly" => true },
{ "full_name" => "user/repo/from_tap", "version" => "1.0", "revision" => 1, "pkg_version" => "1.0_1",
"declared_directly" => true },
]
expect(tab.runtime_dependencies).to eq(runtime_dependencies)

Expand Down
33 changes: 25 additions & 8 deletions Library/Homebrew/upgrade.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,17 +72,34 @@ def upgrade_formulae(
unless dry_run
fi.prelude

# Don't need to install this bottle if all the runtime dependencies
# are already satisfied.
next if dependents && fi.bottle_tab_runtime_dependencies.presence&.none? do |dependency, hash|
installed_version = begin
Formula[dependency].any_installed_version
# Don't need to install this bottle if all of the runtime
# dependencies have the same or newer version already installed.
next if dependents && fi.bottle_tab_runtime_dependencies.presence&.all? do |dependency, hash|
dependency_formula = begin
Formula[dependency]

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

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/upgrade.rb#L79

Added line #L79 was not covered by tests
rescue FormulaUnavailableError
nil
end
next true unless installed_version

Version.new(hash["version"]) > installed_version.version
next false if dependency_formula.nil?

installed_version = dependency_formula.any_installed_version

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

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/upgrade.rb#L85

Added line #L85 was not covered by tests
next false unless installed_version

next false if hash["version"].blank?

# Tabs prior to 4.1.18 did not have revision or pkg_version fields.
# As a result, we have to be more conversative when we do not have
# a revision in the tab and assume that if the formula has a
# the same version and a non-zero revision that it needs upgraded.
tab_version = Version.new(hash["version"])
if hash["revision"].present?

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

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/upgrade.rb#L94-L95

Added lines #L94 - L95 were not covered by tests
tab_pkg_version = PkgVersion.new(tab_version, hash["revision"])
installed_version >= tab_pkg_version

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

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/upgrade.rb#L97

Added line #L97 was not covered by tests
elsif installed_version.version == tab_version
dependency_formula.revision.zero?
else
installed_version.version > tab_version
end
end

fi.fetch
Expand Down

0 comments on commit 19f27f9

Please sign in to comment.