diff --git a/Library/Homebrew/debrew.rb b/Library/Homebrew/debrew.rb index 7d97389fcc..e151272358 100644 --- a/Library/Homebrew/debrew.rb +++ b/Library/Homebrew/debrew.rb @@ -93,24 +93,14 @@ module Debrew sig { type_parameters(:U) - .params(_block: T.proc.returns(T.type_parameter(:U))) - .returns(T.nilable(T.type_parameter(:U))) + .params(block: T.proc.returns(T.type_parameter(:U))) + .returns(T.type_parameter(:U)) } - def self.debrew(&_block) + def self.debrew(&block) @mutex = Mutex.new - Ignorable.hook_raise - - begin - yield - rescue SystemExit - raise - rescue Ignorable::ExceptionMixin => e - e.ignore if debug(e) == :ignore # execution jumps back to where the exception was thrown - nil - ensure - Ignorable.unhook_raise - @mutex = nil - end + Ignorable.hook_raise(on_ignorable: ->(e) { e.is_a?(SystemExit) ? :raise : debug(e) }, &block) + ensure + @mutex = nil end sig { params(exception: Exception).returns(Symbol) } diff --git a/Library/Homebrew/formulary.rb b/Library/Homebrew/formulary.rb index 0e9d279c3b..7f92c0a2d4 100644 --- a/Library/Homebrew/formulary.rb +++ b/Library/Homebrew/formulary.rb @@ -126,7 +126,6 @@ module Formulary Homebrew::Trust.require_trusted_formula!(name, path) require "formula" - require "ignorable" require "stringio" # Capture stdout to prevent formulae from printing to stdout unexpectedly. @@ -144,16 +143,20 @@ module Formulary mod.const_set(:BUILD_FLAGS, flags) mod.module_eval(contents, path.to_s) rescue NameError, ArgumentError, ScriptError, MethodDeprecatedError, MacOSVersion::Error => e - if e.is_a?(Ignorable::ExceptionMixin) - e.ignore - else - remove_const(namespace) - raise FormulaUnreadableError.new(name, e) - end + remove_const(namespace) + raise FormulaUnreadableError.new(name, e) end ENV.clear_sensitive_environment_for_eval! do if ignore_errors - Ignorable.hook_raise(&eval_formula) + require "ignorable" + + on_ignorable = lambda do |e| + case e + when NameError, ArgumentError, MethodDeprecatedError, MacOSVersion::Error then :ignore + else :raise + end + end + Ignorable.hook_raise(on_ignorable:, &eval_formula) else eval_formula.call end diff --git a/Library/Homebrew/ignorable.rb b/Library/Homebrew/ignorable.rb index 6cd7ba6fd5..6d3444c8b7 100644 --- a/Library/Homebrew/ignorable.rb +++ b/Library/Homebrew/ignorable.rb @@ -1,62 +1,63 @@ # typed: strict # frozen_string_literal: true -deprecated_warnings = Warning[:deprecated] -begin - Warning[:deprecated] = false - require "continuation" -ensure - Warning[:deprecated] = deprecated_warnings -end - # Provides the ability to optionally ignore errors raised and continue execution. module Ignorable - # Marks exceptions which can be ignored and provides - # the ability to jump back to where it was raised. - module ExceptionMixin - sig { returns(T.untyped) } - attr_accessor :continuation + # Marks exceptions which can be ignored and resumed from where they were raised. + module ExceptionMixin; end - sig { void } - def ignore - continuation.call - end - end + # Runs the block in a Fiber whose `raise` pauses at the raise site and passes + # the exception to `on_ignorable`. If it returns `:ignore`, execution resumes + # after the raise site, otherwise the exception is raised there as usual. + sig { + type_parameters(:U) + .params( + on_ignorable: T.proc.params(exception: Exception).returns(Symbol), + block: T.proc.returns(T.type_parameter(:U)), + ) + .returns(T.type_parameter(:U)) + } + def self.hook_raise(on_ignorable:, &block) + fiber = Fiber.new(&block) - sig { params(blk: T.nilable(T.proc.void)).void } - def self.hook_raise(&blk) Object.class_eval do - alias_method :original_raise, :raise - # `define_method` keeps Sorbet happy inside this `class_eval` block. - define_method(:raise) do |*args| - callcc do |continuation| - super(*args) - # Handle all possible exceptions. - rescue Exception => e # rubocop:disable Lint/RescueException - unless e.is_a?(ScriptError) - e.extend(ExceptionMixin) - T.cast(e, ExceptionMixin).continuation = continuation - end + define_method(:raise) do |*args, **kwargs| + super(*args, **kwargs) + # All possible exceptions must be pausable, not just `StandardError`. + rescue Exception => e # rubocop:disable Lint/RescueException + if e.is_a?(ScriptError) || Fiber.current != fiber super(e) + else + e.extend(ExceptionMixin) + super(e) if Fiber.yield(e) != :ignore end end alias_method :fail, :raise end - return unless block_given? + result = fiber.resume + while fiber.alive? + decision = begin + on_ignorable.call(result) + # Even `Interrupt` at the prompt must unwind the fiber, not abandon it. + rescue Exception => e # rubocop:disable Lint/RescueException + e + end - yield - unhook_raise - end - - sig { void } - def self.unhook_raise + result = case decision + when :ignore then fiber.resume(:ignore) + # Raise inside the fiber so its `ensure` blocks and rescues still run. + when Exception then fiber.raise(decision) + else fiber.resume(:raise) + end + end + result + ensure Object.class_eval do - alias_method :raise, :original_raise - alias_method :fail, :original_raise - undef_method :original_raise + remove_method(:raise) + remove_method(:fail) end end end diff --git a/Library/Homebrew/sorbet/rbi/shims/fiber.rbi b/Library/Homebrew/sorbet/rbi/shims/fiber.rbi new file mode 100644 index 0000000000..6dd9d548c7 --- /dev/null +++ b/Library/Homebrew/sorbet/rbi/shims/fiber.rbi @@ -0,0 +1,8 @@ +# typed: strict + +# Sorbet's core RBI for `Fiber` is missing `initialize` so does not know +# `Fiber.new` takes a block. +class Fiber + sig { params(blk: T.proc.returns(T.untyped)).void } + def initialize(&blk); end +end diff --git a/Library/Homebrew/test/formulary_spec.rb b/Library/Homebrew/test/formulary_spec.rb index c6a07df689..8e70222ecc 100644 --- a/Library/Homebrew/test/formulary_spec.rb +++ b/Library/Homebrew/test/formulary_spec.rb @@ -56,6 +56,42 @@ RSpec.describe Formulary do end describe "::load_formula" do + it "continues evaluation after ignorable errors with ignore_errors" do + formula_class = described_class.load_formula( + "ignorable-error", + mktmpdir/"ignorable-error.rb", + <<~RUBY, + class IgnorableError < Formula + raise ArgumentError, "should be ignored" + url "https://brew.sh/ignorable-error-1.0.tar.gz" + end + RUBY + "IgnorableErrorNamespace", + flags: [], + ignore_errors: true, + ) + + expect(formula_class.stable.url).to eq("https://brew.sh/ignorable-error-1.0.tar.gz") + end + + it "raises FormulaUnreadableError for errors it cannot resume despite ignore_errors" do + expect do + described_class.load_formula( + "unreadable-error", + mktmpdir/"unreadable-error.rb", + <<~RUBY, + class UnreadableError < Formula + nonexistent_dsl_method "foo" + url "https://brew.sh/unreadable-error-1.0.tar.gz" + end + RUBY + "UnreadableErrorNamespace", + flags: [], + ignore_errors: true, + ) + end.to raise_error(FormulaUnreadableError) + end + it "masks sensitive environment variables while evaluating formulae" do with_env(HOMEBREW_SECRET_TOKEN: "password") do formula_class = described_class.load_formula( diff --git a/Library/Homebrew/test/ignorable_spec.rb b/Library/Homebrew/test/ignorable_spec.rb new file mode 100644 index 0000000000..3ab10baa93 --- /dev/null +++ b/Library/Homebrew/test/ignorable_spec.rb @@ -0,0 +1,99 @@ +# typed: false +# frozen_string_literal: true + +require "ignorable" + +RSpec.describe Ignorable do + def raise_runtime_error + raise "raised in block" + end + + describe "::hook_raise" do + it "resumes execution after the raise site when the handler returns :ignore" do + steps = [] + result = described_class.hook_raise(on_ignorable: ->(_e) { :ignore }) do + steps << :before + raise_runtime_error + steps << :after + steps + end + expect(result).to eq([:before, :after]) + end + + it "extends exceptions passed to the handler with ExceptionMixin" do + exception = nil + described_class.hook_raise(on_ignorable: lambda { |e| + exception = e + :ignore + }) { raise_runtime_error } + expect(exception).to be_a(described_class::ExceptionMixin) + end + + it "raises at the raise site when the handler returns :raise" do + result = described_class.hook_raise(on_ignorable: ->(_e) { :raise }) do + raise_runtime_error + rescue RuntimeError + :rescued_in_block + end + expect(result).to eq(:rescued_in_block) + end + + it "propagates unrescued exceptions when the handler returns :raise" do + expect do + described_class.hook_raise(on_ignorable: ->(_e) { :raise }) { raise_runtime_error } + end.to raise_error(RuntimeError, "raised in block") + end + + it "preserves the exception's backtrace when the handler returns :raise" do + yielded_backtrace = nil + exception = nil + begin + described_class.hook_raise(on_ignorable: lambda { |e| + yielded_backtrace = e.backtrace.dup + :raise + }) { raise_runtime_error } + rescue RuntimeError => e + exception = e + end + expect(exception.backtrace).to eq(yielded_backtrace) + end + + it "runs the block's ensure blocks when the handler raises" do + ensured = false + expect do + described_class.hook_raise(on_ignorable: ->(e) { raise e }) do + raise_runtime_error + ensure + ensured = true + end + end.to raise_error(RuntimeError, "raised in block") + expect(ensured).to be(true) + end + + it "does not consult the handler for exceptions not raised from Ruby code" do + expect do + described_class.hook_raise(on_ignorable: ->(_e) { :ignore }) { Integer("nope") } + end.to raise_error(ArgumentError) + end + + it "does not consult the handler for ScriptError" do + expect do + described_class.hook_raise(on_ignorable: ->(_e) { :ignore }) { raise NotImplementedError } + end.to raise_error(NotImplementedError) + end + + it "restores the original raise afterwards" do + described_class.hook_raise(on_ignorable: ->(_e) { :raise }) { :noop } + expect(Object.instance_method(:raise).owner).to eq(Kernel) + end + + it "restores the original raise when an exception propagates" do + begin + described_class.hook_raise(on_ignorable: ->(_e) { :raise }) { raise_runtime_error } + rescue RuntimeError + nil + end + expect(Object.instance_method(:fail).owner).to eq(Kernel) + end + end +end diff --git a/Library/Homebrew/test/support/helper/cmd/brew-verify-undefined.rb b/Library/Homebrew/test/support/helper/cmd/brew-verify-undefined.rb index 5cfc6aa5f8..059df48fbe 100755 --- a/Library/Homebrew/test/support/helper/cmd/brew-verify-undefined.rb +++ b/Library/Homebrew/test/support/helper/cmd/brew-verify-undefined.rb @@ -66,7 +66,6 @@ UNDEFINED_CONSTANTS_AFTER_REQUIRE = T.let({ "downloadable" => %w[Concurrent], "extend/os/mac/extend/pathname" => %w[MachO], "formula_cellar_checks" => %w[Plist], - "ignorable" => %w[Warnings], "keg" => %w[MachO], "livecheck/livecheck" => %w[Addressable], "os/mac/xcode" => %w[Plist],