diff --git a/Library/Homebrew/dev-cmd/test-bot.rb b/Library/Homebrew/dev-cmd/test-bot.rb index 45eae7e19ed49..3697d840ff7f5 100755 --- a/Library/Homebrew/dev-cmd/test-bot.rb +++ b/Library/Homebrew/dev-cmd/test-bot.rb @@ -25,7 +25,8 @@ class TestBotCmd < AbstractCommand switch "--build-from-source", description: "Build from source rather than building bottles." switch "--build-dependents-from-source", - description: "Build dependents from source rather than testing bottles." + description: "Build a limited set of dependents from source in addition to testing bottles. " \ + "Up to 10 per formula per shard, prioritising popular dependents in a sharded group." switch "--junit", description: "generate a JUnit XML test results file." switch "--keep-old", diff --git a/Library/Homebrew/test/test_bot/formulae_dependents_spec.rb b/Library/Homebrew/test/test_bot/formulae_dependents_spec.rb index 8fe436f34d104..9183a27716ba7 100644 --- a/Library/Homebrew/test/test_bot/formulae_dependents_spec.rb +++ b/Library/Homebrew/test/test_bot/formulae_dependents_spec.rb @@ -54,4 +54,74 @@ expect(formulae_dependents.dependents_for_shard([[dependent, dependent.deps.to_a]], "2/2")).to be_empty end end + + describe "#split_source_dependents" do + let(:dependents) { dependents_hash.values } + let(:dependents_hash) do + 14.downto(6).to_h do |i| + f = formula "dependent-#{i}" do + T.bind(self, T.class_of(Formula)) + url "https://brew.sh/dependent-#{i}/1.0.tar.gz" + depends_on "dependency" + end + [i, [f, f.deps.to_a]] + end + end + + before do + stub_formula_loader( + formula("dependency") do + T.bind(self, T.class_of(Formula)) + url "https://brew.sh/dependency/1.0.tar.gz" + end, + ) + allow(Homebrew::API::Analytics).to receive(:fetch).with("install", 90).and_return({ + "items" => 1.upto(20).map { |i| { "number" => i, "formula" => "dependent-#{i}" } }, + }) + allow(formulae_dependents).to receive_messages( + build_dependent_from_source?: true, + bottled_or_built?: true, + ) + end + + it "does not source build dependents with unbottled dependency" do + unbottled_dependency = formula "unbottled-dependency" do + T.bind(self, T.class_of(Formula)) + url "https://brew.sh/unbottled-dependency/1.0.tar.gz" + depends_on "dependency" + end + stub_formula_loader unbottled_dependency + allow(formulae_dependents).to receive(:bottled_or_built?).with(unbottled_dependency, []).and_return(false) + + source_dependents = dependents_hash.values + dependent_with_unbottled_dep = formula "dependent-5" do + T.bind(self, T.class_of(Formula)) + url "https://brew.sh/dependent-5/1.0.tar.gz" + depends_on "dependency" + depends_on "unbottled-dependency" + end + not_source_dependent = [dependent_with_unbottled_dep, dependent_with_unbottled_dep.deps.to_a] + dependents_hash[5] = not_source_dependent + + expect(formulae_dependents.split_source_dependents(dependents)).to eq [ + source_dependents, + [not_source_dependent], + ] + end + + it "limits total source build dependents to upper bound and orders on analytics" do + expect(formulae_dependents.split_source_dependents(dependents, 1)).to eq [ + [dependents_hash.fetch(6)], + 7.upto(14).map { dependents_hash.fetch(it) }, + ] + expect(formulae_dependents.split_source_dependents(dependents, 5)).to eq [ + 6.upto(10).map { dependents_hash.fetch(it) }, + 11.upto(14).map { dependents_hash.fetch(it) }, + ] + expect(formulae_dependents.split_source_dependents(dependents, 7)).to eq [ + 6.upto(12).map { dependents_hash.fetch(it) }, + 13.upto(14).map { dependents_hash.fetch(it) }, + ] + end + end end diff --git a/Library/Homebrew/test_bot/formulae_dependents.rb b/Library/Homebrew/test_bot/formulae_dependents.rb index db97f1db3733a..67b453c152c8a 100644 --- a/Library/Homebrew/test_bot/formulae_dependents.rb +++ b/Library/Homebrew/test_bot/formulae_dependents.rb @@ -4,6 +4,9 @@ module Homebrew module TestBot class FormulaeDependents < TestFormulae + MAX_DEPENDENTS_FROM_SOURCE = 10 + private_constant :MAX_DEPENDENTS_FROM_SOURCE + DependentWithDependencies = T.type_alias { [Formula, T::Array[Dependency]] } private_constant :DependentWithDependencies @@ -162,6 +165,51 @@ def dependents_for_shard(dependents, shard) shards.fetch(shard_index - 1).sort_by { |dependent, _| dependent.full_name } end + sig { + params( + dependents: T::Array[DependentWithDependencies], + max: Integer, + ).returns([T::Array[DependentWithDependencies], T::Array[DependentWithDependencies]]) + } + def split_source_dependents(dependents, max = MAX_DEPENDENTS_FROM_SOURCE) + source_dependents, dependents = dependents.partition do |dependent, deps| + next false unless build_dependent_from_source?(dependent) + + deps.all? do |d| + bottled_or_built?(d.to_formula, @dependent_testing_formulae) + end + end + + return [source_dependents, dependents] if source_dependents.count <= max + + ohai "Only source building #{max} of #{source_dependents.count} dependents" + + @formula_install_ranks ||= T.let(begin + analytics = begin + require "api/analytics" + Homebrew::API::Analytics.fetch "install", 90 + rescue ArgumentError + {} + end + analytics["items"].to_a.each_with_object({}) do |item, hash| + formula = item["formula"] + number = item["number"] + next if formula.blank? || number.blank? + + hash[formula.to_s] = number.to_i + end + end, T.nilable(T::Hash[String, Integer])) + + if @formula_install_ranks.present? + last = @formula_install_ranks.each_value.max.to_i + 1 + source_dependents.sort_by! do |dependent, _| + [@formula_install_ranks.fetch(dependent.full_name, last), dependent.full_name] + end + end + dependents.concat(source_dependents.slice!(max..).to_a) + [source_dependents, dependents] + end + private sig { params(installable_bottles: T::Array[String], args: Homebrew::Cmd::TestBotCmd::Args).void } @@ -249,16 +297,12 @@ def dependents_for_formula(formula, formula_name, args:) dependents.reject! { |dependent, _| @tested_dependents.include?(dependent.full_name) } # Split into dependents that we could potentially be building from source and those - # we should not. The criteria is that a dependent must have bottled dependencies, and - # either the `--build-dependents-from-source` flag was passed or a dependent has no - # bottle on the current OS. - source_dependents, dependents = dependents.partition do |dependent, deps| - next false unless build_dependent_from_source?(dependent) - - all_deps_bottled_or_built = deps.all? do |d| - bottled_or_built?(d.to_formula, @dependent_testing_formulae) - end - args.build_dependents_from_source? && all_deps_bottled_or_built + # we should not. The criteria is that a dependent must have bottled dependencies and + # the `--build-dependents-from-source` flag was passed. Total source build dependents + # are limited per formula per shard to avoid overly long CI runtime. + source_dependents = [] + if args.build_dependents_from_source? + source_dependents, dependents = split_source_dependents(dependents) end # From the non-source list, get rid of any dependents we are only a build dependency to diff --git a/completions/fish/brew.fish b/completions/fish/brew.fish index 93f258d1c1886..5f0be55122281 100644 --- a/completions/fish/brew.fish +++ b/completions/fish/brew.fish @@ -1943,7 +1943,7 @@ __fish_brew_complete_arg 'test' -a '(__fish_brew_suggest_formulae_installed)' __fish_brew_complete_cmd 'test-bot' 'Tests the full lifecycle of a Homebrew change to a tap (Git repository)' __fish_brew_complete_arg 'test-bot' -l added-formulae -d 'Use these added formulae rather than running the formulae detection steps' -__fish_brew_complete_arg 'test-bot' -l build-dependents-from-source -d 'Build dependents from source rather than testing bottles' +__fish_brew_complete_arg 'test-bot' -l build-dependents-from-source -d 'Build a limited set of dependents from source in addition to testing bottles. Up to 10 per formula per shard, prioritising popular dependents in a sharded group' __fish_brew_complete_arg 'test-bot' -l build-from-source -d 'Build from source rather than building bottles' __fish_brew_complete_arg 'test-bot' -l cleanup -d 'Clean all state from the Homebrew directory. Use with care!' __fish_brew_complete_arg 'test-bot' -l debug -d 'Display any debugging information' diff --git a/completions/zsh/_brew b/completions/zsh/_brew index e813682adaba4..bb07bb794613a 100644 --- a/completions/zsh/_brew +++ b/completions/zsh/_brew @@ -2511,7 +2511,7 @@ _brew_test() { _brew_test_bot() { _arguments \ '(--only-formulae-detect)--added-formulae[Use these added formulae rather than running the formulae detection steps]' \ - '--build-dependents-from-source[Build dependents from source rather than testing bottles]' \ + '--build-dependents-from-source[Build a limited set of dependents from source in addition to testing bottles. Up to 10 per formula per shard, prioritising popular dependents in a sharded group]' \ '--build-from-source[Build from source rather than building bottles]' \ '--cleanup[Clean all state from the Homebrew directory. Use with care!]' \ '--debug[Display any debugging information]' \ diff --git a/docs/Manpage.md b/docs/Manpage.md index 18ec57f925a1a..47ef469572772 100644 --- a/docs/Manpage.md +++ b/docs/Manpage.md @@ -3750,7 +3750,9 @@ and Linux workers. `--build-dependents-from-source` -: Build dependents from source rather than testing bottles. +: Build a limited set of dependents from source in addition to testing bottles. + Up to 10 per formula per shard, prioritising popular dependents in a sharded + group. `--junit` diff --git a/manpages/brew.1 b/manpages/brew.1 index 7fad4b9a78cf7..20f68c3101963 100644 --- a/manpages/brew.1 +++ b/manpages/brew.1 @@ -2360,7 +2360,7 @@ Don\[u2019]t check if the local system is set up correctly\. Build from source rather than building bottles\. .TP \fB\-\-build\-dependents\-from\-source\fP -Build dependents from source rather than testing bottles\. +Build a limited set of dependents from source in addition to testing bottles\. Up to 10 per formula per shard, prioritising popular dependents in a sharded group\. .TP \fB\-\-junit\fP generate a JUnit XML test results file\.