The Documentation workflow installs Vale unpinned, and 3.17.0 reports two
errors that 3.16.0 did not, so every pull request built after its release
fails the docs job on unchanged content. Both are real violations of our
own styles rather than new false positives.
Drop an Oxford comma in the conference reimbursement list, and link to
How-To-Open-a-Homebrew-Pull-Request.md by the name Homebrew.Terms exempts,
which is also how every other page refers to it.
Sorbet only looks for spec methods at the top level of a class body or
`describe` block, so example groups generated inside a plain `each` are
invisible to it: their bodies typecheck against the group class rather than an
example, and every call in the loop fails to resolve. One loop cascades into an
error per call, which is usually enough to pin the file at `# typed: false`.
Adds `Test::Helper::TestEach`, which Sorbet can see through, and applies the new
`Sorbet/TestsDefinedWithEach` cop from rubocop-sorbet to the 22 loops that
qualify. The rewrites are semantics-preserving: `each` already yielded one row
per iteration, so multi-parameter blocks were auto-splatting what is now an
explicit destructure.
Six files come off `# typed: false` as a result, three of them all the way to
`# typed: strict`.
Both sigs are `checked(:never)`. A `Hash` reaches `test_each` rather than
`test_each_hash` whenever the receiver is not a hash literal, and under
`HOMEBREW_SORBET_RECURSIVE` a `Hash` only validates when the element type is a
tuple, which plain array rows are not. No element type satisfies both, and a
top-level union leaves the block parameter type unbound.
Also bumps `rbi` to 0.4.1, which spoom 1.8.6 requires, and regenerates the RBI
files for both.
`sorbet` is held at 0.6.13342: spoom only needs `>= 0.5.10187`, and bumping it
would rewrite the vendored `sorbet-runtime` copy that is committed in-tree.
Two `brew` processes downloading the same file raced on the download
lock and one failed outright with `OperationInProgressError`, which is
half of #23328 (the scheduling half was fixed in #23342). Waiting is
almost always what the user wants, since the holder is about to produce
exactly the file this process needs.
`DownloadLock#lock_or_wait` polls the existing non-blocking `flock`
every 0.1s instead of giving up on the first failure. At 0.1s the wait
costs about 0.15% of one core per waiting download, dominated by syscall
overhead rather than real work, so the interval buys responsiveness for
no meaningful CPU. The wait is capped at 3 minutes, or at the caller's
remaining `timeout:` budget when that is shorter, so waiting on the lock
can't blow a deadline the caller asked for. An hour is always going to
be too long, and giving up beats waiting much longer because
`RetryableDownload` preserves the `.incomplete` file, so a retry resumes
the holder's partial download via `--continue-at`.
The warning is suppressed when the caller is already rendering progress.
`HOMEBREW_DOWNLOAD_CONCURRENCY` defaults to `cores * 2`, and above 1
`DownloadQueue#fetch` drives a cursor-addressed redraw whose arithmetic
assumes one line per download, so an unscheduled write from a pool
worker desyncs it. `OperationInProgressError` also takes an optional
`waited:` now, because telling someone to "wait for it to finish or
terminate it to continue" after three minutes of waiting is not useful.
The message is unchanged for every existing caller.
Mutation testing found `lock_file_spec.rb` passed with the inode/unlink
recheck neutered and with `ignore_interrupts` removed from `#lock`, so
cover both. Deferring the interrupt itself can't be asserted in-process,
since RSpec owns the `INT` handler that `ignore_interrupts` traps and
`Thread#raise` bypasses `trap`, so the wrapper's presence is asserted
instead.
brew bundle's parallel scheduler avoids running two entries in the same
batch when their recursive declared dependencies overlap, but implicit
dependencies added at formula-instantiation time (e.g. bubblewrap and
its dependents on Linux, when the sandbox executable isn't installed
yet) aren't declared on any formula, so two otherwise-unrelated
formulae can both silently need to fetch the same bottle at once. Since
DownloadLock's flock is non-blocking, the loser dies instead of
waiting.
Add DependencyCollector#implicit_dependency_names (empty by default,
overridden on Linux to report bubblewrap/gcc/glibc when they'd
currently be added), and fold it into every entry's recursive
dependency set before computing overlaps, so entries needing the same
implicit dependency are serialized like any other shared dependency.
See https://github.com/Homebrew/brew/issues/23328
DownloadQueue#fetch's concurrent progress renderer redraws multiple
in-place progress lines using absolute cursor movement, and its
bookkeeping (previous_pending_line_count) assumes every printed
per-download line occupies exactly one terminal row. When a download
failed, the full error detail was printed inline via `ofail`, but that
message can span multiple rows: a Cask download error joins the URL
and raw curl stderr with an explicit newline, and curl's own
`--progress-bar` output uses carriage returns to animate in place. The
loop that prints just-finished downloads discarded the row-count
return value entirely, so after printing a multi-row failure the
cursor ended up further down than the code assumed, and every
subsequent redraw of other in-progress downloads landed on the wrong
rows.
Detailed failure output is now queued via a small
`report_or_defer_failure` helper and printed once the live in-place
redraw has finished and the cursor is restored to normal scrolling,
instead of being printed while the redraw is still live. Every line
printed during the redraw is now guaranteed to be exactly one row, so
the cursor maths stays correct no matter how long the eventual failure
text is.
Two helpers clean up the deferred text itself:
- `Tty.collapse_carriage_returns` simulates how a terminal would
actually render `\r`-based in-place overwrites, collapsing what can
be 100+ concatenated curl progress-bar animation frames down to the
last non-empty one (a `\r` only resets the cursor column, it doesn't
erase, so a trailing `\r` must not discard the last-written
content).
- `Utils::Curl.strip_progress_bar` then removes that final
`--progress-bar` percentage entirely rather than keeping it. A
leftover percentage can belong to an earlier, unrelated request
within the same curl invocation (e.g. a redirect hop that completed
fine before the real target failed to connect), so showing e.g.
"100.0%" next to a connection failure is misleading rather than
merely glued-on, and there's no reliable way to tell from the
captured text whether it ever applied to the transfer that actually
failed.
Fixes https://github.com/Homebrew/brew/issues/23281
Replace T.must(arr.first)/T.must(hash[key]) with arr.fetch(0)/
hash.fetch(key), which express the same non-nil guarantee without
relying on Sorbet's escape hatch.
rubocop 1.88.2's Style/ArrayIntersect autocorrects `include?`/`member?`
block patterns to `intersect?` even when the second operand isn't
actually an Array, which can raise at runtime. Disable the cop
repo-wide (to be re-enabled once rubocop/rubocop#15442 ships) rather
than disabling it at each affected call site.
Also updates rubocop (1.88.1 -> 1.88.2) and rubocop-ast (1.49.1 -> 1.50.0),
which are pulled in transitively since rubocop-sorbet has no upper bound
on its rubocop dependency.
rubocop-sorbet 0.13.0 no longer flags Sorbet type_template/type_member
constants under Style/MutableConstant, so the surrounding disable
comments (and their explanatory comments) are now dead code and removed.
rubocop 1.88.2 also extends Style/ArrayIntersect's block-based pattern
match to cover `include?`, not just `member?`, and applies its
autocorrect even though the cop's own docs note it "cannot guarantee
`array1` and `array2` are actually arrays". Several matches were on
Sets, Strings, or Enumerators, where `Array#intersect?` raises; those
call sites disable the cop with an explanation and keep the original
code. The remaining matches were genuinely between two arrays, so keep
the `intersect?` rewrite there.
Apply the autocorrections from two new rubocop-sorbet cops,
Sorbet/RedundantTLet and Sorbet/RedundantTLetForLiteral, which flag
T.let annotations that Sorbet infers automatically:
- constants assigned a constructor call (optionally .freeze'd)
- constants assigned literals and frozen literal arrays
- initialize instance variables assigned from signature parameters
These rely on Sorbet's freeze-transparent inference (0.6.13304+); brew
ships sorbet-static 0.6.13316. `brew typecheck` passes with the
annotations removed.
Result of running `brew style --fix`, which applies the new
Homebrew/UnreferencedLet cop. These lazy `let` declarations are never
referenced, so their blocks never ran. Removing them also let
RSpec/EmptyLineAfterFinalLet tidy the surrounding blank lines.
All affected specs continue to pass.
- Drop the test/support scanner (git ls-files / Dir.glob enumeration plus the
framework_let_names exemption). Homebrew defines no `let`/`subject` under
test/support, so the exemption never fired -- it was dead weight. Removes
~165 lines and the open3 dependency, with no behavior change in this repo.
- Derive the `definition_name` node pattern from DEFINITION_METHODS instead of
duplicating the `{:let :let! :subject}` literal.
Add a cop that flags and removes lazy `let(:name) { ... }` declarations
whose name is never referenced. A lazy `let` only runs when its name is
called, so an unreferenced one is dead code whose block never executes.
Adapted from Gusto/rubocop-gusto#128 to Homebrew conventions: the
`Homebrew` department, `typed: strict` with Sorbet signatures, scanning
`test/support/**` for framework contract names, and reserving RuboCop's
`:config` shared-context lets (`cop_config`, `other_cops`, `cop_options`,
`gem_versions`).
Detection is file-scoped and conservative, preferring false negatives:
overrides/super chains, shared examples, dynamic dispatch, and names that
appear as a bare call, symbol, or identifier-shaped token in any string
are all left untouched. Auto-correction is unsafe (deletion) and only
runs with explicit `--autocorrect-all`.
Add explicit nil return in formula_installer.rb where the else
branch returns an Array from <<, violating the block's declared
T.nilable(Symbol) return type.
Use T::Sig::WithoutRuntime for Formula#recursive_requirements
(matching recursive_dependencies) since CaskDependent may not be
initialized yet.
PRUNE, SKIP, and KEEP_BUT_PRUNE_RECURSIVE_DEPS were defined
separately on Dependency and Requirement with identical values.
This coupling was fragile — if either constant changed, the
polymorphic code in dependencies_helpers.rb would silently break.
Move the constants to the shared Dependable module (included by
both classes) so there is a single source of truth, and update
all callers to reference Dependable:: directly.
Document why the trailing nil in language/python.rb is necessary
(block sig requires T.nilable(Symbol), not Array) and why
Dependency::PRUNE works in the polymorphic Requirement context
in dependencies_helpers.rb (both constants are :prune).
Ruby's throw/catch is a non-local control flow mechanism (a form of
goto). Replace all three uses with idiomatic Ruby patterns:
- Dependency: remove catch(:action) from `action`, use the block's
return value directly. Callers signal actions via `next` with
named constants (PRUNE, SKIP, KEEP_BUT_PRUNE_RECURSIVE_DEPS).
- Requirement: remove catch(:prune) from `prune?`, check the block's
return value against the PRUNE constant.
- Livecheck: extract nested loop into `find_version_update_revision`
helper and use `return` for early exit.
Remove the now-unnecessary `prune`, `skip`, and
`keep_but_prune_recursive_deps` class methods and update all callers
and type signatures.
The sig for `create_bump_pr` references `BumpCaskPr::Args` which
may not be loaded yet, causing `uninitialized constant` errors in
CI (e.g. homebrew-core autobump). Switch to
`T::Sig::WithoutRuntime.sig` so the type is resolved lazily.
After rebasing onto the perf/autoremove-doctor-tab-data merge, the new
tab data reads in autoremove.rb use dep["full_name"] on a value typed as
RuntimeDependencies. That union includes T::Array[String], so Sorbet
sees String#[] receiving a String key and errors. Cast to
T::Array[T::Hash[String, T.untyped]] — the only non-nil shape actually
returned by Tab.runtime_deps_hash on 1.1.6+ installs.
- Use T::Sig::WithoutRuntime.sig for Dependency#action to avoid
uninitialized constant error when CaskDependent is not yet loaded
- Fix @tap_audit T.let type from T.nilable(TapAuditor) to
T.nilable(T::Boolean) to match the boolean value passed from audit.rb
Add Sorbet `sig` annotations to the cask-related files that were
already marked `# typed: strict` but lacked complete signatures.
Key changes:
- Add `sig` and `T.let` declarations to cask/cask.rb, cask/dsl.rb,
cask/cask_loader.rb, cask/installer.rb, cask/exceptions.rb,
cask/quarantine.rb, and cask/dsl/caveats.rb
- Use `T::Sig::WithoutRuntime.sig` for `DSL#url`, `DSL#set_unique_stanza`,
and `CaskLoader.{path,load,for}` to avoid Sorbet runtime wrappers
interfering with `caller_locations` (used by `URL#unversioned?`) and
RSpec mocking of class methods
- Fix nil guards using raise-unless patterns instead of `T.must`
- Avoid `T.unsafe`; use `T.untyped` only as hash value types
- Fix cascade type errors in callers (audit.rb, info.rb, upgrade.rb,
livecheck, bump-cask-pr.rb, etc.) that now see explicit nilable types
on `Cask#tap`, `CaskLoader.load`, and related methods
Remove 29 unnecessary `require` statements across 30 files where the
required file's exported constants are never referenced in the requiring
file.
Three env-sync commands (`nodenv-sync`, `pyenv-sync`, `rbenv-sync`) had
`require "formula"` despite only using `Keg.new` — replaced with
`require "keg"` to match actual usage.
`commands.rb` previously relied on a transitive load chain
(`require "utils"` → `require "homebrew"`) for `Homebrew.require?`.
That dependency is now explicit.
`diagnostic.rb` was using `GitRepository` via a transitive load through
`system_config.rb`. Now requires `git_repository` directly.
Progresses Homebrew/brew#17297 by upgrading the Sorbet sigil to
`typed: strict` across all files in `Library/Homebrew/cask/artifact/`
and resolving the resulting type errors.
Progresses Homebrew/brew#17297 by upgrading the Sorbet sigil to
`typed: strict` across all files in `Library/Homebrew/bundle/` and
resolving the resulting type errors.
brew leaves was calling Dependency#to_installed_formula -> Formulary.resolve
for every runtime dependency of every installed formula. This triggered
filesystem I/O (reading tabs, resolving symlinks, loading formula files) for
each of those calls, accounting for ~45% of total wall time.
Instead, read dependency names directly from each formula's keg tab data
(Keg#runtime_dependencies returns [{full_name: ...}] hashes) and build a Set
of base names for O(1) membership tests. A fallback to the original
installed_runtime_formula_dependencies path is kept for pre-1.1.6
installations that have no runtime_dependencies in their tabs.
For cask dependencies, use CaskDependent#deps (string names only) instead of
CaskDependent#runtime_dependencies (which called to_installed_formula).
Transitive cask formula deps are already captured by the main tab-data loop.
Use Formula#possible_names ([name, *oldnames, *aliases]) for the membership
check so renamed formulae (stale tab records old name) are correctly excluded
from leaves. As a side-effect, this also eliminates the unintended
"Warning: Formula foo was renamed to bar." stderr output that the original
code emitted on every brew leaves run when stale tabs referenced old names.
version_string() was making two sequential git subprocess calls per
repository (rev-parse + show), now replaced with a single
git log -1 --format='%h %cd' call. Halves git spawns from 4 to 2
for a default install, making the version_string logic ~2.3x faster.
Test through public run! interfaces where practical
(BottlesFetch, CleanupAfter); use send for deeply
internal methods without a reasonable public entry point.
Fix two runtime type errors found in CI after the initial strict typing PR:
1. `Test#test` arguments parameter: Accept `T.any(String, Pathname)` and
convert to strings via `.map(&:to_s)` before passing to `Step.new`.
Callers like `TestCleanup` pass `repository` (a `Pathname`) directly.
2. `BottlesFetch#fetch_bottles!` tag parameter: Accept `Utils::Bottles::Tag`
(what `collector.tags` actually returns) instead of `Symbol`.
