Skip to content

Conversation

@botantony
Copy link
Member

  • 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?

Draft PR, feel free to discuss and suggest changes (what checks should be added/removed, other mirrors with known problems, etc.)

@botantony botantony force-pushed the brew-doctor-check-mirrors branch from 0f7c6bd to 9795c77 Compare January 10, 2026 21:07
Signed-off-by: botantony <antonsm21@gmail.com>
@botantony botantony force-pushed the brew-doctor-check-mirrors branch from 9795c77 to 12f224d Compare January 10, 2026 21:16
Comment on lines +199 to +200
HOMEBREW_CORE_DEFAULT_GIT_REMOTE,
"https://github.com/Homebrew/homebrew-cask", # There's no `HOMEBREW_CASK_DEFAULT_GIT_REMOTE`
Copy link
Member

@Bo98 Bo98 Jan 11, 2026

Choose a reason for hiding this comment

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

These two can intentionally get outdated on a device without HOMEBREW_NO_INSTALL_FROM_API so may want to conditionally include these two.

(as demonstrated by the CI failures here)

You can solve this by setting the origin remote:
git -C "#{repository_path}" remote set-url origin #{Formatter.url(desired_origin)}
EOS
elsif (Time.now - last_commit_time) > Homebrew::API::DEFAULT_API_STALE_SECONDS
Copy link
Member

@Bo98 Bo98 Jan 11, 2026

Choose a reason for hiding this comment

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

Just using the default auto-update stale interval is too short for multiple reasons. Firstly, it might have just not got to auto-forcing an update yet if it has just passed - you want some leeway.

Secondly, there's also some things with slower update cadences, e.g: GitHub Actions images for example could be outdated by as much as a month. For end-users, this cadence might actually be more sporadic as not every command triggers an auto-update.

And thirdly since this also checks Homebrew/brew, people without developer mode enabled might not even have 1 update a week. We sometimes have made releases on 2-3 week intervals if something is baking in main. And it's worth having some extra leeway on top of that in case something major happens.

Overall point being: I don't think we want to make this too tight to simply be a "it's been a couple days, Homebrew might have an update" check and lean more towards a "Homebrew installation is obviously majorly outdated" check. The problems that inspired this check have all been cases where things have been outdated by several months.

Comment on lines +251 to +254
cask_tap_migrations.jws.json
cask.jws.json
formula_tap_migrations.jws.json
formula.jws.json
Copy link
Member

Choose a reason for hiding this comment

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

Are these all correct under the new internal API?

Comment on lines +301 to +305
# These are popular mirrors with known problems
bad_mirrors = [
%r{^https?://mirrors\.aliyun\.com/homebrew/.+}, # Last update was in May 2025
]
problems = []
Copy link
Member

@Bo98 Bo98 Jan 11, 2026

Choose a reason for hiding this comment

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

This seems annoying to maintain - any entry could spring to life at a moments notice and there could be others we're not aware of. It might even do the opposite of what we want and discourage mirrors from fixing their outdated ones in favour of just making a new URL (easier than requesting us to delist it from here). It's technically also incorrect as while the API mirror is outdated, the brew git mirror is not.

In any case, does this check matter if the other one is always going to trigger anyway (and already mentions mirrors)? You're going to get two warnings that effectively say the same thing. This can be an open question to other reviewers too.

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