Fix `Pkg#uninstall` not calling `MacOS.undeletable?` for all files.

2 files changed, 19 insertions, 16 deletions

diff --git a/Library/Homebrew/cask/lib/hbc/pkg.rb b/Library/Homebrew/cask/lib/hbc/pkg.rb
index c9aa3180f..8938268a2 100644
--- a/Library/Homebrew/cask/lib/hbc/pkg.rb
+++ b/Library/Homebrew/cask/lib/hbc/pkg.rb
@@ -16,19 +16,17 @@ module Hbc
     def uninstall
       unless pkgutil_bom_files.empty?
         odebug "Deleting pkg files"
-        @command.run("/usr/bin/xargs", args: ["-0", "--", "/bin/rm", "--"], input: pkgutil_bom_files.join("\0"), sudo: true)
+        @command.run!("/usr/bin/xargs", args: ["-0", "--", "/bin/rm", "--"], input: pkgutil_bom_files.join("\0"), sudo: true)
       end
 
       unless pkgutil_bom_specials.empty?
         odebug "Deleting pkg symlinks and special files"
-        @command.run("/usr/bin/xargs", args: ["-0", "--", "/bin/rm", "--"], input: pkgutil_bom_specials.join("\0"), sudo: true)
+        @command.run!("/usr/bin/xargs", args: ["-0", "--", "/bin/rm", "--"], input: pkgutil_bom_specials.join("\0"), sudo: true)
       end
 
       unless pkgutil_bom_dirs.empty?
         odebug "Deleting pkg directories"
         deepest_path_first(pkgutil_bom_dirs).each do |dir|
-          next if MacOS.undeletable?(dir)
-
           with_full_permissions(dir) do
             clean_broken_symlinks(dir)
             clean_ds_store(dir)
@@ -67,6 +65,7 @@ module Hbc
                                    .stdout
                                    .split("\n")
                                    .map { |path| root.join(path) }
+                                   .reject(&MacOS.public_method(:undeletable?))
     end
 
     def root
diff --git a/Library/Homebrew/test/cask/pkg_spec.rb b/Library/Homebrew/test/cask/pkg_spec.rb
index 56061c9fd..07443e76e 100644
--- a/Library/Homebrew/test/cask/pkg_spec.rb
+++ b/Library/Homebrew/test/cask/pkg_spec.rb
@@ -5,16 +5,16 @@ describe Hbc::Pkg, :cask do
     let(:pkg) { described_class.new("my.fake.pkg", fake_system_command) }
 
     it "removes files and dirs referenced by the pkg" do
-      some_files = Array.new(3) { Pathname.new(Tempfile.new("testfile").path) }
+      some_files = Array.new(3) { Pathname.new(Tempfile.new("plain_file").path) }
       allow(pkg).to receive(:pkgutil_bom_files).and_return(some_files)
 
-      some_specials = Array.new(3) { Pathname.new(Tempfile.new("testfile").path) }
+      some_specials = Array.new(3) { Pathname.new(Tempfile.new("special_file").path) }
       allow(pkg).to receive(:pkgutil_bom_specials).and_return(some_specials)
 
-      some_dirs = Array.new(3) { Pathname.new(Dir.mktmpdir) }
+      some_dirs = Array.new(3) { mktmpdir }
       allow(pkg).to receive(:pkgutil_bom_dirs).and_return(some_dirs)
 
-      root_dir = Pathname.new(Dir.mktmpdir)
+      root_dir = Pathname.new(mktmpdir)
       allow(pkg).to receive(:root).and_return(root_dir)
 
       allow(pkg).to receive(:forget)
@@ -55,8 +55,8 @@ describe Hbc::Pkg, :cask do
     end
 
     it "removes broken symlinks" do
-      fake_dir  = Pathname.new(Dir.mktmpdir)
-      fake_root = Pathname.new(Dir.mktmpdir)
+      fake_root = mktmpdir
+      fake_dir  = mktmpdir
       fake_file = fake_dir.join("ima_file").tap { |path| FileUtils.touch(path) }
 
       intact_symlink = fake_dir.join("intact_symlink").tap { |path| path.make_symlink(fake_file) }
@@ -77,13 +77,13 @@ describe Hbc::Pkg, :cask do
     end
 
     it "snags permissions on ornery dirs, but returns them afterwards" do
-      fake_root = Pathname.new(Dir.mktmpdir)
-      fake_dir = Pathname.new(Dir.mktmpdir)
-      fake_file = fake_dir.join("ima_installed_file").tap { |path| FileUtils.touch(path) }
+      fake_root = mktmpdir
+      fake_dir = mktmpdir
+      fake_file = fake_dir.join("ima_unrelated_file").tap { |path| FileUtils.touch(path) }
       fake_dir.chmod(0000)
 
       allow(pkg).to receive(:pkgutil_bom_specials).and_return([])
-      allow(pkg).to receive(:pkgutil_bom_files).and_return([fake_file])
+      allow(pkg).to receive(:pkgutil_bom_files).and_return([])
       allow(pkg).to receive(:pkgutil_bom_dirs).and_return([fake_dir])
       allow(pkg).to receive(:root).and_return(fake_root)
       allow(pkg).to receive(:forget)
@@ -91,8 +91,12 @@ describe Hbc::Pkg, :cask do
       pkg.uninstall
 
       expect(fake_dir).to be_a_directory
-      expect(fake_file).not_to be_a_file
-      expect((fake_dir.stat.mode % 01000).to_s(8)).to eq("0")
+      expect((fake_dir.stat.mode % 01000)).to eq(0)
+
+      fake_dir.chmod(0777)
+      expect(fake_file).to be_a_file
+
+      FileUtils.rm_r fake_dir
     end
   end