-
-
Notifications
You must be signed in to change notification settings - Fork 10.9k
rubocops: Add HTTP URL check for formulae and casks #21316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this 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 adds RuboCop rules to disallow HTTP URLs (http://) in Homebrew/core and Homebrew/cask repositories, while allowing HTTP for mirror URLs in formulae since they may be needed for bootstrapping.
Key changes:
- Adds
HttpUrlscop for formulae to detect and auto-correct http:// URLs to https:// - Extends cask URL cop to detect and auto-correct http:// URLs to https://
- Both cops only apply to homebrew-core and homebrew-cask taps respectively
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Library/Homebrew/rubocops/urls.rb | Adds new HttpUrls FormulaCop that flags http:// URLs in homebrew-core formulae (excluding mirrors) and provides auto-correction to https:// |
| Library/Homebrew/rubocops/cask/url.rb | Extends the existing Url cop to check for http:// URLs in homebrew-cask and provides auto-correction to https:// |
| Library/Homebrew/test/rubocops/urls/http_spec.rb | Adds comprehensive test coverage for the new formula HttpUrls cop including offense detection, auto-correction, tap filtering, and mirror URL exclusion |
| Library/Homebrew/test/rubocops/cask/url_spec.rb | Adds test coverage for http:// URL detection in casks including offense detection, auto-correction, tap filtering, and https:// URL acceptance |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Recently we got an error frequently, |
4c20b39 to
3a212fa
Compare
3a212fa to
1446ec2
Compare
|
Could exclude livecheck url from formula audit. This is implicitly done when using symbol (e.g. |
|
There are 73 offenses in Homebrew/core currently. Excluding livecheck URLs brings it down to 48. I'm open to adding this, just not sure of the right way to do so. |
|
@p-linnane What's blocking the other 48, out of interest? |
|
Just going through and identifying other HTTPS sources. Sometimes Debian has them, albeit with a different checksum. Other times they're niche and don't really have any other options. I plan to dig in more over the coming days. |
Not sure on best option. Maybe trying to look at parent nodes to see if in Easiest option may just be ignoring any url equal to livecheck url since we already have code that can extract livecheck url: brew/Library/Homebrew/rubocops/urls.rb Lines 21 to 25 in 7543440
|
1446ec2 to
de3a9b0
Compare
Signed-off-by: Patrick Linnane <patrick@linnane.io>
0f47805 to
def4ff6
Compare
brew lgtm(style, typechecking and tests) with your changes locally?After some internal discussion we've decided to no longer allow HTTP URLs in Homebrew/core and Homebrew/cask. This doesn't include mirrors since we often use them for bootstrapping.
This should produce numerous failures right now, but it'll allow us to review them.