cask/quarantine: sudo correctly during tests.

[MikeMcQuaid]

Fixes #15958

[Bo98]

What is to here and why is it not writeable? Seems like there's a problem here if we're demanding sudo and it doesn't fail without it.

[MikeMcQuaid]

What is to here and why is it not writeable?

It's explicitly set to not be writable in the test.

Seems like there's a problem here if we're demanding sudo and it doesn't fail without it.

If you can reproduce the same behaviour in Not The Test Environment: sure. This just feels like an omission when underlying functionality was changed, though.

[Bo98]

Right I understand the situation now.

In the test we pass NeverSudoSystemCommand to install_phase. This down the line gets passed, in this case, to Cask::Artifact::Moved#move as the command parameter, where we run command.run! instead of system_command!.

The issue here is we don't pass this further down to the Quarantine.copy_xattrs call. It seems like we should be doing so here.

[apainintheneck]

I tested out @Bo98's idea and the failing test now succeeds for me locally.

diff --git a/Library/Homebrew/cask/artifact/moved.rb b/Library/Homebrew/cask/artifact/moved.rb
index d41e62cec..a9e3470d7 100644
--- a/Library/Homebrew/cask/artifact/moved.rb
+++ b/Library/Homebrew/cask/artifact/moved.rb
@@ -91,7 +91,7 @@ module Cask
             command.run!("/bin/cp", args: ["-pR", *source.children, target],
                                     sudo: true)
           end
-          Quarantine.copy_xattrs(source, target)
+          Quarantine.copy_xattrs(source, target, command: command)
           source.rmtree
         elsif target.dirname.writable?
           FileUtils.move(source, target)
diff --git a/Library/Homebrew/cask/quarantine.rb b/Library/Homebrew/cask/quarantine.rb
index 227790629..e038011e6 100644
--- a/Library/Homebrew/cask/quarantine.rb
+++ b/Library/Homebrew/cask/quarantine.rb
@@ -174,11 +174,11 @@ module Cask
       raise CaskQuarantinePropagationError.new(to, quarantiner.stderr)
     end
 
-    sig { params(from: Pathname, to: Pathname).void }
-    def self.copy_xattrs(from, to)
+    sig { params(from: Pathname, to: Pathname, command: T.class_of(SystemCommand)).void }
+    def self.copy_xattrs(from, to, command:)
       odebug "Copying xattrs from #{from} to #{to}"
 
-      system_command!(
+      command.run!(
         swift,
         args: [
           *swift_target_args,

[MikeMcQuaid]

@Bo98 Good catch, will try to take a look, thanks.

[MikeMcQuaid]

Thanks @Bo98 @apainintheneck have updated accordingly based on your feedback and diff.

[apainintheneck]

Thanks for the help with fixing this! I thought it was maybe a local problem when I first saw it pop up since CI has been green the whole time.