Refactor UnpackStrategy::Dmg.

Library/Homebrew/test/unpack_strategy/dmg_spec.rb

@@ -2,20 +2,8 @@ require_relative "shared_examples"
 
 describe UnpackStrategy::Dmg, :needs_macos do
   describe "#mount" do
-    subject(:dmg) { described_class.new(path) }
-
     let(:path) { TEST_FIXTURE_DIR/"cask/container.dmg" }
 
-    it "does not store nil mounts for dmgs with extra data" do
-      dmg.mount do |mounts|
-        begin
-          expect(mounts).not_to include nil
-        ensure
-          mounts.each(&dmg.public_method(:eject))
-        end
-      end
-    end
-
     include_examples "UnpackStrategy::detect"
     include_examples "#extract", children: ["container"]
   end

Library/Homebrew/unpack_strategy/dmg.rb

@@ -4,37 +4,116 @@ module UnpackStrategy
   class Dmg
     include UnpackStrategy
 
+    module Bom
+      DMG_METADATA = Set.new %w[
+        .background
+        .com.apple.timemachine.donotpresent
+        .com.apple.timemachine.supported
+        .DocumentRevisions-V100
+        .DS_Store
+        .fseventsd
+        .MobileBackups
+        .Spotlight-V100
+        .TemporaryItems
+        .Trashes
+        .VolumeIcon.icns
+      ].freeze
+      private_constant :DMG_METADATA
+
+      refine Pathname do
+        def dmg_metadata?
+          DMG_METADATA.include?(cleanpath.ascend.to_a.last.to_s)
+        end
+
+        # symlinks to system directories (commonly to /Applications)
+        def system_dir_symlink?
+          symlink? && MacOS.system_dir?(readlink)
+        end
+
+        def bom
+          # We need to use `find` here instead of Ruby in order to properly handle
+          # file names containing special characters, such as “e” + “´” vs. “é”.
+          system_command("find", args: [".", "-print0"], chdir: self, print_stderr: false)
+            .stdout
+            .split("\0")
+            .reject { |path| Pathname(path).dmg_metadata? }
+            .reject { |path| (self/path).system_dir_symlink? }
+            .join("\n")
+        end
+      end
+    end
+    private_constant :Bom
+
+    using Bom
+
+    class Mount
+      include UnpackStrategy
+
+      def eject
+        tries ||= 3
+
+        return unless path.exist?
+
+        if tries > 1
+          system_command! "diskutil",
+                          args: ["eject", path],
+                          print_stderr: false
+        else
+          system_command! "diskutil",
+                          args: ["unmount", "force", path],
+                          print_stderr: false
+        end
+      rescue ErrorDuringExecution => e
+        raise e if (tries -= 1).zero?
+        sleep 1
+        retry
+      end
+
+      private
+
+      def extract_to_dir(unpack_dir, basename:, verbose:)
+        Tempfile.open(["", ".bom"]) do |bomfile|
+          bomfile.close
+
+          Tempfile.open(["", ".list"]) do |filelist|
+            filelist.puts(path.bom)
+            filelist.close
+
+            system_command! "mkbom", args: ["-s", "-i", filelist.path, "--", bomfile.path]
+          end
+
+          system_command! "ditto", args: ["--bom", bomfile.path, "--", path, unpack_dir]
+        end
+      end
+    end
+    private_constant :Mount
+
     def self.can_extract?(path:, magic_number:)
-      imageinfo = system_command("/usr/bin/hdiutil",
+      imageinfo = system_command("hdiutil",
-                                 # realpath is a failsafe against unusual filenames
+                                 args: ["imageinfo", path],
-                                 args: ["imageinfo", path.realpath],
                                  print_stderr: false).stdout
 
       !imageinfo.empty?
     end
 
+    private
+
     def extract_to_dir(unpack_dir, basename:, verbose:)
       mount(verbose: verbose) do |mounts|
-        begin
+        raise "No mounts found in '#{path}'; perhaps it is a bad disk image?" if mounts.empty?
-          raise "No mounts found in '#{path}'; perhaps it is a bad disk image?" if mounts.empty?
+
-          mounts.each do |mount|
+        mounts.each do |mount|
-            extract_mount(mount, to: unpack_dir)
+          mount.extract(to: unpack_dir)
-          end
-        ensure
-          mounts.each(&method(:eject))
         end
       end
     end
-    private :extract_to_dir
 
     def mount(verbose: false)
-      # realpath is a failsafe against unusual filenames
+      Dir.mktmpdir do |mount_dir|
-      realpath = path.realpath
+        mount_dir = Pathname(mount_dir)
-      path = realpath
 
-      Dir.mktmpdir do |unpack_dir|
+        without_eula = system_command("hdiutil",
-        without_eula = system_command("/usr/bin/hdiutil",
+                                      args: ["attach", "-plist", "-nobrowse", "-readonly", "-noidme", "-mountrandom", mount_dir, path],
-                                      args: ["attach", "-plist", "-nobrowse", "-readonly", "-noidme", "-mountrandom", unpack_dir, path],
                                       input: "qn\n",
                                       print_stderr: false)
 
@@ -42,13 +121,13 @@ module UnpackStrategy
         plist = if without_eula.success?
           without_eula.plist
         else
-          cdr_path = Pathname.new(unpack_dir).join("#{path.basename(".dmg")}.cdr")
+          cdr_path = mount_dir/path.basename.sub_ext(".cdr")
 
-          system_command!("/usr/bin/hdiutil", args: ["convert", "-quiet", "-format", "UDTO", "-o", cdr_path, path])
+          system_command!("hdiutil", args: ["convert", "-quiet", "-format", "UDTO", "-o", cdr_path, path])
 
           with_eula = system_command!(
             "/usr/bin/hdiutil",
-            args: ["attach", "-plist", "-nobrowse", "-readonly", "-noidme", "-mountrandom", unpack_dir, cdr_path],
+            args: ["attach", "-plist", "-nobrowse", "-readonly", "-noidme", "-mountrandom", mount_dir, cdr_path],
           )
 
           if verbose && !(eula_text = without_eula.stdout).empty?
@@ -59,95 +138,21 @@ module UnpackStrategy
           with_eula.plist
         end
 
-        yield mounts_from_plist(plist)
+        mounts = if plist.respond_to?(:fetch)
-      end
+          plist.fetch("system-entities", [])
-    end
+               .map { |entity| entity["mount-point"] }
-
+               .compact
-    def eject(mount)
+               .map { |path| Mount.new(path) }
-      # realpath is a failsafe against unusual filenames
-      mountpath = Pathname.new(mount).realpath
-
-      begin
-        tries ||= 3
-
-        return unless mountpath.exist?
-
-        if tries > 1
-          system_command! "/usr/sbin/diskutil",
-                          args: ["eject", mountpath],
-                          print_stderr: false
         else
-          system_command! "/usr/sbin/diskutil",
+          []
-                          args: ["unmount", "force", mountpath],
-                          print_stderr: false
         end
-      rescue ErrorDuringExecution => e
-        raise e if (tries -= 1).zero?
-        sleep 1
-        retry
-      end
-    end
 
-    private
+        begin
-
+          yield mounts
-    def extract_mount(mount, to:)
+        ensure
-      Tempfile.open(["", ".bom"]) do |bomfile|
+          mounts.each(&:eject)
-        bomfile.close
-
-        Tempfile.open(["", ".list"]) do |filelist|
-          filelist.puts(bom_filelist_from_path(mount))
-          filelist.close
-
-          system_command! "/usr/bin/mkbom", args: ["-s", "-i", filelist.path, "--", bomfile.path]
-          system_command! "/usr/bin/ditto", args: ["--bom", bomfile.path, "--", mount, to]
         end
       end
     end
-
-    def bom_filelist_from_path(mount)
-      # We need to use `find` here instead of Ruby in order to properly handle
-      # file names containing special characters, such as “e” + “´” vs. “é”.
-      system_command("/usr/bin/find", args: [".", "-print0"], chdir: mount, print_stderr: false)
-        .stdout
-        .split("\0")
-        .reject { |path| skip_path?(mount, path) }
-        .join("\n")
-    end
-
-    def skip_path?(mount, path)
-      path = Pathname(path.sub(%r{\A\./}, ""))
-      dmg_metadata?(path) || system_dir_symlink?(mount, path)
-    end
-
-    # unnecessary DMG metadata
-    DMG_METADATA_FILES = Set.new %w[
-      .background
-      .com.apple.timemachine.donotpresent
-      .com.apple.timemachine.supported
-      .DocumentRevisions-V100
-      .DS_Store
-      .fseventsd
-      .MobileBackups
-      .Spotlight-V100
-      .TemporaryItems
-      .Trashes
-      .VolumeIcon.icns
-    ].freeze
-
-    def dmg_metadata?(path)
-      relative_root = path.sub(%r{/.*}, "")
-      DMG_METADATA_FILES.include?(relative_root.basename.to_s)
-    end
-
-    def system_dir_symlink?(mount, path)
-      full_path = Pathname(mount).join(path)
-      # symlinks to system directories (commonly to /Applications)
-      full_path.symlink? && MacOS.system_dir?(full_path.readlink)
-    end
-
-    def mounts_from_plist(plist)
-      return [] unless plist.respond_to?(:fetch)
-      plist.fetch("system-entities", []).map { |e| e["mount-point"] }.compact
-    end
   end
 end