Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate installing casks/formulae from paths. #18409

Merged
merged 1 commit into from
Sep 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"))

Check warning on line 215 in Library/Homebrew/brew.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/brew.rb#L215

Added line #L215 was not covered by tests
$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)}"

Check warning on line 219 in Library/Homebrew/brew.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/brew.rb#L219

Added line #L219 was not covered by tests
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?
MikeMcQuaid marked this conversation as resolved.
Show resolved Hide resolved

# 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
16 changes: 13 additions & 3 deletions Library/Homebrew/formulary.rb
Original file line number Diff line number Diff line change
Expand Up @@ -646,9 +646,6 @@ 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/")

options = if (tap = Tap.from_path(path))
# Only treat symlinks in taps as aliases.
if path.symlink?
Expand All @@ -675,6 +672,17 @@ def self.try_new(ref, from: T.unsafe(nil), warn: false)

return if path.extname != ".rb"

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

new(path, **options)
end

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
17 changes: 6 additions & 11 deletions Library/Homebrew/test/formulary_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,11 +129,6 @@ class Wrong#{described_class.class_s(formula_name)} < Formula
end.to raise_error(FormulaUnavailableError)
end

it "returns a Formula when given a URL", :needs_utils_curl, :no_api do
formula = described_class.factory("file://#{formula_path}")
expect(formula).to be_a(Formula)
end

it "errors when given a URL but paths are disabled" do
ENV["HOMEBREW_FORBID_PACKAGES_FROM_PATHS"] = "1"
expect do
Expand Down Expand Up @@ -546,31 +541,31 @@ def formula_json_contents(extra_items = {})
it "raises an error when given an https URL" do
expect do
described_class.factory("https://brew.sh/foo.rb")
end.to raise_error(UnsupportedInstallationMethod)
end.to raise_error(MethodDeprecatedError)
end

it "raises an error when given a bottle URL" do
expect do
described_class.factory("https://brew.sh/foo-1.0.arm64_catalina.bottle.tar.gz")
end.to raise_error(UnsupportedInstallationMethod)
end.to raise_error(MethodDeprecatedError)
end

it "raises an error when given an ftp URL" do
expect do
described_class.factory("ftp://brew.sh/foo.rb")
end.to raise_error(UnsupportedInstallationMethod)
end.to raise_error(MethodDeprecatedError)
end

it "raises an error when given an sftp URL" do
expect do
described_class.factory("sftp://brew.sh/foo.rb")
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
expect do
described_class.factory("file://#{TEST_FIXTURE_DIR}/testball.rb")
end.not_to raise_error(UnsupportedInstallationMethod)
end.to raise_error(MethodDeprecatedError)
end
end

Expand Down