Handle nil cask urls caused by unsupported macOS version #15943

apainintheneck · 2023-09-03T04:01:24Z

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 style with your changes locally?
Have you successfully run brew typecheck with your changes locally?
Have you successfully run brew tests with your changes locally?

The goal here is to handle the case where a cask might have a nil url stanza because that cask is not available on the current version of macOS or the given architecture. This just moves those checks from the end of the Cask::Installer#fetch method to the beginning so that we don't try and download casks that are missing urls. A similar change was also applied to the brew fetch command.

This will now provide a helpful error message like so:

Error: This software does not run on macOS versions older than Big Sur.

Beyond that it no longer tries to run the url stanza with a nil value when loading casks from the API.

The goal here is to handle the case where a cask might have a nil url stanza because that cask is not available on the current version of macOS or the given architecture. This just moves those checks from the end of the `Cask::Installer#fetch` method to the beginning so that we don't try and download casks that are missing urls. This will now provide a helpful error message like so: ``` Error: This software does not run on macOS versions older than Big Sur. ``` Beyond that it no longer tries to run the url stanza with a nil value when loading casks from the API.

These urls can be nil if there is an unsatisfied macos version requirement. We check for false here because either the macos requirement can be satisfied and return true or can not be specified and return nil. If it's not specified, it means it can run on any macos version. The change in Cask::Download should provide better error messages in Downloadable but honestly we're better off just checking for the missing url higher up the call stack which is why I made the changes in the fetch command. Either way it seemed like a good idea while I'm here.

Library/Homebrew/cask/installer.rb

apainintheneck · 2023-09-03T04:14:48Z

Library/Homebrew/cask/cask_loader.rb

@@ -277,7 +277,7 @@ def load(config:)
            sha256 json_cask[:sha256]
          end

-          url json_cask[:url], **json_cask.fetch(:url_specs, {})
+          url json_cask[:url], **json_cask.fetch(:url_specs, {}) if json_cask[:url].present?


Before:

/u/l/Homebrew (master|✔) [1]$ brew info gpgfrontend Error: no implicit conversion of nil into Hash /usr/local/Homebrew/Library/Homebrew/cask/cask_loader.rb:280:in `block in load' /usr/local/Homebrew/Library/Homebrew/cask/cask.rb:102:in `instance_eval' /usr/local/Homebrew/Library/Homebrew/cask/cask.rb:102:in `refresh' /usr/local/Homebrew/Library/Homebrew/cask/cask.rb:95:in `config=' /usr/local/Homebrew/Library/Homebrew/cask/cask.rb:74:in `initialize' ... /usr/local/Homebrew/Library/Homebrew/cmd/info.rb:112:in `info' /usr/local/Homebrew/Library/Homebrew/brew.rb:94:in `<main>'

After:

/u/l/Homebrew (handle-nil-urls-in-cask-installer|✔) $ brew info gpgfrontend ==> gpgfrontend: 2.1.1 https://gpgfrontend.bktus.com/ Not installed From: https://github.com/Homebrew/homebrew-cask/blob/HEAD/Casks/g/gpgfrontend.rb ==> Name GpgFrontend ==> Description OpenPGP/GnuPG crypto, sign and key management tool ==> Artifacts GpgFrontend.app (App) ==> Analytics install: 32 (30 days), 108 (90 days), 210 (365 days)

apainintheneck · 2023-09-03T04:15:30Z

Library/Homebrew/cask/download.rb

@@ -24,6 +24,8 @@ def initialize(cask, quarantine: nil)

    sig { override.returns(T.nilable(::URL)) }
    def url
+      return if cask.url.nil?


Small thing that will give us better error messages in the future if we try downloading casks without urls.

apainintheneck · 2023-09-03T04:21:32Z

Library/Homebrew/cask/installer.rb

      verify_has_sha if require_sha? && !force?
+      check_requirements


Before:

/u/l/Homebrew (master|✔) $ brew install gpgfrontend Error: undefined method `specs' for nil:NilClass /usr/local/Homebrew/Library/Homebrew/cask/download.rb:27:in `url' /usr/local/Homebrew/Library/Homebrew/cask/download.rb:104:in `determine_url' /usr/local/Homebrew/Library/Homebrew/downloadable.rb:126:in `determine_url_mirrors' /usr/local/Homebrew/Library/Homebrew/downloadable.rb:74:in `downloader' /usr/local/Homebrew/Library/Homebrew/downloadable.rb:87:in `fetch' /usr/local/Homebrew/Library/Homebrew/cask/download.rb:51:in `fetch' /usr/local/Homebrew/Library/Homebrew/cask/installer.rb:167:in `download' /usr/local/Homebrew/Library/Homebrew/cask/installer.rb:70:in `fetch' /usr/local/Homebrew/Library/Homebrew/cask/installer.rb:99:in `install' /usr/local/Homebrew/Library/Homebrew/cmd/install.rb:244:in `block in install' /usr/local/Homebrew/Library/Homebrew/cmd/install.rb:233:in `each' /usr/local/Homebrew/Library/Homebrew/cmd/install.rb:233:in `install' /usr/local/Homebrew/Library/Homebrew/brew.rb:94:in `<main>'

(obviously without the Sorbet lines in the stack trace)

After:

/u/l/Homebrew (handle-nil-urls-in-cask-installer|✔) [1]$ brew install gpgfrontend Error: This software does not run on macOS versions older than Big Sur.

apainintheneck · 2023-09-03T04:26:23Z

Library/Homebrew/cmd/fetch.rb

+            if cask.depends_on.macos&.satisfied? == false
+              opoo "#{cask.token}: #{cask.depends_on.macos.message(type: :cask)}"
+              next
+            end


Command: brew fetch gpgfrontend

Before:

Error: undefined method `specs' for nil:NilClass /usr/local/Homebrew/Library/Homebrew/cask/download.rb:27:in `url' /usr/local/Homebrew/Library/Homebrew/cask/download.rb:104:in `determine_url' /usr/local/Homebrew/Library/Homebrew/downloadable.rb:126:in `determine_url_mirrors' /usr/local/Homebrew/Library/Homebrew/downloadable.rb:74:in `downloader' /usr/local/Homebrew/Library/Homebrew/downloadable.rb:50:in `cached_download' /usr/local/Homebrew/Library/Homebrew/cmd/fetch.rb:232:in `fetch_fetchable' /usr/local/Homebrew/Library/Homebrew/cmd/fetch.rb:197:in `fetch_cask' /usr/local/Homebrew/Library/Homebrew/cmd/fetch.rb:174:in `block (3 levels) in fetch' /usr/local/Homebrew/Library/Homebrew/simulate_system.rb:29:in `with' /usr/local/Homebrew/Library/Homebrew/cmd/fetch.rb:167:in `block (2 levels) in fetch' /usr/local/Homebrew/Library/Homebrew/cmd/fetch.rb:164:in `each' /usr/local/Homebrew/Library/Homebrew/cmd/fetch.rb:164:in `block in fetch' /usr/local/Homebrew/Library/Homebrew/cmd/fetch.rb:94:in `each' /usr/local/Homebrew/Library/Homebrew/cmd/fetch.rb:94:in `fetch' /usr/local/Homebrew/Library/Homebrew/brew.rb:94:in `<main>'

(without the Sorbet stack trace lines)

After:

Warning: gpgfrontend: This software does not run on macOS versions older than Big Sur.

MikeMcQuaid

Great work!

EricFromCanada · 2023-09-04T16:00:07Z

There's a similar issue in brew audit --cask for casks that don't have an on_<system> block for the current system, e.g.:

$ brew audit --skip-style --except=version homebrew/cask/apparency -d
/usr/local/Homebrew/Library/Homebrew/brew.rb (Formulary::TapLoader): loading /usr/local/Homebrew/Library/Taps/homebrew/homebrew-cask/Formula/apparency.rb
/usr/local/Homebrew/Library/Homebrew/brew.rb (Cask::CaskLoader::FromTapLoader): loading homebrew/cask/apparency
==> Auditing Cask apparency on os high_sierra and arch intel
/usr/local/Homebrew/Library/Homebrew/brew.rb (Cask::CaskLoader::FromTapPathLoader): loading /usr/local/Homebrew/Library/Taps/homebrew/homebrew-cask/Casks/a/apparency.rb
==> Auditing pkg stanza: allow_untrusted
==> Auditing stanzas which require an uninstall
==> Auditing preflight and postflight stanzas
==> Auditing single uninstall_* and zap stanzas
==> Auditing required stanzas
==> undefined method `latest?' for nil:NilClass
/usr/local/Homebrew/Library/Homebrew/cask/audit.rb:270:in `audit_latest_with_livecheck'
...

apainintheneck · 2023-09-04T17:04:53Z

@EricFromCanada Good catch! I was wondering if this would affect any other part of the codebase but I couldn't find anything with a quick search. I can try and make a PR for that if you're not already working on it.

EricFromCanada · 2023-09-04T18:10:58Z

Go ahead, I've got other metaphorical fish to fry.

apainintheneck · 2023-09-04T21:01:36Z

It looks like MacOSRequirement and ArchRequirement do not respect SimulateSystem which means this isn't a super easy, one line fix. It also means that the changes to brew fetch made in this PR won't work as intended when users are simulating other os/arch combinations but I guess it never really worked as intended anyway. I'm not sure how common of a use case that was before though.

/u/l/Homebrew (master|✔) $ brew fetch --os=all gpgfrontend -d
/usr/local/Homebrew/Library/Homebrew/brew.rb (Cask::CaskLoader::FromAPILoader): loading gpgfrontend
/usr/local/Homebrew/Library/Homebrew/brew.rb (Cask::CaskLoader::FromAPILoader): loading gpgfrontend
Warning: gpgfrontend: This software does not run on macOS versions older than Big Sur.
/usr/local/Homebrew/Library/Homebrew/brew.rb (Cask::CaskLoader::FromAPILoader): loading gpgfrontend
Warning: gpgfrontend: This software does not run on macOS versions older than Big Sur.
/usr/local/Homebrew/Library/Homebrew/brew.rb (Cask::CaskLoader::FromAPILoader): loading gpgfrontend
Warning: gpgfrontend: This software does not run on macOS versions older than Big Sur.
/usr/local/Homebrew/Library/Homebrew/brew.rb (Cask::CaskLoader::FromAPILoader): loading gpgfrontend
Warning: gpgfrontend: This software does not run on macOS versions older than Big Sur.
/usr/local/Homebrew/Library/Homebrew/brew.rb (Cask::CaskLoader::FromAPILoader): loading gpgfrontend
Warning: gpgfrontend: This software does not run on macOS versions older than Big Sur.
/usr/local/Homebrew/Library/Homebrew/brew.rb (Cask::CaskLoader::FromAPILoader): loading gpgfrontend
Warning: gpgfrontend: This software does not run on macOS versions older than Big Sur.
/usr/local/Homebrew/Library/Homebrew/brew.rb (Cask::CaskLoader::FromAPILoader): loading gpgfrontend
Warning: gpgfrontend: This software does not run on macOS versions older than Big Sur.
/usr/local/Homebrew/Library/Homebrew/brew.rb (Cask::CaskLoader::FromAPILoader): loading gpgfrontend
Warning: gpgfrontend: This software does not run on macOS versions older than Big Sur.
/usr/local/Homebrew/Library/Homebrew/brew.rb (Cask::CaskLoader::FromAPILoader): loading gpgfrontend
Warning: gpgfrontend: This software does not run on macOS versions older than Big Sur.

apainintheneck · 2023-09-04T21:06:19Z

On second thought, we could probably just skip all os/arch combinations that have nil cask urls. I'm not sure if this affects anything else though.

MikeMcQuaid · 2023-09-04T21:29:31Z

It looks like MacOSRequirement and ArchRequirement do not respect SimulateSystem which means this isn't a super easy, one line fix.

@apainintheneck could you open up a help wanted issue to get this fixed?

apainintheneck · 2023-09-04T22:09:12Z

@MikeMcQuaid I'm not sure it is needed for most use cases or if we want requirements to check the simulated system instead of the real one.

@Rylan12 @reitermarkus Do you have any thoughts on this? I seem to remember you've both worked a bit on this part of the codebase before.

MikeMcQuaid · 2023-09-05T21:01:15Z

@MikeMcQuaid I'm not sure it is needed for most use cases or if we want requirements to check the simulated system instead of the real one.

I feel like the simulated system should be checked, yeh.

apainintheneck added 2 commits September 2, 2023 20:56

apainintheneck added bug Reproducible Homebrew/brew bug cask Homebrew Cask labels Sep 3, 2023

apainintheneck commented Sep 3, 2023

View reviewed changes

apainintheneck changed the title ~~Handle nil urls in cask installer~~ Handle nil cask urls caused by unsupported macOS version Sep 3, 2023

apainintheneck marked this pull request as ready for review September 3, 2023 04:35

MikeMcQuaid approved these changes Sep 3, 2023

View reviewed changes

cask/installer: remove outdated comment.

daa175e

MikeMcQuaid enabled auto-merge September 3, 2023 13:09

MikeMcQuaid merged commit db1267b into Homebrew:master Sep 3, 2023
24 checks passed

apainintheneck mentioned this pull request Sep 4, 2023

cmd/fetch & cmd/audit: handle unsupported cask os/arch combos #15956

Merged

7 tasks

This was referenced Sep 7, 2023

Improve cask audit #15977

Merged

Extend SimulateSystem to MacOS.version #15985

Closed

github-actions bot added the outdated PR was locked due to age label Oct 6, 2023

github-actions bot locked as resolved and limited conversation to collaborators Oct 6, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Handle nil cask urls caused by unsupported macOS version #15943

Handle nil cask urls caused by unsupported macOS version #15943

apainintheneck commented Sep 3, 2023 •

edited

Loading

apainintheneck Sep 3, 2023

apainintheneck Sep 3, 2023

apainintheneck Sep 3, 2023

apainintheneck Sep 3, 2023

MikeMcQuaid left a comment

EricFromCanada commented Sep 4, 2023

apainintheneck commented Sep 4, 2023

EricFromCanada commented Sep 4, 2023

apainintheneck commented Sep 4, 2023

apainintheneck commented Sep 4, 2023

MikeMcQuaid commented Sep 4, 2023

apainintheneck commented Sep 4, 2023

MikeMcQuaid commented Sep 5, 2023

Handle nil cask urls caused by unsupported macOS version #15943

Handle nil cask urls caused by unsupported macOS version #15943

Conversation

apainintheneck commented Sep 3, 2023 • edited Loading

apainintheneck Sep 3, 2023

Choose a reason for hiding this comment

apainintheneck Sep 3, 2023

Choose a reason for hiding this comment

apainintheneck Sep 3, 2023

Choose a reason for hiding this comment

apainintheneck Sep 3, 2023

Choose a reason for hiding this comment

MikeMcQuaid left a comment

Choose a reason for hiding this comment

EricFromCanada commented Sep 4, 2023

apainintheneck commented Sep 4, 2023

EricFromCanada commented Sep 4, 2023

apainintheneck commented Sep 4, 2023

apainintheneck commented Sep 4, 2023

MikeMcQuaid commented Sep 4, 2023

apainintheneck commented Sep 4, 2023

MikeMcQuaid commented Sep 5, 2023

apainintheneck commented Sep 3, 2023 •

edited

Loading