Skip to content

Treat cask OS support as explicit data - #23433

Open
MikeMcQuaid wants to merge 2 commits into
cask-os-stanza-fixfrom
cask-explicit-os-support
Open

Treat cask OS support as explicit data#23433
MikeMcQuaid wants to merge 2 commits into
cask-os-stanza-fixfrom
cask-explicit-os-support

Conversation

@MikeMcQuaid

Copy link
Copy Markdown
Member
  • Align casks with formulae: platform support comes from depends_on :macos/:linux/macos: data rather than generation heuristics guessing intent from os stanzas, Linux checksums or on_linux blocks.
  • Cask#to_hash_with_variations now emits variations for every valid OS/arch tag whenever on_system blocks exist, matching Formula#to_hash_with_variations; the Linux-specific gate and its sha256_set_for_linux? and on_linux_blocks_exist? tracking are removed. macOS-only casks publish truthful Linux variations, e.g. a null sha256, instead of omitting them.
  • Cask::Installer gains a first-class unsupported-system error: API-loaded casks with no activatable artifact for the running system fail with "This cask is not available on macOS/Linux." instead of installing nothing. Audited casks always declare an activatable artifact for the systems they support, so missing artifacts in API data mean the system is unsupported. Source loads keep working for unaudited casks, e.g. naked containers.
  • A sweep of the full homebrew/cask generation pipeline (all casks, both Linux tags, including internal per-tag payloads) confirmed no cask needs new depends_on annotations and nothing regresses.

  • Have you followed our Contributing guidelines?
  • Have you checked for other open Pull Requests for the same change?
  • Have you explained what your changes do? Performance claims (e.g. "this is faster") must include Hyperfine benchmarks.
  • Have you explained why you'd like these changes included, not just what they do?
  • For bug fixes, have you given step-by-step brew commands to reproduce the bug?
  • Have you written new tests (excluding integration tests)? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) locally?

  • I did not use AI/LLM to create this PR, or I disclosed the tool/model below and reviewed its output; I did not attribute commits to AI and will answer maintainer questions and review comments myself without AI/LLM.

Fable 5 max with local review and testing.


Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 shifts cask OS/arch support to be treated as explicit data (via depends_on :macos/:linux and related stanza data) rather than inferred heuristically, and updates API variation output and installer behavior to match that model.

Changes:

  • Emit Cask#to_hash_with_variations entries for all valid OS/arch tags whenever on_system blocks exist (removing Linux-variation gating heuristics).
  • Add Cask::Installer#check_supported_system to fail fast for API-loaded casks that have no activatable install artifacts for the current system.
  • Remove now-unused Linux tracking (on_linux_blocks_exist?, sha256_set_for_linux?) from the DSL and Sorbet RBI; adjust specs accordingly.

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
Library/Homebrew/test/cask/installer_spec.rb Adds coverage for unsupported-system errors; adjusts API-loaded behavior expectations and fixtures.
Library/Homebrew/test/cask/cask_spec.rb Updates expected variations output to include Linux tags for casks with on_system blocks.
Library/Homebrew/sorbet/rbi/dsl/cask/cask.rbi Removes the RBI signature for on_linux_blocks_exist? after DSL tracking removal.
Library/Homebrew/on_system.rb Removes @on_linux_blocks_exist bookkeeping from on_linux/on_system block setup.
Library/Homebrew/cask/installer.rb Introduces check_supported_system into the installer requirements flow.
Library/Homebrew/cask/dsl.rb Removes Linux-specific tracking accessors/state from the cask DSL.
Library/Homebrew/cask/cask.rb Removes the Linux-variation skip heuristic so variations are generated for all OS/arch tags.
Files not reviewed (1)
  • Library/Homebrew/sorbet/rbi/dsl/cask/cask.rbi: File type not supported
Suppressed comments (3)

Library/Homebrew/test/cask/installer_spec.rb:255

  • This expectation hardcodes "macOS", but the installer chooses "macOS" vs "Linux" at runtime. The test should build the expected string dynamically so it passes on both macOS and Linux CI.
    it "treats uninstall-only artifacts as nothing to install" do
      zap_only_cask = Cask::Cask.new("with-zap-only", loaded_from_api: true) do
        version "1.0"
        sha256 :no_check
        url "https://brew.sh/x.zip"
        zap trash: "~/Library/Caches/brew-test"
      end
      expect do
        described_class.new(zap_only_cask).check_supported_system
      end.to raise_error(Cask::CaskError, "with-zap-only: This cask is not available on macOS.")
    end

Library/Homebrew/test/cask/installer_spec.rb:552

  • This spec is asserting an exact call count for loaded_from_api?, which makes it sensitive to internal refactors (like adding another requirement check). Prefer stubbing with allow (or at least avoiding exact counts) so the test asserts behavior rather than implementation details.
      it "uninstalls cask" do
        source_caffeine = Cask::CaskLoader.load(path)
        expect(Homebrew::API::Cask).to receive(:source_download_cask).twice.and_return(source_caffeine)

        caffeine = Cask::CaskLoader.load(path)
        expect(caffeine).to receive(:loaded_from_api?).exactly(3).times.and_return(true)
        expect(caffeine).to receive(:caskfile_only?).twice.and_return(true)
        expect(caffeine).to receive(:installed_caskfile).once.and_return(invalid_path)

Library/Homebrew/cask/cask.rb:629

  • Now that Linux variations are always emitted, casks that set url using #{version} inside macOS-only on_* blocks can produce Linux-variation URLs containing a double-slash (because version is nil on Linux tags). That makes the emitted Linux url misleading/invalid for API consumers; consider emitting null/omitting url (and url_specs) when version is nil for a given OS/arch tag.
      if dsl!.on_system_blocks_exist?
        begin
          OnSystem::VALID_OS_ARCH_TAGS.each do |bottle_tag|
            macos_requirements = [depends_on.macos, depends_on.maximum_macos].compact
            next if bottle_tag.macos? &&
                    macos_requirements.present? &&
                    !dsl!.depends_on_set_in_block? &&
                    macos_requirements.any? { |requirement| !requirement.allows?(bottle_tag.to_macos_version) }

            refresh_for_tag(bottle_tag) do
              to_h_with_language_variations.each do |key, value|
                next if HASH_KEYS_TO_SKIP.include? key
                next if value.to_s == hash[key].to_s

                variations[bottle_tag.to_sym] ||= {}
                variations[bottle_tag.to_sym][key] = value
              end

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

Comment thread Library/Homebrew/test/cask/installer_spec.rb
Comment thread Library/Homebrew/test/cask/installer_spec.rb Outdated
- `Cask#to_hash_with_variations` skipped all Linux variations for
  casks declaring Linux support only inside `on_linux`/`on_system`
  blocks, e.g. `zen`, as the API JSON is generated on macOS where
  those blocks are invisible; the published JSON then fell back to
  macOS artifacts on Linux and installs failed with a misleading
  "This cask requires macOS." error. Track `on_linux`/`on_system`
  blocks and emit Linux variations for casks that have them.
- `sha256` raised for an architecture missing a checksum for the
  running OS, e.g. `unity-hub` on `arm64_linux`, so its variation
  was silently dropped and ARM64 Linux fell back to macOS data too.
  Raise only on the real system: under `SimulateSystem` simulation
  `sha256` is now nil so variations for missing architectures carry
  their `on_linux` `depends_on arch:` and artifacts and Linux users
  get the correct unsupported-architecture error instead.
- `Readall.valid_casks?` skipped casks whose files textually matched
  `depends_on macos:` anywhere, including inside `on_macos` blocks,
  so cross-OS casks like `unity-hub` were never validated for Linux.
  Rely on `supports_linux?` instead and let `depends_on arch:` excuse
  architectures a cask deliberately omits Linux checksums for.
- Add `appimagedir` to the test cask config so `app_image` artifacts
  can be serialised in tests.

Fixes #23427.
- Align casks with formulae: platform support comes from
  `depends_on :macos`/`:linux`/`macos:` data rather than generation
  heuristics guessing intent from `os` stanzas, Linux checksums or
  `on_linux` blocks.
- `Cask#to_hash_with_variations` now emits variations for every
  valid OS/arch tag whenever `on_system` blocks exist, matching
  `Formula#to_hash_with_variations`; the Linux-specific gate and its
  `sha256_set_for_linux?` and `on_linux_blocks_exist?` tracking are
  removed. macOS-only casks publish truthful Linux variations, e.g.
  a `null` `sha256`, instead of omitting them.
- `Cask::Installer` gains a first-class unsupported-system error:
  API-loaded casks with no activatable artifact for the running
  system fail with "This cask is not available on macOS/Linux."
  instead of installing nothing. Audited casks always declare an
  activatable artifact for the systems they support, so missing
  artifacts in API data mean the system is unsupported. Source
  loads keep working for unaudited casks, e.g. naked containers.
- A sweep of the full homebrew/cask generation pipeline (all casks,
  both Linux tags, including internal per-tag payloads) confirmed
  no cask needs new `depends_on` annotations and nothing regresses.
@MikeMcQuaid
MikeMcQuaid force-pushed the cask-explicit-os-support branch from 8b5275a to 0d37802 Compare August 4, 2026 16:04
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.

2 participants