Skip to content

brew determine-test-runners may exclude runners for supported architectures #19838

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

Open
3 tasks done
botantony opened this issue Apr 28, 2025 · 8 comments · Fixed by #19873
Open
3 tasks done

brew determine-test-runners may exclude runners for supported architectures #19838

botantony opened this issue Apr 28, 2025 · 8 comments · Fixed by #19873
Assignees
Labels
stale No recent activity

Comments

@botantony
Copy link
Contributor

brew doctor output

irrelevant

Verification

brew config output

irrelevant

What were you trying to do (and why)?

I was trying to update ibex formula (Homebrew/homebrew-core#221747). It used to have bottles for all supported configurations. After ARM Linux Homebrew support was added, cho-m added on_linux block with depends_on arch: :x86_64 dependency (Homebrew/homebrew-core@146de34)

CI jobs started runners only for x86 runners, even though this limit exists only for Linux.

What happened (include all command output)?

CI / setup_tests (pull_request) job had started only x86 runners (missing macOS <version>-arm64 runners)

What did you expect to happen?

I expected CI job to start all runners except ARM Linux

Step-by-step reproduction instructions (by running brew commands)

# update any formula with `on_macos` or `on_linux` block that has `depends_on arch:`
# if you can run `determine-test-runners` command on your machine try to
# modify any formula
@xakep8
Copy link

xakep8 commented Apr 29, 2025

@MikeMcQuaid I'd like to work on this issue could you please direct me to the right files for this issue?
I'm kinda new and not familiar with Homebrew's file structure 😓

@botantony
Copy link
Contributor Author

I do not want to discourage you, but if you're not able to find where this command is called, it wouldn't be easy to resolve this issue

@MikeMcQuaid
Copy link
Member

Exactly, sorry, we can't help you more than this.

@botantony
Copy link
Contributor Author

@xakep8 I can give you a hint: the problem is with test_runner_formula.rb and github_runner_matrix.rb files. Compatibility with platform/architecture is defined this way:

  sig { params(runner: GitHubRunner).returns(T::Array[TestRunnerFormula]) }
  def compatible_testing_formulae(runner)
    @compatible_testing_formulae[runner] ||= begin
      platform = runner.platform
      arch = runner.arch
      macos_version = runner.macos_version

      @testing_formulae.select do |formula|
        next false if macos_version && !formula.compatible_with?(macos_version)

        formula.public_send(:"#{platform}_compatible?") &&
          formula.public_send(:"#{arch}_compatible?")
      end
    end
  end

The problem is that methods in test_runner_formula like arm64_compatible? do not differentiate between depends_on arch: in on_<platform> block and outside of it. The code looks like this:

  sig { returns(T::Boolean) }
  def x86_64_only?
    formula.requirements.any? { |r| r.is_a?(ArchRequirement) && (r.arch == :x86_64) }
  end

  sig { returns(T::Boolean) }
  def x86_64_compatible?
    !arm64_only?
  end

  sig { returns(T::Boolean) }
  def arm64_only?
    formula.requirements.any? { |r| r.is_a?(ArchRequirement) && (r.arch == :arm64) }
  end

  sig { returns(T::Boolean) }
  def arm64_compatible?
    !x86_64_only?
  end

If you can figure out how to fix this: feel free to submit a PR

@botantony
Copy link
Contributor Author

I got some interesting results after playing with interactive ruby shell:

brew(main):013> :intercal.f.requirements
=> #<Set: {#<ArchRequirement: arch="x86_64" []>}>
brew(main):014> :ibex.f.requirements
=> #<Set: {}>
brew(main):015> :xlearn.f.requirements
=> #<Set: {#<ArchRequirement: arch="x86_64" []>}>

xlearn formula has ArchRequirement set for on platforms (both for Linux and macOS). ibex has these requirements only on Linux. intercal has these requirements only on macOS. It seems like because Homebrew runners use Linux, it doesn't take into account that the requirements are set for Linux but not macOS.

@botantony
Copy link
Contributor Author

@MikeMcQuaid can you reopen the issue and assign it to me 🙏

@botantony
Copy link
Contributor Author

As a proof of concept I made these changes

diff --git a/Library/Homebrew/github_runner_matrix.rb b/Library/Homebrew/github_runner_matrix.rb
index 2e424c479d..241356141f 100644
--- a/Library/Homebrew/github_runner_matrix.rb
+++ b/Library/Homebrew/github_runner_matrix.rb
@@ -255,18 +255,8 @@ class GitHubRunnerMatrix
       @testing_formulae.select do |formula|
         next false if macos_version && !formula.compatible_with?(macos_version)
 
-        simulate_arch = case arch
-        when :x86_64
-          :intel
-        when :arm64
-          :arm
-        else
-          :dunno
-        end
-        Homebrew::SimulateSystem.with(os: platform, arch: simulate_arch) do
-          formula.public_send(:"#{platform}_compatible?") &&
-            formula.public_send(:"#{arch}_compatible?")
-        end
+        formula.public_send(:"#{platform}_compatible?") &&
+          formula.public_send(:"#{arch}_#{platform}_compatible?")
       end
     end
   end
@@ -284,7 +274,7 @@ class GitHubRunnerMatrix
           next false if macos_version && !dependent_f.compatible_with?(macos_version)
 
           dependent_f.public_send(:"#{platform}_compatible?") &&
-            dependent_f.public_send(:"#{arch}_compatible?")
+            dependent_f.public_send(:"#{arch}_#{platform}_compatible?")
         end
 
         # These arrays will generally have been generated by different Formulary caches,
diff --git a/Library/Homebrew/test/github_runner_matrix_spec.rb b/Library/Homebrew/test/github_runner_matrix_spec.rb
index 2920ca6698..6f59d0ea12 100644
--- a/Library/Homebrew/test/github_runner_matrix_spec.rb
+++ b/Library/Homebrew/test/github_runner_matrix_spec.rb
@@ -27,6 +27,19 @@ RSpec.describe GitHubRunnerMatrix do
     setup_test_runner_formula("testball-depender-intel", ["testball", { arch: :x86_64 }])
   end
   let(:testball_depender_arm) { setup_test_runner_formula("testball-depender-arm", ["testball", { arch: :arm64 }]) }
+  let(:testball_complex) do
+    f = formula "testball-complex" do
+      url "https://brew.sh/testball-complex-1.0.tar.gz"
+
+      { linux: :x86_64, macos: :arm64}.each do |k, v|
+        send(:"on_#{k}") do
+          depends_on arch: v
+        end
+      end
+    end
+
+    TestRunnerFormula.new(f)
+  end
   let(:testball_depender_newest) do
     symbol, = newest_supported_macos
     setup_test_runner_formula("testball-depender-newest", ["testball", { macos: symbol }])
@@ -217,6 +230,19 @@ RSpec.describe GitHubRunnerMatrix do
             expect(get_runner_names(runner_matrix)).to eq(get_runner_names(runner_matrix, :arm64?))
           end
         end
+
+        context "when foo bar" do
+          it "activates baz quiz" do
+            allow(Homebrew::EnvConfig).to receive(:eval_all?).and_return(true)
+            allow(Formula).to receive(:all).and_return([testball, testball_complex].map(&:formula))
+
+            runner_matrix = described_class.new([testball], [], all_supported: false, dependent_matrix: true)
+            # expect(runner_matrix.runners.all?(&:active)).to be(false)
+            # expect(runner_matrix.runners.any?(&:active)).to be(true)
+            expect(get_runner_names(runner_matrix)).to eq(get_runner_names(runner_matrix, :arm64?))
+          end
+        end
+
       end
     end
   end
diff --git a/Library/Homebrew/test_runner_formula.rb b/Library/Homebrew/test_runner_formula.rb
index e3c3c987d6..4dedb11c7b 100644
--- a/Library/Homebrew/test_runner_formula.rb
+++ b/Library/Homebrew/test_runner_formula.rb
@@ -63,6 +63,34 @@ class TestRunnerFormula
     !x86_64_only?
   end
 
+  sig { returns(T::Boolean) }
+  def arm64_linux_compatible?
+    Homebrew::SimulateSystem.with(os: :linux) do
+      arm64_compatible?
+    end
+  end
+
+  sig { returns(T::Boolean) }
+  def arm64_macos_compatible?
+    Homebrew::SimulateSystem.with(os: :macos) do
+      arm64_compatible?
+    end
+  end
+
+  sig { returns(T::Boolean) }
+  def x86_64_linux_compatible?
+    Homebrew::SimulateSystem.with(os: :linux) do
+      x86_64_compatible?
+    end
+  end
+
+  sig { returns(T::Boolean) }
+  def x86_64_macos_compatible?
+    Homebrew::SimulateSystem.with(os: :macos) do
+      x86_64_compatible?
+    end
+  end
+
   sig { returns(T.nilable(MacOSRequirement)) }
   def versioned_macos_requirement
     formula.requirements.find { |r| r.is_a?(MacOSRequirement) && r.version_specified? }

Confusingly it works fine but the new test fails because list of runners is empty 🤔

Copy link

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@github-actions github-actions bot added the stale No recent activity label May 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale No recent activity
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants