Merge pull request #2772 from reitermarkus/dmg-eula

Fix DMG mounting.

5 files changed, 73 insertions, 56 deletions

diff --git a/Library/Homebrew/cask/lib/hbc/artifact/nested_container.rb b/Library/Homebrew/cask/lib/hbc/artifact/nested_container.rb
index 45f23fe37..84253ea30 100644
--- a/Library/Homebrew/cask/lib/hbc/artifact/nested_container.rb
+++ b/Library/Homebrew/cask/lib/hbc/artifact/nested_container.rb
@@ -16,7 +16,7 @@ module Hbc
         end
 
         ohai "Extracting nested container #{source.basename}"
-        container.new(@cask, source, @command).extract
+        container.new(@cask, source, @command, verbose: verbose?).extract
         FileUtils.remove_entry_secure(source)
       end
     end
diff --git a/Library/Homebrew/cask/lib/hbc/container/base.rb b/Library/Homebrew/cask/lib/hbc/container/base.rb
index 1f1c9ad9b..52eb5aab1 100644
--- a/Library/Homebrew/cask/lib/hbc/container/base.rb
+++ b/Library/Homebrew/cask/lib/hbc/container/base.rb
@@ -1,11 +1,16 @@
 module Hbc
   class Container
     class Base
-      def initialize(cask, path, command, nested: false)
+      def initialize(cask, path, command, nested: false, verbose: false)
         @cask = cask
         @path = path
         @command = command
         @nested = nested
+        @verbose = verbose
+      end
+
+      def verbose?
+        @verbose
       end
 
       def extract_nested_inside(dir)
@@ -32,7 +37,7 @@ module Hbc
         return false unless container
 
         ohai "Extracting nested container #{source.basename}"
-        container.new(@cask, source, @command, nested: true).extract
+        container.new(@cask, source, @command, nested: true, verbose: verbose?).extract
 
         true
       end
diff --git a/Library/Homebrew/cask/lib/hbc/container/dmg.rb b/Library/Homebrew/cask/lib/hbc/container/dmg.rb
index 113c6fb11..1d172a4b7 100644
--- a/Library/Homebrew/cask/lib/hbc/container/dmg.rb
+++ b/Library/Homebrew/cask/lib/hbc/container/dmg.rb
@@ -13,61 +13,76 @@ module Hbc
                               print_stderr: false).stdout.empty?
       end
 
-      attr_reader :mounts
-      def initialize(*args)
-        super(*args)
-        @mounts = []
-      end
-
       def extract
-        mount!
-        assert_mounts_found
-        extract_mounts
-      ensure
-        eject!
+        mount do |mounts|
+          begin
+            raise CaskError, "No mounts found in '#{@path}'; perhaps it is a bad DMG?" if mounts.empty?
+            mounts.each(&method(:extract_mount))
+          ensure
+            mounts.each(&method(:eject))
+          end
+        end
       end
 
-      def mount!
-        plist = @command.run!("/usr/bin/hdiutil",
-                              # realpath is a failsafe against unusual filenames
-                              args:  %w[mount -plist -nobrowse -readonly -noidme -mountrandom /tmp] + [Pathname.new(@path).realpath],
-                              input: "y\n")
-                        .plist
-        @mounts = mounts_from_plist(plist)
-      end
+      def mount
+        # realpath is a failsafe against unusual filenames
+        path = Pathname.new(@path).realpath
 
-      def eject!
-        @mounts.each do |mount|
-          # realpath is a failsafe against unusual filenames
-          mountpath = Pathname.new(mount).realpath
-          next unless mountpath.exist?
+        Dir.mktmpdir do |unpack_dir|
+          cdr_path = Pathname.new(unpack_dir).join("#{path.basename(".dmg")}.cdr")
 
-          begin
-            tries ||= 3
-            if tries > 1
-              @command.run("/usr/sbin/diskutil",
-                           args:         ["eject", mountpath],
-                           print_stderr: false)
-            else
-              @command.run("/usr/sbin/diskutil",
-                           args:         ["unmount", "force", mountpath],
-                           print_stderr: false)
+          without_eula = @command.run("/usr/bin/hdiutil",
+                                 args:  ["attach", "-plist", "-nobrowse", "-readonly", "-noidme", "-mountrandom", unpack_dir, path],
+                                 input: "qn\n",
+                                 print_stderr: false)
+
+          # If mounting without agreeing to EULA succeeded, there is none.
+          plist = if without_eula.success?
+            without_eula.plist
+          else
+            @command.run!("/usr/bin/hdiutil", args: ["convert", "-quiet", "-format", "UDTO", "-o", cdr_path, path])
+
+            with_eula = @command.run!("/usr/bin/hdiutil",
+                          args: ["attach", "-plist", "-nobrowse", "-readonly", "-noidme", "-mountrandom", unpack_dir, cdr_path])
+
+            if verbose? && !(eula_text = without_eula.stdout).empty?
+              ohai "Software License Agreement for '#{path}':"
+              puts eula_text
             end
-            raise CaskError, "Failed to eject #{mountpath}" if mountpath.exist?
-          rescue CaskError => e
-            raise e if (tries -= 1).zero?
-            sleep 1
-            retry
+
+            with_eula.plist
           end
+
+          yield mounts_from_plist(plist)
         end
       end
 
-      private
-
-      def extract_mounts
-        @mounts.each(&method(:extract_mount))
+      def eject(mount)
+        # realpath is a failsafe against unusual filenames
+        mountpath = Pathname.new(mount).realpath
+        return unless mountpath.exist?
+
+        begin
+          tries ||= 3
+          if tries > 1
+            @command.run("/usr/sbin/diskutil",
+                         args:         ["eject", mountpath],
+                         print_stderr: false)
+          else
+            @command.run("/usr/sbin/diskutil",
+                         args:         ["unmount", "force", mountpath],
+                         print_stderr: false)
+          end
+          raise CaskError, "Failed to eject #{mountpath}" if mountpath.exist?
+        rescue CaskError => e
+          raise e if (tries -= 1).zero?
+          sleep 1
+          retry
+        end
       end
 
+      private
+
       def extract_mount(mount)
         Tempfile.open(["", ".bom"]) do |bomfile|
           bomfile.close
@@ -124,10 +139,6 @@ module Hbc
         return [] unless plist.respond_to?(:fetch)
         plist.fetch("system-entities", []).map { |e| e["mount-point"] }.compact
       end
-
-      def assert_mounts_found
-        raise CaskError, "No mounts found in '#{@path}'; perhaps it is a bad DMG?" if @mounts.empty?
-      end
     end
   end
 end
diff --git a/Library/Homebrew/cask/lib/hbc/installer.rb b/Library/Homebrew/cask/lib/hbc/installer.rb
index 252205a3b..53210ed4b 100644
--- a/Library/Homebrew/cask/lib/hbc/installer.rb
+++ b/Library/Homebrew/cask/lib/hbc/installer.rb
@@ -153,7 +153,7 @@ module Hbc
       end
 
       odebug "Using container class #{container} for #{@downloaded_path}"
-      container.new(@cask, @downloaded_path, @command).extract
+      container.new(@cask, @downloaded_path, @command, verbose: verbose?).extract
     end
 
     def install_artifacts
diff --git a/Library/Homebrew/test/cask/container/dmg_spec.rb b/Library/Homebrew/test/cask/container/dmg_spec.rb
index a94362aba..4f3f57071 100644
--- a/Library/Homebrew/test/cask/container/dmg_spec.rb
+++ b/Library/Homebrew/test/cask/container/dmg_spec.rb
@@ -9,11 +9,12 @@ describe Hbc::Container::Dmg, :cask do
         Hbc::SystemCommand,
       )
 
-      begin
-        dmg.mount!
-        expect(dmg.mounts).not_to include nil
-      ensure
-        dmg.eject!
+      dmg.mount do |mounts|
+        begin
+          expect(mounts).not_to include nil
+        ensure
+          mounts.each(&dmg.public_method(:eject))
+        end
       end
     end
   end