Skip to content

Commit

Permalink
formulae_dependents: apply some optimisations
Browse files Browse the repository at this point in the history
1. Doing `brew install --only-dependencies` and `brew postinstall` can
   be quite expensive. Let's defer this to when we're sure we have
   dependents to test.
2. Calling `brew uses` is slow, because this requires traversing the
   dependency tree. [*] Let's avoid doing this unless we know we really
   need to. Here, we test for "needing to" by checking if another `.rb`
   file in the tap that might be a formula contains a `depends_on` line
   declaring a dependency on the formula being tested.
3. Restrict the second `brew uses` call to when we are building
   dependents from source, since that is the only instance where we are
   interested in the build dependents.

While we're here, make sure to call `brew postinstall` on all
dependencies that were rebuilt, and not just the one being tested
currently. This may address Homebrew#805.

Locally, this results in the following speed up for a formula with no
dependents:

Before

    ( brew test-bot --only-formulae-dependents --testing-formulae=hello --dry-run)  28.95s user 6.70s system 76% cpu 46.875 total

After

    ( brew test-bot --only-formulae-dependents --testing-formulae=hello --dry-run)  0.91s user 1.02s system 51% cpu 3.738 total

This makes testing formulae with dependents slightly slower. However,
the vast majority of formulae in Homebrew/core have no dependents (on
macOS, at least), so this is likely a net win for the average workflow.

[*] Potential future optimisation: calling `Dependency.expand` directly
    might give us better opportunities to exploit caching.
  • Loading branch information
carlocab committed Sep 26, 2022
1 parent d17dd36 commit 507686e
Showing 1 changed file with 44 additions and 14 deletions.
58 changes: 44 additions & 14 deletions lib/tests/formulae_dependents.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ class FormulaeDependents < TestFormulae
def run!(args:)
@source_tested_dependents = []
@bottle_tested_dependents = []
@testable_formulae = @testing_formulae - skipped_or_failed_formulae
@postinstalled_formulae = []

(@testing_formulae - skipped_or_failed_formulae).each do |f|
@testable_formulae.each do |f|
dependent_formulae!(f, args: args)
puts
end
Expand All @@ -22,19 +24,29 @@ def dependent_formulae!(formula_name, args:)

test_header(:FormulaeDependents, method: "dependent_formulae!(#{formula_name})")

formula = Formulary.factory(formula_name)

source_dependents, bottled_dependents, testable_dependents =
dependents_for_formula(formula, formula_name, args: args)

return if source_dependents.blank? &&
bottled_dependents.blank? &&
testable_dependents.blank?

# Install formula dependencies. These will have been uninstalled after building.
test "brew", "install", "--only-dependencies", formula_name,
env: { "HOMEBREW_DEVELOPER" => nil }
return if steps.last.failed?

# Restore etc/var files that may have been nuked in the build stage.
test "brew", "postinstall", formula_name
formula_dependencies = Utils.safe_popen_read("brew", "deps", formula_name).split("\n")
# Some dependencies will need postinstalling too.
postinstall_dependencies = @testable_formulae & formula_dependencies
postinstall_formulae = [formula_name, *postinstall_dependencies] - @postinstalled_formulae
test "brew", "postinstall", *postinstall_formulae
return if steps.last.failed?

formula = Formulary.factory(formula_name)

source_dependents, bottled_dependents, testable_dependents =
dependents_for_formula(formula, formula_name, args: args)
@postinstalled_formulae += postinstall_formulae

source_dependents.each do |dependent|
next if @source_tested_dependents.include?(dependent)
Expand All @@ -54,6 +66,21 @@ def dependent_formulae!(formula_name, args:)
end

def dependents_for_formula(formula, formula_name, args:)
# Calling `brew uses` is slow, so let's reserve doing that for
# if we can find a formula in the same tap that declares a dependency
# on the formula we are testing.
has_dependents = formula.tap.potential_formula_dirs.any? do |dir|
next false unless dir.exist?

dir.children.any? do |child|
next false unless child.file?
next false unless child.extname == ".rb"

child.read.include? "depends_on \"#{formula_name}\""
end
end
return unless has_dependents

info_header "Determining dependents..."

uses_args = %w[--formula --eval-all]
Expand All @@ -64,13 +91,16 @@ def dependents_for_formula(formula, formula_name, args:)
.split("\n")
end

# TODO: Consider handling the following case better.
# `foo` has a build dependency on `bar`, and `bar` has a runtime dependency on
# `baz`. When testing `baz` with `--build-dependents-from-source`, `foo` is
# not tested, but maybe should be.
dependents += with_env(HOMEBREW_STDERR: "1") do
Utils.safe_popen_read("brew", "uses", *uses_args, "--include-build", formula_name)
.split("\n")
# We care about build dependents only if we are building them from source.
if args.build_dependents_from_source?
# TODO: Consider handling the following case better.
# `foo` has a build dependency on `bar`, and `bar` has a runtime dependency on
# `baz`. When testing `baz` with `--build-dependents-from-source`, `foo` is
# not tested, but maybe should be.
dependents += with_env(HOMEBREW_STDERR: "1") do
Utils.safe_popen_read("brew", "uses", *uses_args, "--include-build", formula_name)
.split("\n")
end
end
dependents&.uniq!
dependents&.sort!
Expand Down Expand Up @@ -104,7 +134,7 @@ def dependents_for_formula(formula, formula_name, args:)
next false if OS.linux? && dependent.requirements.exclude?(LinuxRequirement.new)

all_deps_bottled_or_built = deps.all? do |d|
bottled_or_built?(d.to_formula, @testing_formulae - @skipped_or_failed_formulae)
bottled_or_built?(d.to_formula, @testable_formulae)
end
args.build_dependents_from_source? && all_deps_bottled_or_built
end
Expand Down

0 comments on commit 507686e

Please sign in to comment.