From 7c6201524ee5aa129c33d24faadac520c696f9e4 Mon Sep 17 00:00:00 2001 From: Mike McQuaid Date: Wed, 25 Sep 2024 10:05:12 +0100 Subject: [PATCH] Deprecate installing casks/formulae from paths. We've already disabled installing casks/formulae from URLs and we regularly tell people not to install from paths so let's just deprecate this behaviour entirely. Even Homebrew developers do not need to work this way. --- Library/Homebrew/brew.rb | 9 +++++++-- Library/Homebrew/cask/cask_loader.rb | 15 +++++++++++++-- Library/Homebrew/formulary.rb | 14 ++++++++++++-- .../test/cask/cask_loader/from_uri_loader_spec.rb | 10 +++++----- Library/Homebrew/test/cask/cask_spec.rb | 8 +------- 5 files changed, 38 insertions(+), 18 deletions(-) diff --git a/Library/Homebrew/brew.rb b/Library/Homebrew/brew.rb index 6898eff1f13e0..59cb5e11a0103 100644 --- a/Library/Homebrew/brew.rb +++ b/Library/Homebrew/brew.rb @@ -211,8 +211,13 @@ $stderr.puts "If reporting this issue please do so at (not Homebrew/brew or Homebrew/homebrew-core):" $stderr.puts " #{Formatter.url(issues_url)}" elsif internal_cmd - $stderr.puts "#{Tty.bold}Please report this issue:#{Tty.reset}" - $stderr.puts " #{Formatter.url(OS::ISSUES_URL)}" + if e.is_a?(MethodDeprecatedError) && (first_backtrace = e.backtrace&.first.presence) && + (first_backtrace.include?("/formulary.rb") || first_backtrace.include?("/cask_loader.rb")) + $stderr.puts Utils::Backtrace.clean(e) if ARGV.include?("--debug") || args.debug? + else + $stderr.puts "#{Tty.bold}Please report this issue:#{Tty.reset}" + $stderr.puts " #{Formatter.url(OS::ISSUES_URL)}" + end end exit 1 diff --git a/Library/Homebrew/cask/cask_loader.rb b/Library/Homebrew/cask/cask_loader.rb index a91882a6e5049..af0949f853e98 100644 --- a/Library/Homebrew/cask/cask_loader.rb +++ b/Library/Homebrew/cask/cask_loader.rb @@ -105,10 +105,19 @@ def self.try_new(ref, warn: false) end return if %w[.rb .json].exclude?(path.extname) + return if path.to_s.include?("/Formula/") return unless path.expand_path.exist? - return if Homebrew::EnvConfig.forbid_packages_from_paths? && - !path.realpath.to_s.start_with?("#{Caskroom.path}/", "#{HOMEBREW_LIBRARY}/Taps/") + unless path.realpath.to_s.start_with?("#{Caskroom.path}/", "#{HOMEBREW_LIBRARY}/Taps/", + "#{HOMEBREW_LIBRARY_PATH}/test/support/fixtures/") + return if Homebrew::EnvConfig.forbid_packages_from_paths? + + # So many tests legimately use casks from paths that we can't warn about them all. + # Let's focus on end-users for now. + unless ENV["HOMEBREW_TESTS"] + odeprecated "installing formulae from paths or URLs", "installing formulae from taps" + end + end new(path) end @@ -195,6 +204,8 @@ def initialize(url) def load(config:) path.dirname.mkpath + odeprecated "installing casks from paths or URLs", "installing casks from taps" + if ALLOWED_URL_SCHEMES.exclude?(url.scheme) raise UnsupportedInstallationMethod, "Non-checksummed download of #{name} formula file from an arbitrary URL is unsupported! " \ diff --git a/Library/Homebrew/formulary.rb b/Library/Homebrew/formulary.rb index 67ae6b78ad7c7..7c56c57cb2433 100644 --- a/Library/Homebrew/formulary.rb +++ b/Library/Homebrew/formulary.rb @@ -646,8 +646,16 @@ def self.try_new(ref, from: T.unsafe(nil), warn: false) return unless path.expand_path.exist? - return if Homebrew::EnvConfig.forbid_packages_from_paths? && - !path.realpath.to_s.start_with?("#{HOMEBREW_CELLAR}/", "#{HOMEBREW_LIBRARY}/Taps/") + unless path.realpath.to_s.start_with?("#{HOMEBREW_CELLAR}/", "#{HOMEBREW_TAP_DIRECTORY}/", + "#{HOMEBREW_LIBRARY_PATH}/test/support/fixtures/") + return if Homebrew::EnvConfig.forbid_packages_from_paths? + + # So many tests legimately use formulae from paths that we can't warn about them all. + # Let's focus on end-users for now. + unless ENV["HOMEBREW_TESTS"] + odeprecated "installing formulae from paths or URLs", "installing formulae from taps" + end + end options = if (tap = Tap.from_path(path)) # Only treat symlinks in taps as aliases. @@ -716,6 +724,8 @@ def self.try_new(ref, from: T.unsafe(nil), warn: false) return unless uri.path return unless uri.scheme.present? + odeprecated "installing formulae from paths or URLs", "installing formulae from taps" + new(uri, from:) end diff --git a/Library/Homebrew/test/cask/cask_loader/from_uri_loader_spec.rb b/Library/Homebrew/test/cask/cask_loader/from_uri_loader_spec.rb index d1777c9f6dcaa..60db335071828 100644 --- a/Library/Homebrew/test/cask/cask_loader/from_uri_loader_spec.rb +++ b/Library/Homebrew/test/cask/cask_loader/from_uri_loader_spec.rb @@ -29,28 +29,28 @@ loader = described_class.new("https://brew.sh/foo.rb") expect do loader.load(config: nil) - end.to raise_error(UnsupportedInstallationMethod) + end.to raise_error(MethodDeprecatedError) end it "raises an error when given an ftp URL" do loader = described_class.new("ftp://brew.sh/foo.rb") expect do loader.load(config: nil) - end.to raise_error(UnsupportedInstallationMethod) + end.to raise_error(MethodDeprecatedError) end it "raises an error when given an sftp URL" do loader = described_class.new("sftp://brew.sh/foo.rb") expect do loader.load(config: nil) - end.to raise_error(UnsupportedInstallationMethod) + end.to raise_error(MethodDeprecatedError) end - it "does not raise an error when given a file URL" do + it "raises an error when given a file URL" do loader = described_class.new("file://#{TEST_FIXTURE_DIR}/cask/Casks/local-caffeine.rb") expect do loader.load(config: nil) - end.not_to raise_error(UnsupportedInstallationMethod) + end.to raise_error(MethodDeprecatedError) end end end diff --git a/Library/Homebrew/test/cask/cask_spec.rb b/Library/Homebrew/test/cask/cask_spec.rb index 038316a682715..ccb0f23da273e 100644 --- a/Library/Homebrew/test/cask/cask_spec.rb +++ b/Library/Homebrew/test/cask/cask_spec.rb @@ -46,16 +46,10 @@ expect(c.token).to eq("caffeine") end - it "returns an instance of the Cask from a URL", :needs_utils_curl, :no_api do - c = Cask::CaskLoader.load("file://#{tap_path}/Casks/local-caffeine.rb") - expect(c).to be_a(described_class) - expect(c.token).to eq("local-caffeine") - end - it "raises an error when failing to download a Cask from a URL", :needs_utils_curl, :no_api do expect do Cask::CaskLoader.load("file://#{tap_path}/Casks/notacask.rb") - end.to raise_error(Cask::CaskUnavailableError) + end.to raise_error(MethodDeprecatedError) end it "returns an instance of the Cask from a relative file location" do