From 2d47193d22f5847ce73a31b22a9094852ab80148 Mon Sep 17 00:00:00 2001 From: apainintheneck Date: Tue, 2 Apr 2024 23:10:37 -0700 Subject: [PATCH] cmd/untap: move module methods back into the cmd The extra module was their to facilitate testing but now that everything is properly namespaced and each command is an instance we can just move the methods into the command. Since it's an instance now we don't need to be as careful about caching either. --- Library/Homebrew/cmd/untap.rb | 55 ++++++++++- Library/Homebrew/test/cmd/untap_spec.rb | 125 ++++++++++++++++++++++- Library/Homebrew/test/untap_spec.rb | 126 ------------------------ Library/Homebrew/untap.rb | 60 ----------- 4 files changed, 175 insertions(+), 191 deletions(-) delete mode 100644 Library/Homebrew/test/untap_spec.rb delete mode 100644 Library/Homebrew/untap.rb diff --git a/Library/Homebrew/cmd/untap.rb b/Library/Homebrew/cmd/untap.rb index f2a2547d41..17c5c11af6 100644 --- a/Library/Homebrew/cmd/untap.rb +++ b/Library/Homebrew/cmd/untap.rb @@ -2,11 +2,10 @@ # frozen_string_literal: true require "abstract_command" -require "untap" module Homebrew module Cmd - class UntapCmd < AbstractCommand + class Untap < AbstractCommand cmd_args do description <<~EOS Remove a tapped formula repository. @@ -23,8 +22,8 @@ module Homebrew odie "Untapping #{tap} is not allowed" if tap.core_tap? && Homebrew::EnvConfig.no_install_from_api? if Homebrew::EnvConfig.no_install_from_api? || (!tap.core_tap? && !tap.core_cask_tap?) - installed_tap_formulae = Untap.installed_formulae_for(tap:) - installed_tap_casks = Untap.installed_casks_for(tap:) + installed_tap_formulae = installed_formulae_for(tap:) + installed_tap_casks = installed_casks_for(tap:) if installed_tap_formulae.present? || installed_tap_casks.present? installed_names = (installed_tap_formulae + installed_tap_casks.map(&:token)).join("\n") @@ -45,6 +44,54 @@ module Homebrew tap.uninstall manual: true end end + + # All installed formulae currently available in a tap by formula full name. + sig { params(tap: Tap).returns(T::Array[Formula]) } + def installed_formulae_for(tap:) + tap.formula_names.filter_map do |formula_name| + next unless installed_formulae_names.include?(T.must(formula_name.split("/").last)) + + formula = begin + Formulary.factory(formula_name) + rescue FormulaUnavailableError + # Don't blow up because of a single unavailable formula. + next + end + + # Can't use Formula#any_version_installed? because it doesn't consider + # taps correctly. + formula if formula.installed_kegs.any? { |keg| keg.tab.tap == tap } + end + end + + # All installed casks currently available in a tap by cask full name. + sig { params(tap: Tap).returns(T::Array[Cask::Cask]) } + def installed_casks_for(tap:) + tap.cask_tokens.filter_map do |cask_token| + next unless installed_cask_tokens.include?(T.must(cask_token.split("/").last)) + + cask = begin + Cask::CaskLoader.load(cask_token) + rescue Cask::CaskUnavailableError + # Don't blow up because of a single unavailable cask. + next + end + + cask if cask.installed? + end + end + + private + + sig { returns(T::Set[String]) } + def installed_formulae_names + @installed_formulae_names ||= T.let(Formula.installed_formula_names.to_set.freeze, T.nilable(T::Set[String])) + end + + sig { returns(T::Set[String]) } + def installed_cask_tokens + @installed_cask_tokens ||= T.let(Cask::Caskroom.tokens.to_set.freeze, T.nilable(T::Set[String])) + end end end end diff --git a/Library/Homebrew/test/cmd/untap_spec.rb b/Library/Homebrew/test/cmd/untap_spec.rb index ba9bb56ec4..bc5d286724 100644 --- a/Library/Homebrew/test/cmd/untap_spec.rb +++ b/Library/Homebrew/test/cmd/untap_spec.rb @@ -3,7 +3,9 @@ require "cmd/shared_examples/args_parse" require "cmd/untap" -RSpec.describe Homebrew::Cmd::UntapCmd do +RSpec.describe Homebrew::Cmd::Untap do + let(:class_instance) { described_class.new(%w[arg1]) } + it_behaves_like "parseable arguments" it "untaps a given Tap", :integration_test do @@ -14,4 +16,125 @@ RSpec.describe Homebrew::Cmd::UntapCmd do .and not_to_output.to_stdout .and be_a_success end + + describe "#installed_formulae_for", :integration_test do + shared_examples "finds installed formulae in tap" do + def load_formula(name:, with_formula_file: false, mock_install: false) + formula = if with_formula_file + path = setup_test_formula(name, tap:) + Formulary.factory(path) + else + formula(name, tap:) do + url "https://brew.sh/#{name}-1.0.tgz" + end + end + + if mock_install + keg_path = HOMEBREW_CELLAR/name/"1.2.3" + keg_path.mkpath + + tab_path = keg_path/Tab::FILENAME + tab_path.write <<~JSON + { + "source": { + "tap": "#{tap}" + } + } + JSON + end + + formula + end + + let!(:currently_installed_formula) do + load_formula(name: "current_install", with_formula_file: true, mock_install: true) + end + + before do + # Formula that is available from a tap but not installed. + load_formula(name: "no_install", with_formula_file: true) + + # Formula that was installed from a tap but is no longer available from that tap. + load_formula(name: "legacy_install", mock_install: true) + + tap.clear_cache + end + + it "returns the expected formulae" do + expect(class_instance.installed_formulae_for(tap:).map(&:full_name)) + .to eq([currently_installed_formula.full_name]) + end + end + + context "with core tap" do + let(:tap) { CoreTap.instance } + + include_examples "finds installed formulae in tap" + end + + context "with non-core tap" do + let(:tap) { Tap.fetch("homebrew", "foo") } + + before do + tap.formula_dir.mkpath + end + + include_examples "finds installed formulae in tap" + end + end + + describe "#installed_casks_for", :cask do + shared_examples "finds installed casks in tap" do + def load_cask(token:, with_cask_file: false, mock_install: false) + cask_loader = Cask::CaskLoader::FromContentLoader.new(<<~RUBY, tap:) + cask '#{token}' do + version "1.2.3" + sha256 :no_check + + url 'https://brew.sh/' + end + RUBY + + cask = cask_loader.load(config: nil) + + if with_cask_file + cask_path = tap.cask_dir/"#{token}.rb" + cask_path.parent.mkpath + cask_path.write cask.source + end + + InstallHelper.install_with_caskfile(cask) if mock_install + + cask + end + + let!(:currently_installed_cask) do + load_cask(token: "current_install", with_cask_file: true, mock_install: true) + end + + before do + # Cask that is available from a tap but not installed. + load_cask(token: "no_install", with_cask_file: true) + + # Cask that was installed from a tap but is no longer available from that tap. + load_cask(token: "legacy_install", mock_install: true) + end + + it "returns the expected casks" do + expect(class_instance.installed_casks_for(tap:)).to eq([currently_installed_cask]) + end + end + + context "with core cask tap" do + let(:tap) { CoreCaskTap.instance } + + include_examples "finds installed casks in tap" + end + + context "with non-core cask tap" do + let(:tap) { Tap.fetch("homebrew", "foo") } + + include_examples "finds installed casks in tap" + end + end end diff --git a/Library/Homebrew/test/untap_spec.rb b/Library/Homebrew/test/untap_spec.rb deleted file mode 100644 index e88f3d27f6..0000000000 --- a/Library/Homebrew/test/untap_spec.rb +++ /dev/null @@ -1,126 +0,0 @@ -# frozen_string_literal: true - -require "untap" - -RSpec.describe Homebrew::Untap do - describe ".installed_formulae_for", :integration_test do - shared_examples "finds installed formulae in tap" do - def load_formula(name:, with_formula_file: false, mock_install: false) - formula = if with_formula_file - path = setup_test_formula(name, tap:) - Formulary.factory(path) - else - formula(name, tap:) do - url "https://brew.sh/#{name}-1.0.tgz" - end - end - - if mock_install - keg_path = HOMEBREW_CELLAR/name/"1.2.3" - keg_path.mkpath - - tab_path = keg_path/Tab::FILENAME - tab_path.write <<~JSON - { - "source": { - "tap": "#{tap}" - } - } - JSON - end - - formula - end - - let!(:currently_installed_formula) do - load_formula(name: "current_install", with_formula_file: true, mock_install: true) - end - - before do - # Formula that is available from a tap but not installed. - load_formula(name: "no_install", with_formula_file: true) - - # Formula that was installed from a tap but is no longer available from that tap. - load_formula(name: "legacy_install", mock_install: true) - - tap.clear_cache - end - - it "returns the expected formulae" do - expect(described_class.installed_formulae_for(tap:).map(&:full_name)) - .to eq([currently_installed_formula.full_name]) - end - end - - context "with core tap" do - let(:tap) { CoreTap.instance } - - include_examples "finds installed formulae in tap" - end - - context "with non-core tap" do - let(:tap) { Tap.fetch("homebrew", "foo") } - - before do - tap.formula_dir.mkpath - end - - include_examples "finds installed formulae in tap" - end - end - - describe ".installed_casks_for", :cask do - shared_examples "finds installed casks in tap" do - def load_cask(token:, with_cask_file: false, mock_install: false) - cask_loader = Cask::CaskLoader::FromContentLoader.new(<<~RUBY, tap:) - cask '#{token}' do - version "1.2.3" - sha256 :no_check - - url 'https://brew.sh/' - end - RUBY - - cask = cask_loader.load(config: nil) - - if with_cask_file - cask_path = tap.cask_dir/"#{token}.rb" - cask_path.parent.mkpath - cask_path.write cask.source - end - - InstallHelper.install_with_caskfile(cask) if mock_install - - cask - end - - let!(:currently_installed_cask) do - load_cask(token: "current_install", with_cask_file: true, mock_install: true) - end - - before do - # Cask that is available from a tap but not installed. - load_cask(token: "no_install", with_cask_file: true) - - # Cask that was installed from a tap but is no longer available from that tap. - load_cask(token: "legacy_install", mock_install: true) - end - - it "returns the expected casks" do - expect(described_class.installed_casks_for(tap:)).to eq([currently_installed_cask]) - end - end - - context "with core cask tap" do - let(:tap) { CoreCaskTap.instance } - - include_examples "finds installed casks in tap" - end - - context "with non-core cask tap" do - let(:tap) { Tap.fetch("homebrew", "foo") } - - include_examples "finds installed casks in tap" - end - end -end diff --git a/Library/Homebrew/untap.rb b/Library/Homebrew/untap.rb deleted file mode 100644 index 3a856f2a91..0000000000 --- a/Library/Homebrew/untap.rb +++ /dev/null @@ -1,60 +0,0 @@ -# typed: true -# frozen_string_literal: true - -require "extend/cachable" - -module Homebrew - # Helpers for the `brew untap` command. - # @api private - module Untap - extend Cachable - - # All installed formulae currently available in a tap by formula full name. - sig { params(tap: Tap).returns(T::Array[Formula]) } - def self.installed_formulae_for(tap:) - tap.formula_names.filter_map do |formula_name| - next unless installed_formulae_names.include?(T.must(formula_name.split("/").last)) - - formula = begin - Formulary.factory(formula_name) - rescue FormulaUnavailableError - # Don't blow up because of a single unavailable formula. - next - end - - # Can't use Formula#any_version_installed? because it doesn't consider - # taps correctly. - formula if formula.installed_kegs.any? { |keg| keg.tab.tap == tap } - end - end - - sig { returns(T::Set[String]) } - def self.installed_formulae_names - cache[:installed_formulae_names] ||= Formula.installed_formula_names.to_set.freeze - end - private_class_method :installed_formulae_names - - # All installed casks currently available in a tap by cask full name. - sig { params(tap: Tap).returns(T::Array[Cask::Cask]) } - def self.installed_casks_for(tap:) - tap.cask_tokens.filter_map do |cask_token| - next unless installed_cask_tokens.include?(T.must(cask_token.split("/").last)) - - cask = begin - Cask::CaskLoader.load(cask_token) - rescue Cask::CaskUnavailableError - # Don't blow up because of a single unavailable cask. - next - end - - cask if cask.installed? - end - end - - sig { returns(T::Set[String]) } - def self.installed_cask_tokens - cache[:installed_cask_tokens] ||= Cask::Caskroom.tokens.to_set.freeze - end - private_class_method :installed_cask_tokens - end -end