Skip to content

Commit

Permalink
Deprecate installing casks/formulae from paths.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
MikeMcQuaid committed Sep 26, 2024
1 parent 05cde76 commit 7c62015
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 18 deletions.
9 changes: 7 additions & 2 deletions Library/Homebrew/brew.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 13 additions & 2 deletions Library/Homebrew/cask/cask_loader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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! " \
Expand Down
14 changes: 12 additions & 2 deletions Library/Homebrew/formulary.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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

Expand Down
10 changes: 5 additions & 5 deletions Library/Homebrew/test/cask/cask_loader/from_uri_loader_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 1 addition & 7 deletions Library/Homebrew/test/cask/cask_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 7c62015

Please sign in to comment.