Skip to content

Conversation

@samford
Copy link
Member

@samford samford commented Dec 23, 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?

The existing livecheck-related cask audits can end up fetching the livecheck URL four times and this can cause problems if a server has sensitive rate limiting (e.g., Homebrew/homebrew-cask#241429 (comment)). This modifies the livecheck_version audit to cache the result that's also used in the hosting_with_livecheck and livecheck_https_availability audits. This will avoid a duplicate fetch and can avoid another if the livecheck URL uses HTTPS and the check doesn't fail. Most casks meet that criteria, so this should be a notable improvement.

There's still room for improvement, as the min_os audit calls cask_sparkle_min_os, which can also fetch the livecheck URL. It would be ideal if we could cache the livecheck content to avoid this duplicate fetch but I don't think that's possible with the current setup, so I'll have to revisit that later. [I have some in-progress work to add caching to livecheck and I may be able to leverage that in the future as a way of addressing this.]

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 optimizes cask audit performance by reducing redundant livecheck URL fetches from up to three times down to one in most cases. This is achieved by introducing a caching mechanism for the livecheck version audit result.

  • Adds @livecheck_result instance variable to cache livecheck audit results
  • Modifies audit_livecheck_version to cache and reuse results across multiple audit methods
  • Updates audit_hosting_with_livecheck to call audit_livecheck_version directly instead of accepting it as a parameter
  • Adds early return optimization in audit_livecheck_https_availability to skip unnecessary checks when livecheck already uses HTTPS and succeeds

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

@samford samford force-pushed the cask-audit-minimize-livecheck-fetches branch from 40ae65b to 45c08f7 Compare December 23, 2025 01:31
@samford samford changed the title Cask::Audit: minimize livecheck fetches Cask::Audit: reduce livecheck fetches Dec 23, 2025
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.

Looks good once small nit addressed! Can self-merge without rereview.

The existing livecheck-related cask audits can end up fetching the
livecheck URL four times and this can cause problems if a server has
sensitive rate limiting. This modifies the `livecheck_version` audit
to cache the result that's also used in the `hosting_with_livecheck`
and `livecheck_https_availability` audits. This will avoid a duplicate
fetch and can avoid another if the livecheck URL uses HTTPS and the
check doesn't fail. Most casks meet that criteria, so this should be
a notable improvement.

There's still room for improvement, as the `min_os` audit calls
`cask_sparkle_min_os`, which can also fetch the livecheck URL. It
would be ideal if we could cache the livecheck content to avoid this
duplicate fetch but I don't think that's possible with the current
setup, so I'll have to revisit that later.
@samford samford force-pushed the cask-audit-minimize-livecheck-fetches branch from 45c08f7 to 9ba98a6 Compare December 23, 2025 15:59
@samford samford added this pull request to the merge queue Dec 23, 2025
Merged via the queue into main with commit ff14abc Dec 23, 2025
37 checks passed
@samford samford deleted the cask-audit-minimize-livecheck-fetches branch December 23, 2025 19:48
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.

4 participants