From 6d318761d21e43ef8785ae4da24b8f6fdbd52db8 Mon Sep 17 00:00:00 2001 From: Nathan Toone Date: Wed, 28 Dec 2016 10:56:03 -0700 Subject: [PATCH 1/3] Delete pkgutil directories that are really files. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Sometimes, pkgutil will return actual files (usually .nib files) as if they were part of the directory. Microsoft Office is an example of this: in a recent update the file `/Library/Application Support/Microsoft/MAU2.0/Microsoft AutoUpdate.app/Contents/SharedSupport/Microsoft Error Reporting.app/Contents/Resources/en.lproj/MainWindowAlt.nib` was returning from `/usr/sbin/pkgutil --only-dirs --files com.microsoft.package.component` even though it should have been a file instead of a directory. This caused the `rmdir` command to fail. This patch will check if we are trying to delete a “directory” that is really a “file” - and if we are, we just delete the file instead. This will allow packages that get in this state to be uninstalled. A unit test which can be run using `brew cask-tests` is also included. --- Library/Homebrew/cask/lib/hbc/pkg.rb | 8 ++++++++ Library/Homebrew/cask/test/cask/pkg_test.rb | 17 +++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/Library/Homebrew/cask/lib/hbc/pkg.rb b/Library/Homebrew/cask/lib/hbc/pkg.rb index 2fb634f248..497a7b6ddb 100644 --- a/Library/Homebrew/cask/lib/hbc/pkg.rb +++ b/Library/Homebrew/cask/lib/hbc/pkg.rb @@ -26,6 +26,7 @@ module Hbc _deepest_path_first(pkgutil_bom_dirs).each do |dir| next unless dir.exist? && !MacOS.undeletable?(dir) _with_full_permissions(dir) do + _delete_broken_file_dir(dir) && next _clean_broken_symlinks(dir) _clean_ds_store(dir) _rmdir(dir) @@ -97,6 +98,13 @@ module Hbc end end + # Some pkgs (microsoft office for one) leave files (generally nibs) but + # report them as directories. We remove these as files instead. + def _delete_broken_file_dir(path) + return unless path.file? && !path.symlink? + @command.run!("/bin/rm", args: ["-f", "--", path], sudo: true) + end + # Some pkgs leave broken symlinks hanging around; we clean them out before # attempting to rmdir to prevent extra cruft from lying around after # uninstall diff --git a/Library/Homebrew/cask/test/cask/pkg_test.rb b/Library/Homebrew/cask/test/cask/pkg_test.rb index 85a42356e2..b99d49ddd4 100644 --- a/Library/Homebrew/cask/test/cask/pkg_test.rb +++ b/Library/Homebrew/cask/test/cask/pkg_test.rb @@ -68,6 +68,23 @@ describe Hbc::Pkg do fake_dir.must_be :exist? end + it "chokes on directories that are really files" do + pkg = Hbc::Pkg.new("my.fake.pkg", Hbc::NeverSudoSystemCommand) + + fake_dir = Pathname(Dir.mktmpdir) + fake_file = fake_dir.join("ima_file").tap { |path| FileUtils.touch(path) } + + pkg.stubs(:pkgutil_bom_specials).returns([]) + pkg.stubs(:pkgutil_bom_files).returns([]) + pkg.stubs(:pkgutil_bom_dirs).returns([fake_file, fake_dir]) + pkg.stubs(:forget) + + pkg.uninstall + + fake_file.wont_be :exist? + fake_dir.wont_be :exist? + end + it "snags permissions on ornery dirs, but returns them afterwords" do pkg = Hbc::Pkg.new("my.fake.pkg", Hbc::NeverSudoSystemCommand) From fc712b0089dbb4e6ebeb3f5bc846b00a62029f69 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sat, 4 Feb 2017 20:24:29 +0100 Subject: [PATCH 2/3] Make descriptions a bit clearer. --- Library/Homebrew/cask/lib/hbc/pkg.rb | 4 ++-- Library/Homebrew/cask/test/cask/pkg_test.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Library/Homebrew/cask/lib/hbc/pkg.rb b/Library/Homebrew/cask/lib/hbc/pkg.rb index 497a7b6ddb..39252b48a5 100644 --- a/Library/Homebrew/cask/lib/hbc/pkg.rb +++ b/Library/Homebrew/cask/lib/hbc/pkg.rb @@ -98,8 +98,8 @@ module Hbc end end - # Some pkgs (microsoft office for one) leave files (generally nibs) but - # report them as directories. We remove these as files instead. + # Some pkgs incorrectly report files (generally nibs) + # as directories; we remove these as files instead. def _delete_broken_file_dir(path) return unless path.file? && !path.symlink? @command.run!("/bin/rm", args: ["-f", "--", path], sudo: true) diff --git a/Library/Homebrew/cask/test/cask/pkg_test.rb b/Library/Homebrew/cask/test/cask/pkg_test.rb index b99d49ddd4..a52c90a0e6 100644 --- a/Library/Homebrew/cask/test/cask/pkg_test.rb +++ b/Library/Homebrew/cask/test/cask/pkg_test.rb @@ -68,11 +68,11 @@ describe Hbc::Pkg do fake_dir.must_be :exist? end - it "chokes on directories that are really files" do + it "cleans files incorrectly reported as directories" do pkg = Hbc::Pkg.new("my.fake.pkg", Hbc::NeverSudoSystemCommand) fake_dir = Pathname(Dir.mktmpdir) - fake_file = fake_dir.join("ima_file").tap { |path| FileUtils.touch(path) } + fake_file = fake_dir.join("ima_file_pretending_to_be_a_dir").tap { |path| FileUtils.touch(path) } pkg.stubs(:pkgutil_bom_specials).returns([]) pkg.stubs(:pkgutil_bom_files).returns([]) From d22cfd3866257abb9c3ade46c6c34f341cf70514 Mon Sep 17 00:00:00 2001 From: Markus Reiter Date: Sat, 4 Feb 2017 20:24:45 +0100 Subject: [PATCH 3/3] Always use `Pathname.new` in `pkg_test.rb`. --- Library/Homebrew/cask/test/cask/pkg_test.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Library/Homebrew/cask/test/cask/pkg_test.rb b/Library/Homebrew/cask/test/cask/pkg_test.rb index a52c90a0e6..ac43f9e633 100644 --- a/Library/Homebrew/cask/test/cask/pkg_test.rb +++ b/Library/Homebrew/cask/test/cask/pkg_test.rb @@ -5,13 +5,13 @@ describe Hbc::Pkg do it "removes files and dirs referenced by the pkg" do pkg = Hbc::Pkg.new("my.fake.pkg", Hbc::NeverSudoSystemCommand) - some_files = Array.new(3) { Pathname(Tempfile.new("testfile").path) } + some_files = Array.new(3) { Pathname.new(Tempfile.new("testfile").path) } pkg.stubs(:pkgutil_bom_files).returns some_files - some_specials = Array.new(3) { Pathname(Tempfile.new("testfile").path) } + some_specials = Array.new(3) { Pathname.new(Tempfile.new("testfile").path) } pkg.stubs(:pkgutil_bom_specials).returns some_specials - some_dirs = Array.new(3) { Pathname(Dir.mktmpdir) } + some_dirs = Array.new(3) { Pathname.new(Dir.mktmpdir) } pkg.stubs(:pkgutil_bom_dirs).returns some_dirs pkg.stubs(:forget) @@ -50,7 +50,7 @@ describe Hbc::Pkg do it "cleans broken symlinks, but leaves AOK symlinks" do pkg = Hbc::Pkg.new("my.fake.pkg", Hbc::NeverSudoSystemCommand) - fake_dir = Pathname(Dir.mktmpdir) + fake_dir = Pathname.new(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) } @@ -71,7 +71,7 @@ describe Hbc::Pkg do it "cleans files incorrectly reported as directories" do pkg = Hbc::Pkg.new("my.fake.pkg", Hbc::NeverSudoSystemCommand) - fake_dir = Pathname(Dir.mktmpdir) + fake_dir = Pathname.new(Dir.mktmpdir) fake_file = fake_dir.join("ima_file_pretending_to_be_a_dir").tap { |path| FileUtils.touch(path) } pkg.stubs(:pkgutil_bom_specials).returns([]) @@ -88,7 +88,7 @@ describe Hbc::Pkg do it "snags permissions on ornery dirs, but returns them afterwords" do pkg = Hbc::Pkg.new("my.fake.pkg", Hbc::NeverSudoSystemCommand) - fake_dir = Pathname(Dir.mktmpdir) + fake_dir = Pathname.new(Dir.mktmpdir) fake_file = fake_dir.join("ima_installed_file").tap { |path| FileUtils.touch(path) }