Skip to content

Allow limiting total build-from-source dependents - #23884

Open
cho-m wants to merge 1 commit into
mainfrom
test-bot-allow-limiting-source-dependents
Open

Allow limiting total build-from-source dependents#23884
cho-m wants to merge 1 commit into
mainfrom
test-bot-allow-limiting-source-dependents

Conversation

@cho-m

@cho-m cho-m commented Sep 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

One possible idea for issue we have been discussing with overly long CI times.

This PR uses a limit ranked by analytics data. Other ideas mentioned were testing dependents with dependents, no longer source building dependents, etc.

Rust and Go PRs run for over 1 day and continue to grow in total dependents. To make these more reasonable, allow limiting the maximum number of build-from-source dependents per runner (shard).

Selected based on analytics data as more popular formulae are more likely to be built from source by users so identifying these earlier would allow reporting issues or opening PRs upstream.

Examples:

brew test-bot '--testing-formulae=rust' '--tested-formulae=rust' --only-formulae-dependents --junit --build-dependents-from-source 
==> Running FormulaeDependents#dependent_formulae!(rust)
==> Determining dependents...
==> Only source building 10 of 1084 dependents
==> Source dependents:
uv
ripgrep
mise
pnpm
deno
just
ruby
usage
librsvg
azure-cli
==> Bottled dependents:
maturin
pake
hk
corrosion
anchor
diesel
helix-db
rust-wasm
cbfmt
rust-script
==> Testable dependents:
...
brew test-bot '--testing-formulae=go' '--tested-formulae=go' --only-formulae-dependents --junit --build-dependents-from-source 
==> Running FormulaeDependents#dependent_formulae!(go)
==> Determining dependents...
==> Only source building 10 of 1418 dependents
==> Source dependents:
gh
docker
fzf
ollama
yq
kubernetes-cli
helm
mole
glab
cloudflared
==> Bottled dependents:
golangci-lint
osv-scanner
akamai
govulncheck
staticcheck
bitrise
gotestsum
gosec
goimports
operator-sdk
gofumpt
kubebuilder
go-air
ibazel
gup
mage
wails
xk6
go-bindata
go-size-analyzer
vet
llgo
ignite
go-critic
pkgsite
cyclonedx-gomod
revive
cloudformation-cli
gf
xgo
gotests
ifacemaker
errcheck
go-blueprint
cobra-cli
solod
kitex
hz
terramaid
gops
richgo
goproxy
counterfeiter
kubehound
bunster
go-rice
naml
vgt
tygo
stuffbin
tscriptify
spdx-sbom-generator
==> Testable dependents:
...
  • Have you followed our Contributing guidelines?
  • Have you checked for other open Pull Requests for the same change?
  • Have you explained what your changes do? Performance claims (e.g. "this is faster") must include brew benchmark results.
  • Have you explained why you'd like these changes included, not just what they do?
  • For bug fixes, have you given step-by-step brew commands to reproduce the bug?
  • Have you written new tests (excluding integration tests)? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) locally?
  • I did not use AI/LLM to create this PR, or I disclosed the tool/model below and reviewed its output; I did not attribute commits to AI and will answer maintainer questions and review comments myself without AI/LLM.

n/a

Copilot AI balanced review requested due to automatic review settings September 8, 2026 00:38
Copilot AI reviewed Sep 8, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

馃煛 Changes recommended

The limit resets for each tested formula, so a runner can exceed the advertised total cap.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds an analytics-ranked cap on dependents built from source by brew test-bot.

Changes:

  • Adds the hidden --max-dependents-from-source flag.
  • Selects the most popular source dependents using analytics.
  • Adds Sorbet declarations and unit tests.
File summaries
File Description
Library/Homebrew/dev-cmd/test-bot.rb Defines the new limit flag.
Library/Homebrew/test_bot/formulae_dependents.rb Applies source-build selection and limiting.
Library/Homebrew/test/test_bot/formulae_dependents_spec.rb Tests filtering, limits and ordering.
Library/Homebrew/sorbet/rbi/dsl/homebrew/cmd/test_bot_cmd.rbi Types the new argument accessor.
Review details

Files not reviewed (1)

  • Library/Homebrew/sorbet/rbi/dsl/homebrew/cmd/test_bot_cmd.rbi: File type not supported
  • Files reviewed: 3/4 changed files
  • Comments generated: 2
  • Review effort level: Balanced

馃挕 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

Comment thread Library/Homebrew/test_bot/formulae_dependents.rb Outdated
Comment thread Library/Homebrew/dev-cmd/test-bot.rb Outdated
@cho-m
cho-m force-pushed the test-bot-allow-limiting-source-dependents branch from 88104dd to 538dc8b Compare September 8, 2026 00:47
@cho-m
cho-m marked this pull request as draft September 8, 2026 04:38
MikeMcQuaid
MikeMcQuaid approved these changes Sep 8, 2026
View reviewed changes

@MikeMcQuaid MikeMcQuaid left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, seems like a good idea. We should definitely cap (and randomise) this.

Do you have a sense of how often this source dependent building actually catches problems? I am inclined to say this could be set to a very low random number (~10 or something) and/or use analytics information to pick the most popular affected packages and just use those.

I wouldn't be opposed to removing this entirely.

Comment thread Library/Homebrew/dev-cmd/test-bot.rb Outdated
@cho-m

cho-m commented Sep 8, 2026 •
edited
Loading

Copy link
Copy Markdown
Member Author

Do you have a sense of how often this source dependent building actually catches problems?

For Rust, hard to say. Given CI takes too long, we've often had to ignore results. There is an ongoing sharded run right now (one shard finished 18h, some still running 22h+). Finished shard shows 2 checksum issues and 2 rust-related build failures (aider and mesa). Haven't checked but these may need backports or new release.

For Go, we are able to take actionable steps for breaking changes (either by switching to older versioned Go or updating/backporting from upstream):

Other formulae on source-build-deps list don't have crazy number of dependents. Build-specific tooling like cabal-install and pyqt-builder are useful to source build dependents as they are mainly for our usage so better to delay a release than break majority of dependents.

I am inclined to say this could be set to a very low random number (~10 or something) and/or use analytics information to pick the most popular affected packages and just use those.

I went with analytics as I'd expect those to have more source builds.

I wouldn't be opposed to removing this entirely.

Ignoring Go/Rust, would still want it for other formulae.

Perhaps can consider removing Rust from list if reduced test doesn't actually catch anything.

Go PRs do seem to result in actionable tasks, but Homebrew's Go users are still the noisiest if a release is not out right away, e.g.

@MikeMcQuaid

MikeMcQuaid commented Sep 8, 2026

Copy link
Copy Markdown
Member

I went with analytics as I'd expect those to have more source builds.

Yeh, sorry: I was suggesting we could perhaps use analytics to decide which source builds to perform dynamically at test-bot runtime?

Go PRs do seem to result in actionable tasks, but Homebrew's Go users are still the noisiest if a release is not out right away, e.g.

I think we should be aiming to merge these straight away when they don't break our own bottles, honestly.

There is an ongoing sharded run right now (one shard finished 18h, some still running 22h+)

Yes, this is crazy, need to not do this 馃槶

@cho-m
Allow limiting total build-from-source dependents
1674f4f 
Rust and Go PRs run for over 1 day and continue to grow in total
dependents. To make these more reasonable, limit the maximum
number of build-from-source dependents per runner (shard).

Selected based on analytics data as more popular formulae are more
likely to be built from source by users so identifying these earlier
would allow reporting issues or opening PRs upstream.
@cho-m
cho-m force-pushed the test-bot-allow-limiting-source-dependents branch from 538dc8b to 1674f4f Compare September 8, 2026 14:58
@cho-m
cho-m marked this pull request as ready for review September 8, 2026 14:58
@cho-m
cho-m requested a balanced review from Copilot September 8, 2026 14:58
Copilot AI reviewed Sep 8, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

馃煛 Changes recommended

Analytics ranking is applied per formula rather than across the runner-wide candidate set.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread Library/Homebrew/test_bot/formulae_dependents.rb
Comment on lines +308 to +311
max = MAX_DEPENDENTS_FROM_SOURCE - @tested_source_dependents_count
source_dependents = []
if args.build_dependents_from_source? && max.positive?
source_dependents, dependents = split_source_dependents(dependents, max)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort, defaults to Balanced
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@MikeMcQuaid MikeMcQuaid MikeMcQuaid approved these changes

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@cho-m @MikeMcQuaid