Use Utils::Cp to copy files

This replaces `FileUtils.cp` and `system_command! "cp"` with the new `Utils::Cp` utility where it is expected that the performance improvement outweighs the cost of the system command invocation.
2024-05-26 08:46:38 +09:00 · 2024-05-26 08:46:38 +09:00 · deaac7ce47
commit deaac7ce47
parent b905959a99
12 changed files with 38 additions and 20 deletions
--- a/Library/Homebrew/cask/artifact/moved.rb
+++ b/Library/Homebrew/cask/artifact/moved.rb
@ -3,6 +3,7 @@

 require "cask/artifact/relocated"
 require "cask/quarantine"
+require "utils/cp"

 module Cask
  module Artifact
@ -108,8 +109,7 @@ module Cask
          if target.writable?
            source.children.each { |child| FileUtils.move(child, target/child.basename) }
          else
-            command.run!("/bin/cp", args: ["-pR", *source.children, target],
-                                    sudo: true)
+            ::Utils::Cp.copy_recursive(source.children, target, sudo: true, command:)
          end
          Quarantine.copy_xattrs(source, target, command:)
          source.rmtree
@ -118,7 +118,7 @@ module Cask
        else
          # default sudo user isn't necessarily able to write to Homebrew's locations
          # e.g. with runas_default set in the sudoers (5) file.
-          command.run!("/bin/cp", args: ["-pR", source, target], sudo: true)
+          ::Utils::Cp.copy_recursive(source, target, sudo: true, command:)
          source.rmtree
        end

@ -161,8 +161,8 @@ module Cask
        ohai "Backing #{self.class.english_name} '#{target.basename}' up to '#{source}'"
        source.dirname.mkpath

-        # We need to preserve extended attributes between copies.
-        command.run!("/bin/cp", args: ["-pR", target, source], sudo: !source.parent.writable?)
+        # `Utils::Cp` preserves extended attributes between copies.
+        ::Utils::Cp.copy_recursive(target, source, sudo: !source.parent.writable?, command:)

        delete(target, force:, command:, **options)
      end
--- a/Library/Homebrew/dev-cmd/bottle.rb
+++ b/Library/Homebrew/dev-cmd/bottle.rb
@ -11,6 +11,7 @@ require "keg"
 require "formula_versions"
 require "utils/inreplace"
 require "erb"
+require "utils/cp"
 require "utils/gzip"
 require "api"
 require "extend/hash/deep_merge"
@ -767,7 +768,7 @@ module Homebrew
              all_bottle_hash = { formula_name => all_bottle_formula_hash }

              puts "Copying #{filename} to #{all_filename}" if args.verbose?
-              FileUtils.cp filename.to_s, all_filename.to_s
+              Utils::Cp.copy filename.to_s, all_filename.to_s

              puts "Writing #{all_filename.json}" if args.verbose?
              all_local_json_path = Pathname(all_filename.json)
--- a/Library/Homebrew/extend/pathname.rb
+++ b/Library/Homebrew/extend/pathname.rb
@ -6,6 +6,7 @@ require "resource"
 require "metafiles"
 require "extend/file/atomic"
 require "system_command"
+require "utils/cp"

 module DiskUsageExtension
  sig { returns(Integer) }
@ -226,7 +227,7 @@ class Pathname
    else
      dst.dirname.mkpath
      dst = yield(self, dst) if block_given?
-      FileUtils.cp(self, dst)
+      Utils::Cp.copy(self, dst)
    end
  end

--- a/Library/Homebrew/test/cask/artifact/app_spec.rb
+++ b/Library/Homebrew/test/cask/artifact/app_spec.rb
@ -350,8 +350,9 @@ RSpec.describe Cask::Artifact::App, :cask do
        allow(command).to receive(:run!).with(any_args).and_call_original

        expect(command).to receive(:run!)
-          .with("/bin/cp", args: ["-pR", source_contents_path, target_path],
-                           sudo: true)
+          .with(a_string_ending_with("cp"),
+                hash_including(args: include(source_contents_path, target_path),
+                               sudo: true))
          .and_call_original
        expect(FileUtils).not_to receive(:move).with(source_contents_path, an_instance_of(Pathname))

--- a/Library/Homebrew/unpack_strategy/bzip2.rb
+++ b/Library/Homebrew/unpack_strategy/bzip2.rb
@ -1,6 +1,8 @@
 # typed: true
 # frozen_string_literal: true

+require "utils/cp"
+
 module UnpackStrategy
  # Strategy for unpacking bzip2 archives.
  class Bzip2
@ -19,7 +21,7 @@ module UnpackStrategy

    sig { override.params(unpack_dir: Pathname, basename: Pathname, verbose: T::Boolean).returns(T.untyped) }
    def extract_to_dir(unpack_dir, basename:, verbose:)
-      FileUtils.cp path, unpack_dir/basename, preserve: true
+      Utils::Cp.copy path, unpack_dir/basename
      quiet_flags = verbose ? [] : ["-q"]
      system_command! "bunzip2",
                      args:    [*quiet_flags, unpack_dir/basename],
--- a/Library/Homebrew/unpack_strategy/directory.rb
+++ b/Library/Homebrew/unpack_strategy/directory.rb
@ -1,6 +1,8 @@
 # typed: true
 # frozen_string_literal: true

+require "utils/cp"
+
 module UnpackStrategy
  # Strategy for unpacking directories.
  class Directory
@ -20,10 +22,9 @@ module UnpackStrategy
    sig { override.params(unpack_dir: Pathname, basename: Pathname, verbose: T::Boolean).returns(T.untyped) }
    def extract_to_dir(unpack_dir, basename:, verbose:)
      path.children.each do |child|
-        system_command! "cp",
-                        args:    ["-pR", (child.directory? && !child.symlink?) ? "#{child}/." : child,
-                                  unpack_dir/child.basename],
-                        verbose:
+        Utils::Cp.copy_recursive (child.directory? && !child.symlink?) ? "#{child}/." : child,
+                                 unpack_dir/child.basename,
+                                 verbose:
      end
    end
  end
--- a/Library/Homebrew/unpack_strategy/gzip.rb
+++ b/Library/Homebrew/unpack_strategy/gzip.rb
@ -1,6 +1,8 @@
 # typed: true
 # frozen_string_literal: true

+require "utils/cp"
+
 module UnpackStrategy
  # Strategy for unpacking gzip archives.
  class Gzip
@ -19,7 +21,7 @@ module UnpackStrategy

    sig { override.params(unpack_dir: Pathname, basename: Pathname, verbose: T::Boolean).returns(T.untyped) }
    def extract_to_dir(unpack_dir, basename:, verbose:)
-      FileUtils.cp path, unpack_dir/basename, preserve: true
+      Utils::Cp.copy path, unpack_dir/basename
      quiet_flags = verbose ? [] : ["-q"]
      system_command! "gunzip",
                      args:    [*quiet_flags, "-N", "--", unpack_dir/basename],
--- a/Library/Homebrew/unpack_strategy/lzip.rb
+++ b/Library/Homebrew/unpack_strategy/lzip.rb
@ -1,6 +1,8 @@
 # typed: true
 # frozen_string_literal: true

+require "utils/cp"
+
 module UnpackStrategy
  # Strategy for unpacking lzip archives.
  class Lzip
@ -23,7 +25,7 @@ module UnpackStrategy

    sig { override.params(unpack_dir: Pathname, basename: Pathname, verbose: T::Boolean).returns(T.untyped) }
    def extract_to_dir(unpack_dir, basename:, verbose:)
-      FileUtils.cp path, unpack_dir/basename, preserve: true
+      Utils::Cp.copy path, unpack_dir/basename
      quiet_flags = verbose ? [] : ["-q"]
      system_command! "lzip",
                      args:    ["-d", *quiet_flags, unpack_dir/basename],
--- a/Library/Homebrew/unpack_strategy/lzma.rb
+++ b/Library/Homebrew/unpack_strategy/lzma.rb
@ -1,6 +1,8 @@
 # typed: true
 # frozen_string_literal: true

+require "utils/cp"
+
 module UnpackStrategy
  # Strategy for unpacking LZMA archives.
  class Lzma
@ -23,7 +25,7 @@ module UnpackStrategy

    sig { override.params(unpack_dir: Pathname, basename: Pathname, verbose: T::Boolean).returns(T.untyped) }
    def extract_to_dir(unpack_dir, basename:, verbose:)
-      FileUtils.cp path, unpack_dir/basename, preserve: true
+      Utils::Cp.copy path, unpack_dir/basename
      quiet_flags = verbose ? [] : ["-q"]
      system_command! "unlzma",
                      args:    [*quiet_flags, "--", unpack_dir/basename],
--- a/Library/Homebrew/unpack_strategy/uncompressed.rb
+++ b/Library/Homebrew/unpack_strategy/uncompressed.rb
@ -1,6 +1,8 @@
 # typed: true
 # frozen_string_literal: true

+require "utils/cp"
+
 module UnpackStrategy
  # Strategy for unpacking uncompressed files.
  class Uncompressed
@ -22,7 +24,7 @@ module UnpackStrategy

    sig { override.params(unpack_dir: Pathname, basename: Pathname, verbose: T::Boolean).returns(T.untyped) }
    def extract_to_dir(unpack_dir, basename:, verbose: false)
-      FileUtils.cp path, unpack_dir/basename.sub(/^[\da-f]{64}--/, ""), preserve: true, verbose:
+      Utils::Cp.copy path, unpack_dir/basename.sub(/^[\da-f]{64}--/, ""), verbose:
    end
  end
 end
--- a/Library/Homebrew/unpack_strategy/xz.rb
+++ b/Library/Homebrew/unpack_strategy/xz.rb
@ -1,6 +1,8 @@
 # typed: true
 # frozen_string_literal: true

+require "utils/cp"
+
 module UnpackStrategy
  # Strategy for unpacking xz archives.
  class Xz
@ -23,7 +25,7 @@ module UnpackStrategy

    sig { override.params(unpack_dir: Pathname, basename: Pathname, verbose: T::Boolean).returns(T.untyped) }
    def extract_to_dir(unpack_dir, basename:, verbose:)
-      FileUtils.cp path, unpack_dir/basename, preserve: true
+      Utils::Cp.copy path, unpack_dir/basename
      quiet_flags = verbose ? [] : ["-q"]
      system_command! "unxz",
                      args:    [*quiet_flags, "-T0", "--", unpack_dir/basename],
--- a/Library/Homebrew/unpack_strategy/zstd.rb
+++ b/Library/Homebrew/unpack_strategy/zstd.rb
@ -1,6 +1,8 @@
 # typed: true
 # frozen_string_literal: true

+require "utils/cp"
+
 module UnpackStrategy
  # Strategy for unpacking zstd archives.
  class Zstd
@ -23,7 +25,7 @@ module UnpackStrategy

    sig { override.params(unpack_dir: Pathname, basename: Pathname, verbose: T::Boolean).returns(T.untyped) }
    def extract_to_dir(unpack_dir, basename:, verbose:)
-      FileUtils.cp path, unpack_dir/basename, preserve: true
+      Utils::Cp.copy path, unpack_dir/basename
      quiet_flags = verbose ? [] : ["-q"]
      system_command! "unzstd",
                      args:    [*quiet_flags, "-T0", "--rm", "--", unpack_dir/basename],