Merge pull request #18560 from Homebrew/mktemp-type

mktemp: strict type and allow `#run` without chdir

Library/Homebrew/formula.rb

@@ -2804,7 +2804,7 @@ class Formula
 
     mktemp("#{name}-test") do |staging|
       staging.retain! if keep_tmp
-      @testpath = staging.tmpdir
+      @testpath = T.must(staging.tmpdir)
       test_env[:HOME] = @testpath
       setup_home @testpath
       begin
@@ -3125,8 +3125,16 @@ class Formula
   # recursively delete the temporary directory. Passing `opts[:retain]`
   # or calling `do |staging| ... staging.retain!` in the block will skip
   # the deletion and retain the temporary directory's contents.
-  def mktemp(prefix = name, opts = {}, &block)
+  sig {
-    Mktemp.new(prefix, opts).run(&block)
+    params(
+      prefix:          String,
+      retain:          T::Boolean,
+      retain_in_cache: T::Boolean,
+      block:           T.proc.params(arg0: Mktemp).void,
+    ).void
+  }
+  def mktemp(prefix = name, retain: false, retain_in_cache: false, &block)
+    Mktemp.new(prefix, retain:, retain_in_cache:).run(&block)
   end
 
   # A version of `FileUtils.mkdir` that also changes to that folder in

Library/Homebrew/mktemp.rb

@@ -1,19 +1,23 @@
-# typed: true # rubocop:todo Sorbet/StrictSigil
+# typed: strict
 # frozen_string_literal: true
 
 # Performs {Formula#mktemp}'s functionality and tracks the results.
 # Each instance is only intended to be used once.
+# Can also be used to create a temporary directory with the brew instance's group.
 class Mktemp
   include FileUtils
 
-  # Path to the tmpdir used in this run, as a {Pathname}.
+  # Path to the tmpdir used in this run
+  sig { returns(T.nilable(Pathname)) }
   attr_reader :tmpdir
 
-  def initialize(prefix, opts = {})
+  sig { params(prefix: String, retain: T::Boolean, retain_in_cache: T::Boolean).void }
+  def initialize(prefix, retain: false, retain_in_cache: false)
     @prefix = prefix
-    @retain_in_cache = opts[:retain_in_cache]
+    @retain_in_cache = T.let(retain_in_cache, T::Boolean)
-    @retain = opts[:retain] || @retain_in_cache
+    @retain = T.let(retain || @retain_in_cache, T::Boolean)
-    @quiet = false
+    @quiet = T.let(false, T::Boolean)
+    @tmpdir = T.let(nil, T.nilable(Pathname))
   end
 
   # Instructs this {Mktemp} to retain the staged files.
@@ -23,11 +27,13 @@ class Mktemp
   end
 
   # True if the staged temporary files should be retained.
+  sig { returns(T::Boolean) }
   def retain?
     @retain
   end
 
   # True if the source files should be retained.
+  sig { returns(T::Boolean) }
   def retain_in_cache?
     @retain_in_cache
   end
@@ -43,7 +49,8 @@ class Mktemp
     "[Mktemp: #{tmpdir} retain=#{@retain} quiet=#{@quiet}]"
   end
 
-  def run
+  sig { params(chdir: T::Boolean, _block: T.proc.params(arg0: Mktemp).void).void }
+  def run(chdir: true, &_block)
     prefix_name = @prefix.tr "@", "AT"
     @tmpdir = if retain_in_cache?
       tmp_dir = HOMEBREW_CACHE/"Sources/#{prefix_name}"
@@ -66,13 +73,24 @@ class Mktemp
       Process.gid
     end
     begin
-      chown(nil, group_id, @tmpdir)
+      @tmpdir.chown(nil, group_id)
     rescue Errno::EPERM
-      opoo "Failed setting group \"#{T.must(Etc.getgrgid(group_id)).name}\" on #{@tmpdir}"
+      require "etc"
+      group_name = begin
+        Etc.getgrgid(group_id)&.name
+      rescue ArgumentError
+        # Cover for misconfigured NSS setups
+        nil
+      end
+      opoo "Failed setting group \"#{group_name || group_id}\" on #{@tmpdir}"
     end
 
     begin
-      Dir.chdir(tmpdir) { yield self }
+      if chdir
+        Dir.chdir(@tmpdir) { yield self }
+      else
+        yield self
+      end
     ensure
       ignore_interrupts { chmod_rm_rf(@tmpdir) } unless retain?
     end
@@ -85,6 +103,7 @@ class Mktemp
 
   private
 
+  sig { params(path: Pathname).void }
   def chmod_rm_rf(path)
     if path.directory? && !path.symlink?
       chmod("u+rw", path) if path.owned? # Need permissions in order to see the contents

Library/Homebrew/unpack_strategy.rb

@@ -1,6 +1,7 @@
 # typed: strict
 # frozen_string_literal: true
 
+require "mktemp"
 require "system_command"
 
 # Module containing all available strategies for unpacking archives.
@@ -157,29 +158,8 @@ module UnpackStrategy
     ).returns(T.untyped)
   }
   def extract_nestedly(to: nil, basename: nil, verbose: false, prioritize_extension: false)
-    Dir.mktmpdir("homebrew-unpack", HOMEBREW_TEMP) do |tmp_unpack_dir|
+    Mktemp.new("homebrew-unpack").run(chdir: false) do |unpack_dir|
-      tmp_unpack_dir = Pathname(tmp_unpack_dir)
+      tmp_unpack_dir = T.must(unpack_dir.tmpdir)
-
-      # Make sure files inside the temporary directory have the same group as the brew instance.
-      #
-      # @see https://github.com/Homebrew/brew/blob/4.4.0/Library/Homebrew/mktemp.rb#L57-L72
-      group_id = if HOMEBREW_BREW_FILE.grpowned?
-        HOMEBREW_BREW_FILE.stat.gid
-      else
-        Process.gid
-      end
-      begin
-        tmp_unpack_dir.chown(nil, group_id)
-      rescue Errno::EPERM
-        require "etc"
-        group_name = begin
-          Etc.getgrgid(group_id)&.name
-        rescue ArgumentError
-          # Cover for misconfigured NSS setups
-          nil
-        end
-        opoo "Failed setting group \"#{group_name || group_id}\" on #{tmp_unpack_dir}"
-      end
 
       extract(to: tmp_unpack_dir, basename:, verbose:)