diff --git a/Library/Homebrew/attestation.rb b/Library/Homebrew/attestation.rb index a828da9d52..eb8c9729d1 100644 --- a/Library/Homebrew/attestation.rb +++ b/Library/Homebrew/attestation.rb @@ -93,6 +93,24 @@ module Homebrew T.must(@gh_executable) end + # Prioritize installing `gh` first if it's in the formula list + # or check for the existence of the `gh` executable elsewhere. + # + # This ensures that a valid version of `gh` is installed before + # we use it to check the attestations of any other formulae we + # want to install. + # + # @api private + sig { params(formulae: T::Array[Formula]).returns(T::Array[Formula]) } + def self.sort_formulae_for_install(formulae) + if formulae.include?(Formula["gh"]) + [Formula["gh"]] | formulae + else + Homebrew::Attestation.gh_executable + formulae + end + end + # Verifies the given bottle against a cryptographic attestation of build provenance. # # The provenance is verified as originating from `signing_repository`, which is a `String` diff --git a/Library/Homebrew/cmd/install.rb b/Library/Homebrew/cmd/install.rb index 9b8b9d59e4..8197a6b694 100644 --- a/Library/Homebrew/cmd/install.rb +++ b/Library/Homebrew/cmd/install.rb @@ -263,13 +263,7 @@ module Homebrew end end - if Homebrew::Attestation.enabled? - if formulae.include?(Formula["gh"]) - formulae.unshift(T.must(formulae.delete(Formula["gh"]))) - else - Homebrew::Attestation.gh_executable - end - end + formulae = Homebrew::Attestation.sort_formulae_for_install(formulae) if Homebrew::Attestation.enabled? # if the user's flags will prevent bottle only-installations when no # developer tools are available, we need to stop them early on diff --git a/Library/Homebrew/cmd/reinstall.rb b/Library/Homebrew/cmd/reinstall.rb index 5f8a75144d..b81b390f20 100644 --- a/Library/Homebrew/cmd/reinstall.rb +++ b/Library/Homebrew/cmd/reinstall.rb @@ -109,7 +109,7 @@ module Homebrew sig { override.void } def run formulae, casks = T.cast( - args.named.to_formulae_and_casks(method: :resolve).partition { _1.is_a?(Formula) }, + args.named.to_resolved_formulae_to_casks, [T::Array[Formula], T::Array[Cask::Cask]], ) @@ -124,13 +124,7 @@ module Homebrew end end - if Homebrew::Attestation.enabled? - if formulae.include?(Formula["gh"]) - formulae.unshift(T.must(formulae.delete(Formula["gh"]))) - else - Homebrew::Attestation.gh_executable - end - end + formulae = Homebrew::Attestation.sort_formulae_for_install(formulae) if Homebrew::Attestation.enabled? Install.perform_preinstall_checks diff --git a/Library/Homebrew/cmd/upgrade.rb b/Library/Homebrew/cmd/upgrade.rb index cab4f27556..b803edee84 100644 --- a/Library/Homebrew/cmd/upgrade.rb +++ b/Library/Homebrew/cmd/upgrade.rb @@ -134,13 +134,7 @@ module Homebrew only_upgrade_formulae = formulae.present? && casks.blank? only_upgrade_casks = casks.present? && formulae.blank? - if Homebrew::Attestation.enabled? - if formulae.include?(Formula["gh"]) - formulae.unshift(formulae.delete(Formula["gh"])) - else - Homebrew::Attestation.gh_executable - end - end + formulae = Homebrew::Attestation.sort_formulae_for_install(formulae) if Homebrew::Attestation.enabled? upgrade_outdated_formulae(formulae) unless only_upgrade_casks upgrade_outdated_casks(casks) unless only_upgrade_formulae diff --git a/Library/Homebrew/test/attestation_spec.rb b/Library/Homebrew/test/attestation_spec.rb index 6e8d528061..312a1ba787 100644 --- a/Library/Homebrew/test/attestation_spec.rb +++ b/Library/Homebrew/test/attestation_spec.rb @@ -92,6 +92,46 @@ RSpec.describe Homebrew::Attestation do end end + # NOTE: `Homebrew::CLI::NamedArgs` will often return frozen arrays of formulae + # so that's why we test with frozen arrays here. + describe "::sort_formulae_for_install", :integration_test do + let(:gh) { Formula["gh"] } + let(:other) { Formula["other"] } + + before do + setup_test_formula("gh") + setup_test_formula("other") + end + + context "when `gh` is in the formula list" do + it "moves `gh` formulae to the front of the list" do + expect(described_class).not_to receive(:gh_executable) + + [ + [[gh], [gh]], + [[gh, other], [gh, other]], + [[other, gh], [gh, other]], + ].each do |input, output| + expect(described_class.sort_formulae_for_install(input.freeze)).to eq(output) + end + end + end + + context "when the formula list is empty" do + it "checks for the `gh` executable" do + expect(described_class).to receive(:gh_executable).once + expect(described_class.sort_formulae_for_install([].freeze)).to eq([]) + end + end + + context "when `gh` is not in the formula list" do + it "checks for the `gh` executable" do + expect(described_class).to receive(:gh_executable).once + expect(described_class.sort_formulae_for_install([other].freeze)).to eq([other]) + end + end + end + describe "::check_attestation" do before do allow(described_class).to receive(:gh_executable)