Merge pull request #18805 from Homebrew/perm-fixes

cask: try fix a couple of permission edge cases under multi-user
Homebrew · Nov 25, 2024 · e78a0ad · e78a0ad
2 parents f94c7d0 + 7308c10
commit e78a0ad
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 11 deletions.
diff --git a/Library/Homebrew/cask/artifact/abstract_uninstall.rb b/Library/Homebrew/cask/artifact/abstract_uninstall.rb
@@ -116,12 +116,15 @@ def uninstall_launchctl(*services, command: nil, **_)
               print_stderr: false,
             ).stdout
             if plist_status.start_with?("{")
-              command.run!(
+              result = command.run(
                 "/bin/launchctl",
                 args:         ["remove", service],
+                must_succeed: sudo,
                 sudo:,
                 sudo_as_root: sudo,
               )
+              next if !sudo && !result.success?
+
               sleep 1
             end
             paths = [

diff --git a/Library/Homebrew/cask/artifact/moved.rb b/Library/Homebrew/cask/artifact/moved.rb
@@ -164,7 +164,16 @@ def move_back(skip: false, force: false, adopt: false, command: nil, **options)
         source.dirname.mkpath
 
         # We need to preserve extended attributes between copies.
-        command.run!("/bin/cp", args: ["-pR", target, source], sudo: !source.parent.writable?)
+        # This may fail and need sudo if the source has files with restricted permissions.
+        [!source.parent.writable?, true].uniq.each do |sudo|
+          result = command.run(
+            "/bin/cp",
+            args:         ["-pR", target, source],
+            must_succeed: sudo,
+            sudo:,
+          )
+          break if result.success?
+        end
 
         delete(target, force:, command:, **options)
       end

diff --git a/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb b/Library/Homebrew/test/cask/artifact/shared_examples/uninstall_zap.rb
@@ -49,9 +49,10 @@
         )
         .and_return(instance_double(SystemCommand::Result, stdout: unknown_response))
 
-      expect(fake_system_command).to receive(:run!)
-        .with("/bin/launchctl", args: ["remove", "my.fancy.package.service"], sudo: false, sudo_as_root: false)
-        .and_return(instance_double(SystemCommand::Result))
+      expect(fake_system_command).to receive(:run)
+        .with("/bin/launchctl", args: ["remove", "my.fancy.package.service"],
+        must_succeed: false, sudo: false, sudo_as_root: false)
+        .and_return(instance_double(SystemCommand::Result, success?: true))
 
       subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
     end
@@ -76,9 +77,10 @@
         )
         .and_return(instance_double(SystemCommand::Result, stdout: service_info))
 
-      expect(fake_system_command).to receive(:run!)
-        .with("/bin/launchctl", args: ["remove", "my.fancy.package.service"], sudo: true, sudo_as_root: true)
-        .and_return(instance_double(SystemCommand::Result))
+      expect(fake_system_command).to receive(:run)
+        .with("/bin/launchctl", args: ["remove", "my.fancy.package.service"],
+        must_succeed: true, sudo: true, sudo_as_root: true)
+        .and_return(instance_double(SystemCommand::Result, success?: true))
 
       subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
     end
@@ -136,9 +138,10 @@
         )
         .and_return(instance_double(SystemCommand::Result, stdout: service_info))
 
-      expect(fake_system_command).to receive(:run!)
-        .with("/bin/launchctl", args: ["remove", "my.fancy.package.service.12345"], sudo: true, sudo_as_root: true)
-        .and_return(instance_double(SystemCommand::Result))
+      expect(fake_system_command).to receive(:run)
+        .with("/bin/launchctl", args: ["remove", "my.fancy.package.service.12345"],
+        must_succeed: true, sudo: true, sudo_as_root: true)
+        .and_return(instance_double(SystemCommand::Result, success?: true))
 
       subject.public_send(:"#{artifact_dsl_key}_phase", command: fake_system_command)
     end