From f0b8845ad6d19b3de41860841780b916ceb38ca5 Mon Sep 17 00:00:00 2001 From: Bevan Kay Date: Tue, 6 Feb 2024 20:16:03 +0000 Subject: [PATCH] Revert "Handle tap migrations in `CaskLoader`." --- Library/Homebrew/cask/cask_loader.rb | 99 +++---------------- Library/Homebrew/tap_constants.rb | 2 +- .../cask/cask_loader/from_api_loader_spec.rb | 8 +- .../Homebrew/test/cask/cask_loader_spec.rb | 70 ------------- 4 files changed, 23 insertions(+), 156 deletions(-) delete mode 100644 Library/Homebrew/test/cask/cask_loader_spec.rb diff --git a/Library/Homebrew/cask/cask_loader.rb b/Library/Homebrew/cask/cask_loader.rb index 591daa9e47802..9516792de8ebc 100644 --- a/Library/Homebrew/cask/cask_loader.rb +++ b/Library/Homebrew/cask/cask_loader.rb @@ -18,11 +18,6 @@ module ILoader extend T::Helpers interface! - sig { overridable.params(_ref: T.any(String, Pathname, Cask, URI::Generic)).returns(T::Boolean) } - def self.can_load?(_ref) - false - end - sig { abstract.params(config: Config).returns(Cask) } def load(config:); end end @@ -55,11 +50,10 @@ def cask(header_token, **options, &block) # Loads a cask from a string. class FromContentLoader < AbstractContentLoader - sig { params(ref: T.any(String, Pathname, Cask, URI::Generic)).returns(T::Boolean) } def self.can_load?(ref) return false unless ref.respond_to?(:to_str) - content = T.unsafe(ref).to_str + content = ref.to_str # Cache compiled regex @regex ||= begin @@ -72,8 +66,7 @@ def self.can_load?(ref) content.match?(@regex) end - sig { params(content: String, tap: Tap).void } - def initialize(content, tap: T.unsafe(nil)) + def initialize(content, tap: nil) super() @content = content.force_encoding("UTF-8") @@ -89,19 +82,14 @@ def load(config:) # Loads a cask from a path. class FromPathLoader < AbstractContentLoader - sig { params(ref: T.any(String, Pathname, Cask, URI::Generic)).returns(T::Boolean) } def self.can_load?(ref) - ref = Pathname(ref) if ref.is_a?(String) - return false unless ref.is_a?(Pathname) - - path = ref + path = Pathname(ref) %w[.rb .json].include?(path.extname) && path.expand_path.exist? end attr_reader :token, :path - sig { params(path: T.any(Pathname, String), token: String).void } - def initialize(path, token: T.unsafe(nil)) + def initialize(path, token: nil) super() path = Pathname(path).expand_path @@ -146,7 +134,6 @@ def cask(header_token, **options, &block) # Loads a cask from a URI. class FromURILoader < FromPathLoader - sig { params(ref: T.any(String, Pathname, Cask, URI::Generic)).returns(T::Boolean) } def self.can_load?(ref) # Cache compiled regex @uri_regex ||= begin @@ -154,10 +141,9 @@ def self.can_load?(ref) Regexp.new("\\A#{uri_regex.source}\\Z", uri_regex.options) end - uri = ref.to_s - return false unless uri.match?(@uri_regex) + return false unless ref.to_s.match?(@uri_regex) - uri = URI(uri) + uri = URI(ref) return false unless uri.path true @@ -187,12 +173,10 @@ def load(config:) # Loads a cask from a tap path. class FromTapPathLoader < FromPathLoader - sig { params(ref: T.any(String, Pathname, Cask, URI::Generic)).returns(T::Boolean) } def self.can_load?(ref) super && !Tap.from_path(ref).nil? end - sig { params(path: T.any(Pathname, String)).void } def initialize(path) super(path) @tap = Tap.from_path(path) @@ -201,12 +185,10 @@ def initialize(path) # Loads a cask from a specific tap. class FromTapLoader < FromTapPathLoader - sig { params(ref: T.any(String, Pathname, Cask, URI::Generic)).returns(T::Boolean) } def self.can_load?(ref) ref.to_s.match?(HOMEBREW_TAP_CASK_REGEX) end - sig { params(tapped_name: String).void } def initialize(tapped_name) user, repo, token = tapped_name.split("/", 3) tap = Tap.fetch(user, repo) @@ -223,12 +205,10 @@ def load(config:) # Loads a cask from the default tap path. class FromDefaultTapPathLoader < FromTapPathLoader - sig { params(ref: T.any(String, Pathname, Cask, URI::Generic)).returns(T::Boolean) } def self.can_load?(ref) super CaskLoader.default_path(ref) end - sig { params(ref: String).void } def initialize(ref) super CaskLoader.default_path(ref) end @@ -237,13 +217,10 @@ def initialize(ref) # Loads a cask from an existing {Cask} instance. class FromInstanceLoader include ILoader - - sig { params(ref: T.any(String, Pathname, Cask, URI::Generic)).returns(T::Boolean) } def self.can_load?(ref) ref.is_a?(Cask) end - sig { params(cask: Cask).void } def initialize(cask) @cask = cask end @@ -256,32 +233,25 @@ def load(config:) # Loads a cask from the JSON API. class FromAPILoader include ILoader - attr_reader :token, :path - sig { returns(T.nilable(Hash)) } - attr_reader :from_json - - sig { params(ref: T.any(String, Pathname, Cask, URI::Generic)).returns(T::Boolean) } def self.can_load?(ref) return false if Homebrew::EnvConfig.no_install_from_api? return false unless ref.is_a?(String) + return false unless ref.match?(HOMEBREW_MAIN_TAP_CASK_REGEX) - return false unless (match = ref.match(HOMEBREW_MAIN_TAP_CASK_REGEX)) - - token = match[:token] + token = ref.sub(%r{^homebrew/(?:homebrew-)?cask/}i, "") Homebrew::API::Cask.all_casks.key?(token) end - sig { params(token: String, from_json: Hash).void } - def initialize(token, from_json: T.unsafe(nil)) + def initialize(token, from_json: nil) @token = token.sub(%r{^homebrew/(?:homebrew-)?cask/}i, "") @path = CaskLoader.default_path(@token) @from_json = from_json end def load(config:) - json_cask = from_json || Homebrew::API::Cask.all_casks[token] + json_cask = @from_json || Homebrew::API::Cask.all_casks[token] cask_options = { loaded_from_api: true, @@ -431,8 +401,7 @@ def from_h_gsubs(value, appdir) # Pseudo-loader which raises an error when trying to load the corresponding cask. class NullLoader < FromPathLoader - sig { params(_ref: T.any(String, Pathname, Cask, URI::Generic)).returns(T::Boolean) } - def self.can_load?(_ref) + def self.can_load?(*) true end @@ -455,33 +424,6 @@ def self.load(ref, config: nil, warn: true) self.for(ref, warn: warn).load(config: config) end - def self.tap_cask_token_type(tapped_token, warn:) - user, repo, token = tapped_token.split("/", 3).map(&:downcase) - tap = Tap.fetch(user, repo) - type = nil - - if (new_token = tap.cask_renames[token].presence) - old_token = token - token = new_token - new_token = tap.core_cask_tap? ? token : "#{tap}/#{token}" - type = :rename - elsif (new_tap_name = tap.tap_migrations[token].presence) - new_tap_user, new_tap_repo, = new_tap_name.split("/") - new_tap_name = "#{new_tap_user}/#{new_tap_repo}" - new_tap = Tap.fetch(new_tap_name) - new_tap.ensure_installed! - new_tapped_token = "#{new_tap_name}/#{token}" - token, tap, = tap_cask_token_type(new_tapped_token, warn: false) - old_token = tapped_token - new_token = new_tap.core_cask_tap? ? token : new_tapped_token - type = :migration - end - - opoo "Cask #{old_token} was renamed to #{new_token}." if warn && old_token && new_token - - [token, tap, type] - end - def self.for(ref, need_path: false, warn: true) [ FromInstanceLoader, @@ -493,18 +435,10 @@ def self.for(ref, need_path: false, warn: true) FromPathLoader, FromDefaultTapPathLoader, ].each do |loader_class| - next unless loader_class.can_load?(ref) - - $stderr.puts "#{$PROGRAM_NAME} (#{loader_class}): loading #{ref}" if debug? - - if [FromAPILoader, FromTapLoader].include?(loader_class) - ref = "#{CoreCaskTap.instance}/#{ref}" if CoreCaskTap.instance.cask_renames.key?(ref) - token, tap, = tap_cask_token_type(ref, warn: warn) - loader_class = T.cast(loader_class, T.any(T.class_of(FromAPILoader), T.class_of(FromTapLoader))) - return loader_class.new("#{tap}/#{token}") + if loader_class.can_load?(ref) + $stderr.puts "#{$PROGRAM_NAME} (#{loader_class}): loading #{ref}" if debug? + return loader_class.new(ref) end - - return loader_class.new(ref) end case (possible_tap_casks = tap_paths(ref, warn: warn)).count @@ -512,13 +446,12 @@ def self.for(ref, need_path: false, warn: true) return FromTapPathLoader.new(possible_tap_casks.first) when 2..Float::INFINITY loaders = possible_tap_casks.map(&FromTapPathLoader.method(:new)) + raise TapCaskAmbiguityError.new(ref, loaders) end possible_installed_cask = Cask.new(ref) - if (installed_caskfile = possible_installed_cask.installed_caskfile) - return FromPathLoader.new(installed_caskfile) - end + return FromPathLoader.new(possible_installed_cask.installed_caskfile) if possible_installed_cask.installed? NullLoader.new(ref) end diff --git a/Library/Homebrew/tap_constants.rb b/Library/Homebrew/tap_constants.rb index a9fc1bdd9bcdb..2df0d9a759831 100644 --- a/Library/Homebrew/tap_constants.rb +++ b/Library/Homebrew/tap_constants.rb @@ -6,7 +6,7 @@ # Match taps' casks, e.g. `someuser/sometap/somecask` HOMEBREW_TAP_CASK_REGEX = T.let(%r{^([\w-]+)/([\w-]+)/([a-z0-9\-_]+)$}, Regexp) # Match main cask taps' casks, e.g. `homebrew/cask/somecask` or `somecask` -HOMEBREW_MAIN_TAP_CASK_REGEX = T.let(%r{^(?[Hh]omebrew/(?:homebrew-)?cask/)?(?[a-z0-9\-_]+)$}, Regexp) +HOMEBREW_MAIN_TAP_CASK_REGEX = T.let(%r{^([Hh]omebrew/(?:homebrew-)?cask/)?[a-z0-9\-_]+$}, Regexp) # Match taps' directory paths, e.g. `HOMEBREW_LIBRARY/Taps/someuser/sometap` HOMEBREW_TAP_DIR_REGEX = T.let( %r{#{Regexp.escape(HOMEBREW_LIBRARY.to_s)}/Taps/(?[\w-]+)/(?[\w-]+)}, Regexp diff --git a/Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb b/Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb index deccbb9780f69..90b6d91d56e47 100644 --- a/Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb +++ b/Library/Homebrew/test/cask/cask_loader/from_api_loader_spec.rb @@ -26,7 +26,9 @@ context "when not using the API" do before do - ENV["HOMEBREW_NO_INSTALL_FROM_API"] = "1" + allow(Homebrew::EnvConfig) + .to receive(:no_install_from_api?) + .and_return(true) end it "returns false" do @@ -36,7 +38,9 @@ context "when using the API" do before do - ENV.delete("HOMEBREW_NO_INSTALL_FROM_API") + allow(Homebrew::EnvConfig) + .to receive(:no_install_from_api?) + .and_return(false) end it "returns true for valid token" do diff --git a/Library/Homebrew/test/cask/cask_loader_spec.rb b/Library/Homebrew/test/cask/cask_loader_spec.rb deleted file mode 100644 index b9c83cbb450b3..0000000000000 --- a/Library/Homebrew/test/cask/cask_loader_spec.rb +++ /dev/null @@ -1,70 +0,0 @@ -# frozen_string_literal: true - -describe Cask::CaskLoader, :cask do - describe "::for" do - let(:tap) { CoreCaskTap.instance } - - context "when a cask is renamed" do - let(:old_token) { "version-newest" } - let(:new_token) { "version-latest" } - - let(:api_casks) do - [old_token, new_token].to_h do |token| - hash = described_class.load(new_token).to_hash_with_variations - json = JSON.pretty_generate(hash) - cask_json = JSON.parse(json) - - [token, cask_json.except("token")] - end - end - let(:cask_renames) do - { old_token => new_token } - end - - before do - allow(Homebrew::API::Cask) - .to receive(:all_casks) - .and_return(api_casks) - - allow(tap).to receive(:cask_renames) - .and_return(cask_renames) - end - - context "when not using the API" do - before do - ENV["HOMEBREW_NO_INSTALL_FROM_API"] = "1" - end - - it "warns when using the short token" do - expect do - expect(described_class.for("version-newest")).to be_a Cask::CaskLoader::FromTapPathLoader - end.to output(/version-newest was renamed to version-latest/).to_stderr - end - - it "warns when using the full token" do - expect do - expect(described_class.for("homebrew/cask/version-newest")).to be_a Cask::CaskLoader::FromTapPathLoader - end.to output(/version-newest was renamed to version-latest/).to_stderr - end - end - - context "when using the API" do - before do - ENV.delete("HOMEBREW_NO_INSTALL_FROM_API") - end - - it "warns when using the short token" do - expect do - expect(described_class.for("version-newest")).to be_a Cask::CaskLoader::FromAPILoader - end.to output(/version-newest was renamed to version-latest/).to_stderr - end - - it "warns when using the full token" do - expect do - expect(described_class.for("homebrew/cask/version-newest")).to be_a Cask::CaskLoader::FromAPILoader - end.to output(/version-newest was renamed to version-latest/).to_stderr - end - end - end - end -end