Skip to content

Conversation

@cho-m
Copy link
Member

@cho-m cho-m commented Nov 16, 2025

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) with your changes locally?

Ignoring errors disables proper exception handling which leads to unexpected code paths that can cause Sorbet errors rather than FormulaUnreadableError. Sorbet errors then cascade up and cause brew to not run properly.


Need someone to check if this was intentionally set to ignore_errors: true

At least helps with brew doctor part of

e.g. reproduced with my existing install of tmux which was installed before we dropped Sierra from Formula

❯ rg on_system $(brew --prefix tmux)/.brew/tmux.rb
30:  on_system :linux, macos: :sierra_or_newer do

before

❯ HOMEBREW_USE_INTERNAL_API=1 brew doctor
Error: Passed `nil` into T.must
Warning: Removed Sorbet lines from backtrace!
Rerun with `--verbose` to see the original backtrace
/opt/homebrew/Library/Homebrew/ignorable.rb:30:in 'block in Object#raise'
/opt/homebrew/Library/Homebrew/ignorable.rb:29:in 'Kernel#callcc'
/opt/homebrew/Library/Homebrew/ignorable.rb:29:in 'Object#raise'
/opt/homebrew/Library/Homebrew/macos_version.rb:58:in 'MacOSVersion#initialize'

after

❯ HOMEBREW_USE_INTERNAL_API=1 brew doctor
Your system is ready to brew.

Copilot AI review requested due to automatic review settings November 16, 2025 01:11
Ignoring errors disables proper exception handling which leads to
unexpected code paths that can cause Sorbet errors rather than
`FormulaUnreadableError`. Sorbet errors then cascade up and cause brew
to not run properly.
@cho-m cho-m force-pushed the formulary-dont-ignore-errors branch from 42c8f65 to 465400f Compare November 16, 2025 01:11
@cho-m cho-m changed the title formulary: leave default ignore_errors=false formulary: leave default ignore_errors=false for internal API load Nov 16, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes error handling when loading formulas from kegs by removing ignore_errors: true parameter from the FromKegLoader.get_formula call in the from_keg method. This ensures proper exception handling that raises FormulaUnreadableError instead of causing Sorbet type errors that cascade and prevent Homebrew from running properly, particularly affecting brew doctor.

Key Changes:

  • Removed ignore_errors: true parameter from loader.get_formula() call in Formulary.from_keg method
  • Relies on the default ignore_errors: false parameter value instead

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

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

Makes sense, thanks @cho-m!

@MikeMcQuaid MikeMcQuaid added this pull request to the merge queue Nov 16, 2025
Merged via the queue into main with commit d455de8 Nov 16, 2025
37 checks passed
@MikeMcQuaid MikeMcQuaid deleted the formulary-dont-ignore-errors branch November 16, 2025 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants