From d17aa66759fb6a3e803c468cacc1efb0433d605e Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Fri, 9 Feb 2024 18:27:58 +0100 Subject: [PATCH 1/6] Avoid `T.cast`. --- Library/Homebrew/cask/cask_loader.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Library/Homebrew/cask/cask_loader.rb b/Library/Homebrew/cask/cask_loader.rb index 8f26061834..eb62f79711 100644 --- a/Library/Homebrew/cask/cask_loader.rb +++ b/Library/Homebrew/cask/cask_loader.rb @@ -232,10 +232,7 @@ module Cask } def self.try_new(ref, warn: false) ref = ref.to_s - - return unless (match = ref.match(HOMEBREW_DEFAULT_TAP_CASK_REGEX)) - - token = match[:token] + return unless (token = ref[HOMEBREW_DEFAULT_TAP_CASK_REGEX, :token]) ref = "#{CoreCaskTap.instance}/#{token}" @@ -298,7 +295,10 @@ module Cask return if Homebrew::EnvConfig.no_install_from_api? return unless ref.is_a?(String) return unless (token = ref[HOMEBREW_DEFAULT_TAP_CASK_REGEX, :token]) - return if !Homebrew::API::Cask.all_casks.key?(token) && !Homebrew::API::Cask.all_renames.key?(token) + if !Homebrew::API::Cask.all_casks.key?(token) && + !Homebrew::API::Cask.all_renames.key?(token) + return + end ref = "#{CoreCaskTap.instance}/#{token}" From 6f28392d6e3374a9c21c00340b6a5ad1b34df5c0 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Fri, 9 Feb 2024 18:26:54 +0100 Subject: [PATCH 2/6] Remove `FromDefaultTapPathLoader` and `FromDefaultTapLoader`. --- Library/Homebrew/cask/cask_loader.rb | 65 +++++++++++++--------------- Library/Homebrew/cask/exceptions.rb | 10 +++-- 2 files changed, 36 insertions(+), 39 deletions(-) diff --git a/Library/Homebrew/cask/cask_loader.rb b/Library/Homebrew/cask/cask_loader.rb index eb62f79711..fdc2d158d9 100644 --- a/Library/Homebrew/cask/cask_loader.rb +++ b/Library/Homebrew/cask/cask_loader.rb @@ -196,6 +196,9 @@ module Cask # Loads a cask from a specific tap. class FromTapLoader < FromPathLoader + sig { returns(Tap) } + attr_reader :tap + sig { params(ref: T.any(String, Pathname, Cask, URI::Generic), warn: T::Boolean) .returns(T.nilable(T.attached_class)) @@ -224,34 +227,6 @@ module Cask end end - # Load a cask from the default tap, using either a full or short token. - class FromDefaultTapLoader < FromTapLoader - sig { - params(ref: T.any(String, Pathname, Cask, URI::Generic), warn: T::Boolean) - .returns(T.nilable(T.attached_class)) - } - def self.try_new(ref, warn: false) - ref = ref.to_s - return unless (token = ref[HOMEBREW_DEFAULT_TAP_CASK_REGEX, :token]) - - ref = "#{CoreCaskTap.instance}/#{token}" - - token, tap, = CaskLoader.tap_cask_token_type(ref, warn: warn) - new("#{tap}/#{token}") - end - end - - # Loads a cask from the default tap path. - class FromDefaultTapPathLoader < FromPathLoader - sig { - params(ref: T.any(String, Pathname, Cask, URI::Generic), warn: T::Boolean) - .returns(T.nilable(T.attached_class)) - } - def self.try_new(ref, warn: false) - super CaskLoader.default_path(ref) - end - end - # Loads a cask from an existing {Cask} instance. class FromInstanceLoader include ILoader @@ -464,14 +439,34 @@ module Cask # Loader which tries loading casks from tap paths, failing # if the same token exists in multiple taps. - class FromAmbiguousTapPathLoader < FromPathLoader + class FromNameLoader < FromPathLoader def self.try_new(ref, warn: false) - case (possible_tap_casks = CaskLoader.tap_paths(ref)).count + return unless ref.is_a?(String) + return if ref.include?("/") + + token = ref + + loaders = Tap.map { |tap| FromTapLoader.try_new("#{tap}/#{token}", warn: warn) } + .compact + .select { _1.path.exist? } + + case loaders.count when 1 - new(possible_tap_casks.first) + loaders.first when 2..Float::INFINITY - loaders = possible_tap_casks.map(&FromPathLoader.method(:new)) - raise TapCaskAmbiguityError.new(ref, loaders) + default_tap_loaders, other_loaders = *loaders.partition { _1.tap.core_cask_tap? } + default_tap_loader = default_tap_loaders.first if default_tap_loaders.count + + # Put default tap last so that the error message always recommends + # using the fully-qualified name for non-default taps. + taps = other_loaders.map(&:tap) + default_tap_loaders.map(&:tap) + + error = TapCaskAmbiguityError.new(token, taps) + + raise error unless default_tap_loader + + opoo error if warn + default_tap_loader end end end @@ -557,10 +552,8 @@ module Cask FromURILoader, FromAPILoader, FromTapLoader, + FromNameLoader, FromPathLoader, - FromDefaultTapPathLoader, - FromAmbiguousTapPathLoader, - FromDefaultTapLoader, FromInstalledPathLoader, NullLoader, ].each do |loader_class| diff --git a/Library/Homebrew/cask/exceptions.rb b/Library/Homebrew/cask/exceptions.rb index acbc0fa338..52091358f2 100644 --- a/Library/Homebrew/cask/exceptions.rb +++ b/Library/Homebrew/cask/exceptions.rb @@ -131,10 +131,14 @@ module Cask # # @api private class TapCaskAmbiguityError < CaskError - def initialize(ref, loaders) + def initialize(token, taps) + casks = taps.map { |tap| "#{tap}/#{token}" } + cask_list = casks.sort.map { |f| "\n * #{f}" }.join + super <<~EOS - Cask #{ref} exists in multiple taps: - #{loaders.map { |loader| " #{loader.tap}/#{loader.token}" }.join("\n")} + Cask #{token} exists in multiple taps:#{cask_list} + + Please use the fully-qualified name (e.g. #{casks.first}) to refer to a specific Cask. EOS end end From 0211feebd7f6d6f648f5a71ee2134b4d7fb95222 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Fri, 9 Feb 2024 19:28:45 +0100 Subject: [PATCH 3/6] Fix loading test fixtures. --- Library/Homebrew/cask/cask_loader.rb | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Library/Homebrew/cask/cask_loader.rb b/Library/Homebrew/cask/cask_loader.rb index fdc2d158d9..2f2be8c66d 100644 --- a/Library/Homebrew/cask/cask_loader.rb +++ b/Library/Homebrew/cask/cask_loader.rb @@ -440,6 +440,10 @@ module Cask # Loader which tries loading casks from tap paths, failing # if the same token exists in multiple taps. class FromNameLoader < FromPathLoader + sig { + params(ref: T.any(String, Pathname, Cask, URI::Generic), warn: T::Boolean) + .returns(T.nilable(T.attached_class)) + } def self.try_new(ref, warn: false) return unless ref.is_a?(String) return if ref.include?("/") @@ -473,7 +477,13 @@ module Cask # Loader which loads a cask from the installed cask file. class FromInstalledPathLoader < FromPathLoader + sig { + params(ref: T.any(String, Pathname, Cask, URI::Generic), warn: T::Boolean) + .returns(T.nilable(T.attached_class)) + } def self.try_new(ref, warn: false) + return unless ref.is_a?(String) + possible_installed_cask = Cask.new(ref) return unless (installed_caskfile = possible_installed_cask.installed_caskfile) From 8e04ab8b424c344c576953a2d65b366c9e9932fc Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Fri, 9 Feb 2024 20:33:19 +0100 Subject: [PATCH 4/6] Fix type signature. --- Library/Homebrew/cask/cask_loader.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Library/Homebrew/cask/cask_loader.rb b/Library/Homebrew/cask/cask_loader.rb index 2f2be8c66d..8f4df90ec3 100644 --- a/Library/Homebrew/cask/cask_loader.rb +++ b/Library/Homebrew/cask/cask_loader.rb @@ -439,10 +439,10 @@ module Cask # Loader which tries loading casks from tap paths, failing # if the same token exists in multiple taps. - class FromNameLoader < FromPathLoader + module FromNameLoader sig { params(ref: T.any(String, Pathname, Cask, URI::Generic), warn: T::Boolean) - .returns(T.nilable(T.attached_class)) + .returns(T.nilable(FromTapLoader)) } def self.try_new(ref, warn: false) return unless ref.is_a?(String) From 48c9897081880b2e3c8abc2a5fcdb8da988c6e44 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Mon, 12 Feb 2024 07:43:29 +0100 Subject: [PATCH 5/6] Convert `FromNameLoader` to class. --- Library/Homebrew/cask/cask_loader.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Library/Homebrew/cask/cask_loader.rb b/Library/Homebrew/cask/cask_loader.rb index 8f4df90ec3..564544017d 100644 --- a/Library/Homebrew/cask/cask_loader.rb +++ b/Library/Homebrew/cask/cask_loader.rb @@ -439,10 +439,10 @@ module Cask # Loader which tries loading casks from tap paths, failing # if the same token exists in multiple taps. - module FromNameLoader + class FromNameLoader < FromTapLoader sig { params(ref: T.any(String, Pathname, Cask, URI::Generic), warn: T::Boolean) - .returns(T.nilable(FromTapLoader)) + .returns(T.nilable(T.attached_class)) } def self.try_new(ref, warn: false) return unless ref.is_a?(String) @@ -450,7 +450,7 @@ module Cask token = ref - loaders = Tap.map { |tap| FromTapLoader.try_new("#{tap}/#{token}", warn: warn) } + loaders = Tap.map { |tap| super("#{tap}/#{token}", warn: warn) } .compact .select { _1.path.exist? } From c39abef045976db7ec2680788fe603f62f69d77d Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Mon, 12 Feb 2024 21:45:07 +0100 Subject: [PATCH 6/6] Always prefer default tap. --- Library/Homebrew/cask/cask_loader.rb | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/Library/Homebrew/cask/cask_loader.rb b/Library/Homebrew/cask/cask_loader.rb index 564544017d..76eae2ee52 100644 --- a/Library/Homebrew/cask/cask_loader.rb +++ b/Library/Homebrew/cask/cask_loader.rb @@ -458,19 +458,12 @@ module Cask when 1 loaders.first when 2..Float::INFINITY - default_tap_loaders, other_loaders = *loaders.partition { _1.tap.core_cask_tap? } - default_tap_loader = default_tap_loaders.first if default_tap_loaders.count + # Always prefer the default tap, i.e. behave the same as if loading from the API. + if (default_tap_loader = loaders.find { _1.tap.core_cask_tap? }) + return default_tap_loader + end - # Put default tap last so that the error message always recommends - # using the fully-qualified name for non-default taps. - taps = other_loaders.map(&:tap) + default_tap_loaders.map(&:tap) - - error = TapCaskAmbiguityError.new(token, taps) - - raise error unless default_tap_loader - - opoo error if warn - default_tap_loader + raise TapCaskAmbiguityError.new(token, loaders.map(&:tap)) end end end