Skip to content
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

various casks: fix macOS version requirements #176710

Merged
merged 4 commits into from
Jun 26, 2024

Conversation

EricFromCanada
Copy link
Member

Important: Do not tick a checkbox if you haven’t performed its action. Honesty is indispensable for a smooth review process.

In the following questions <cask> is the token of the cask you're submitting.

After making any changes to a cask, existing or new, verify:


  • removes minimum macOS version requirements that were preventing installations on older versions
  • consolidates on_system-specific macOS version requirements to a single declaration for the cask, allowing its formulae.brew.sh entry to display all available versions

@EricFromCanada EricFromCanada added the ci-syntax-only Only run syntax checks on CI. Use only for bulk changes. label Jun 14, 2024
@bevanjkay
Copy link
Member

bevanjkay commented Jun 14, 2024

consolidates on_system-specific macOS version requirements to a single declaration for the cask, allowing its formulae.brew.sh entry to display all available versions

I'm not sure how well this will work thinking future-forward. There is an audit that is currently for new casks only (or with the --strict) flag that compares the app's info.plist file with what we have, so these will likely fail the audit if there is a mismatch. There's potential for this audit to be applied to existing casks in future.

@bevanjkay
Copy link
Member

brew audit --online airfoil --new

audit for airfoil: failed
 - Upstream defined :sonoma as the minimum OS version and the cask defined :big_sur

@Bo98
Copy link
Member

Bo98 commented Jun 14, 2024

these will likely fail the audit

That's definitely an audit bug/shortcoming. It should be smarter and check if it's in an on_os block (and if the range of that block is correct).

If using on_system blocks, set a single minimum macOS for any version defined by the cask.
i.e. if all supported macOS versions are compatible, or are handled by on_system blocks.
@EricFromCanada
Copy link
Member Author

With Homebrew/brew#17548 merged, cask/audit.rb now prioritizes on_os block declarations over depends_on, which should address the above concerns.

@krehel
Copy link
Member

krehel commented Jun 26, 2024

Makes a lot of sense @EricFromCanada. Thanks for taking this on and doing some heavy lifting. I'm good here!

@EricFromCanada EricFromCanada removed the request for review from razvanazamfirei June 26, 2024 02:21
@krehel krehel merged commit 15163db into Homebrew:master Jun 26, 2024
7 checks passed
@EricFromCanada EricFromCanada deleted the cask-legacy-blocks branch June 26, 2024 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge-skip ci-syntax-only Only run syntax checks on CI. Use only for bulk changes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants