PoC: Use a factory rather than dynamically redefine methods

dduugg · Aug 15, 2024 · c4f9e5e · c4f9e5e
1 parent 5ffacaf
commit c4f9e5e
Show file tree

Hide file tree

Showing 6 changed files with 31 additions and 21 deletions.
diff --git a/Library/Homebrew/cleaner.rb b/Library/Homebrew/cleaner.rb
@@ -1,4 +1,4 @@
-# typed: true # rubocop:todo Sorbet/StrictSigil
+# typed: strict
 # frozen_string_literal: true
 
 # Cleans a newly installed keg.
@@ -104,7 +104,7 @@ def prune
     end
   end
 
-  sig { params(path: Pathname).returns(T::Boolean) }
+  sig { overridable.params(path: Pathname).returns(T::Boolean) }
   def executable_path?(path)
     path.text_executable? || path.executable?
   end
@@ -113,7 +113,7 @@ def executable_path?(path)
   # pointless conflicts with other formulae. They are removed by Debian,
   # Arch & MacPorts amongst other packagers as well. The files are
   # created as part of installing any Perl module.
-  PERL_BASENAMES = Set.new(%w[perllocal.pod .packlist]).freeze
+  PERL_BASENAMES = T.let(Set.new(%w[perllocal.pod .packlist]).freeze, T::Set[String])
 
   # Clean a top-level (`bin`, `sbin`, `lib`) directory, recursively, by fixing file
   # permissions and removing .la files, unless the files (or parent
@@ -201,5 +201,3 @@ def clean_python_metadata
     end
   end
 end
-
-require "extend/os/cleaner"
diff --git a/Library/Homebrew/cleaner_factory.rb b/Library/Homebrew/cleaner_factory.rb
@@ -0,0 +1,22 @@
+# typed: strict
+# frozen_string_literal: true
+
+require "cleaner"
+
+class CleanerFactory
+  # This just a PoC
+  # rubocop:disable Homebrew/MoveToExtendOS
+  CLEANER_CLASS = T.let(if OS.mac?
+                          require "extend/os/mac/cleaner"
+                          CleanerMac
+                        elsif OS.linux?
+                          require "extend/os/linux/cleaner"
+                          CleanerLinux
+                        else
+                          Cleaner
+  end.freeze, T.class_of(Cleaner))
+  # rubocop:enable Homebrew/MoveToExtendOS
+
+  sig { params(formula: Formula).returns(Cleaner) }
+  def self.create(formula) = CLEANER_CLASS.new(formula)
+end
diff --git a/Library/Homebrew/extend/os/cleaner.rb b/Library/Homebrew/extend/os/cleaner.rb
diff --git a/Library/Homebrew/extend/os/linux/cleaner.rb b/Library/Homebrew/extend/os/linux/cleaner.rb
@@ -1,10 +1,10 @@
 # typed: strict
 # frozen_string_literal: true
 
-class Cleaner
+class CleanerLinux < Cleaner
   private
 
-  sig { params(path: Pathname).returns(T::Boolean) }
+  sig { override.params(path: Pathname).returns(T::Boolean) }
   def executable_path?(path)
     path.elf? || path.text_executable?
   end

diff --git a/Library/Homebrew/extend/os/mac/cleaner.rb b/Library/Homebrew/extend/os/mac/cleaner.rb
@@ -1,12 +1,10 @@
 # typed: strict
 # frozen_string_literal: true
 
-class Cleaner
+class CleanerMac < Cleaner
   private
 
-  undef executable_path?
-
-  sig { params(path: Pathname).returns(T::Boolean) }
+  sig { override.params(path: Pathname).returns(T::Boolean) }
   def executable_path?(path)
     path.mach_o_executable? || path.text_executable?
   end

diff --git a/Library/Homebrew/formula_installer.rb b/Library/Homebrew/formula_installer.rb
@@ -6,7 +6,7 @@
 require "tab"
 require "utils/bottles"
 require "caveats"
-require "cleaner"
+require "cleaner_factory"
 require "formula_cellar_checks"
 require "install_renamed"
 require "sandbox"
@@ -1107,7 +1107,7 @@ def fix_dynamic_linkage(keg)
   sig { void }
   def clean
     ohai "Cleaning" if verbose?
-    Cleaner.new(formula).clean
+    CleanerFactory.create(formula).clean
   rescue Exception => e # rubocop:disable Lint/RescueException
     opoo "The cleaning step did not complete successfully"
     puts "Still, the installation was successful, so we will link it into your prefix."