Merge pull request #23393 from Homebrew/replace-callcc-fiber

ignorable: replace callcc with Fiber

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) }

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

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

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

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(

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

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],