Make Cask CI generate correctly for partial arch dependencies #21380

SMillerDev · 2026-01-09T21:40:32Z

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?

SMillerDev · 2026-01-09T21:40:48Z

Partially split from: #20334

Copilot

Pull request overview

This pull request enhances Cask CI matrix generation to correctly handle partial architecture dependencies, particularly when casks have OS-specific architecture requirements (e.g., requiring x86_64 on macOS but arm64 on Linux). The changes introduce per-OS architecture filtering to ensure CI runners are properly selected based on platform-specific constraints.

Key Changes

Modified the architectures method to accept an os parameter, enabling OS-specific architecture filtering based on simulated system environments
Updated filter_runners to separately filter macOS and Linux runners based on their respective architecture requirements
Added contains_os_specific_artifacts? method to detect casks with platform-specific artifacts that affect OS compatibility
Introduced comprehensive test coverage for various combinations of OS and architecture dependencies

Reviewed changes

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

File	Description
Library/Homebrew/dev-cmd/generate-cask-ci-matrix.rb	Enhanced architecture filtering to support OS-specific dependencies; modified `filter_runners` and `architectures` methods to handle per-OS architecture constraints
Library/Homebrew/cask/cask.rb	Added `contains_os_specific_artifacts?` method to improve Linux support detection by checking for OS-specific artifact constraints
Library/Homebrew/cask/artifact.rb	Added `LINUX_ONLY_ARTIFACTS` constant (empty placeholder for future Linux-specific artifacts)
Library/Homebrew/test/dev-cmd/generate-cask-ci-matrix_spec.rb	Added comprehensive test cases covering various scenarios including mixed OS/arch dependencies, OS-specific dependencies, and system-wide constraints

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

Library/Homebrew/test/dev-cmd/generate-cask-ci-matrix_spec.rb

Library/Homebrew/dev-cmd/generate-cask-ci-matrix.rb

Copilot · 2026-01-09T21:48:55Z

Library/Homebrew/dev-cmd/generate-cask-ci-matrix.rb

          cask = Cask::CaskLoader.load(path.expand_path)

          runners, multi_os = runners(cask:)
          runners.product(architectures(cask:)).filter_map do |runner, arch|


The architectures method is being called on line 274 without the os parameter, which means it will default to :macos. This could cause incorrect architecture filtering when dealing with Linux runners. The method should be called per-runner with the appropriate OS based on the runner's symbol to correctly handle OS-specific architecture dependencies.

Library/Homebrew/cask/cask.rb

Library/Homebrew/dev-cmd/generate-cask-ci-matrix.rb

Copilot · 2026-01-09T21:48:56Z

Library/Homebrew/cask/cask.rb

+    sig { returns(T::Boolean) }
+    def contains_os_specific_artifacts?
+      return false unless @dsl.on_system_blocks_exist?
+
+      any_loaded = false
+      @contains_os_specific_artifacts ||= begin
+        OnSystem::VALID_OS_ARCH_TAGS.each do |bottle_tag|
+          Homebrew::SimulateSystem.with_tag(bottle_tag) do
+            refresh
+
+            any_loaded = true if artifacts.any? do |artifact|
+              (bottle_tag.linux? && ::Cask::Artifact::MACOS_ONLY_ARTIFACTS.include?(artifact.class)) ||
+              (bottle_tag.macos? && ::Cask::Artifact::LINUX_ONLY_ARTIFACTS.include?(artifact.class))
+            end
+          end
+        end
+
+        any_loaded
+      rescue CaskInvalidError
+        # Invalid cask
+      ensure
+        refresh
      end

-      @dsl.os.present?
+      @contains_os_specific_artifacts
    end


The new contains_os_specific_artifacts? method lacks test coverage. Given that Library/Homebrew/test/cask/cask_spec.rb exists with comprehensive tests for other Cask methods, this new method should have corresponding test cases to verify its behavior across different scenarios (e.g., casks with OS-specific artifacts, casks without, error cases).

I agree it'd be good to have even just one very simple test for this. Could ask Copilot to try it for you.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

MikeMcQuaid

Thanks! A few small comments.

MikeMcQuaid · 2026-01-12T09:43:54Z

Library/Homebrew/dev-cmd/generate-cask-ci-matrix.rb

            native_runner_arch = arch == runner.fetch(:arch)
-            # we don't need to run simulated archs on Linux
+            # we don't need to run simulated archs on Linux or macOS Sequoia
+            # because it has a GitHub hosted x86_64 runner


Suggested change

# because it has a GitHub hosted x86_64 runner

# because there is a GitHub hosted Linux x86_64 runner

or something? current comment doesn't make sense

MikeMcQuaid · 2026-01-12T09:44:21Z

Library/Homebrew/cask/cask.rb

+    sig { returns(T::Boolean) }
+    def contains_os_specific_artifacts?
+      return false unless @dsl.on_system_blocks_exist?
+
+      any_loaded = false
+      @contains_os_specific_artifacts ||= begin
+        OnSystem::VALID_OS_ARCH_TAGS.each do |bottle_tag|
+          Homebrew::SimulateSystem.with_tag(bottle_tag) do
+            refresh
+
+            any_loaded = true if artifacts.any? do |artifact|
+              (bottle_tag.linux? && ::Cask::Artifact::MACOS_ONLY_ARTIFACTS.include?(artifact.class)) ||
+              (bottle_tag.macos? && ::Cask::Artifact::LINUX_ONLY_ARTIFACTS.include?(artifact.class))
+            end
+          end
+        end
+
+        any_loaded
+      rescue CaskInvalidError
+        # Invalid cask
+      ensure
+        refresh
      end

-      @dsl.os.present?
+      @contains_os_specific_artifacts
    end


I agree it'd be good to have even just one very simple test for this. Could ask Copilot to try it for you.

Copilot AI review requested due to automatic review settings January 9, 2026 21:40

Copilot started reviewing on behalf of SMillerDev January 9, 2026 21:41 View session

SMillerDev force-pushed the feat/cask/arch_ci branch from 644dfa9 to 22ab6a6 Compare January 9, 2026 21:41

Copilot AI reviewed Jan 9, 2026

View reviewed changes

SMillerDev and others added 3 commits January 12, 2026 09:16

Make Cask CI generate correctly for partial arch dependencies

56e832f

Update Library/Homebrew/cask/cask.rb

dbb02e7

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Update Library/Homebrew/dev-cmd/generate-cask-ci-matrix.rb

e4c9380

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

SMillerDev force-pushed the feat/cask/arch_ci branch from b8bbc6a to e4c9380 Compare January 12, 2026 08:17

MikeMcQuaid reviewed Jan 12, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Make Cask CI generate correctly for partial arch dependencies #21380

Make Cask CI generate correctly for partial arch dependencies #21380

SMillerDev commented Jan 9, 2026

Uh oh!

SMillerDev commented Jan 9, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 9, 2026

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 9, 2026

Uh oh!

MikeMcQuaid Jan 12, 2026

Uh oh!

MikeMcQuaid left a comment

Uh oh!

MikeMcQuaid Jan 12, 2026

Uh oh!

MikeMcQuaid Jan 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	# because it has a GitHub hosted x86_64 runner
	# because there is a GitHub hosted Linux x86_64 runner

Uh oh!

Make Cask CI generate correctly for partial arch dependencies #21380

Are you sure you want to change the base?

Make Cask CI generate correctly for partial arch dependencies #21380

Conversation

SMillerDev commented Jan 9, 2026

Uh oh!

SMillerDev commented Jan 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 9, 2026

Choose a reason for hiding this comment

Uh oh!

MikeMcQuaid Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

MikeMcQuaid left a comment

Choose a reason for hiding this comment

Uh oh!

MikeMcQuaid Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

MikeMcQuaid Jan 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants