Merge pull request #17373 from tesaguri/cp-reflink

2024-06-13 08:57:20 +01:00 · 2024-06-13 08:57:20 +01:00 · 1e0add6d66
commit 1e0add6d66
parent c997fa5327 a30cd15a73
17 changed files with 296 additions and 19 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/copy"

 module Cask
  module Artifact
@ -108,8 +109,8 @@ 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::Copy.recursive_with_attributes(source.children, target,
+                                                    force_command: true, sudo: true, command:)
          end
          Quarantine.copy_xattrs(source, target, command:)
          source.rmtree
@ -118,7 +119,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::Copy.recursive_with_attributes(source, target, force_command: true, sudo: true, command:)
          source.rmtree
        end

@ -161,8 +162,9 @@ 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::Copy.recursive_with_attributes(target, source, sudo: !source.parent.writable?, command:,
+                                              # This is required to preserve extended attributes between copies.
+                                              force_command: true)

        delete(target, force:, command:, **options)
      end
--- a/Library/Homebrew/extend/os/copy.rb
+++ b/Library/Homebrew/extend/os/copy.rb
@ -0,0 +1,4 @@
+# typed: strict
+# frozen_string_literal: true
+
+require "extend/os/mac/utils/copy" if OS.mac?
--- a/Library/Homebrew/extend/os/mac/utils/copy.rb
+++ b/Library/Homebrew/extend/os/mac/utils/copy.rb
@ -0,0 +1,28 @@
+# typed: strict
+# frozen_string_literal: true
+
+module Utils
+  module Copy
+    class << self
+      module MacOSOverride
+        private
+
+        # Use the lightweight `clonefile(2)` syscall if applicable.
+        SONOMA_FLAGS = T.let(["-c"].freeze, T::Array[String])
+
+        sig { returns(T.nilable(T::Array[String])) }
+        def extra_flags
+          # The `cp` command on older macOS versions also had the `-c` option, but before Sonoma,
+          # the command would fail if the `clonefile` syscall isn't applicable (the underlying
+          # filesystem doesn't support the feature or the source and the target are on different
+          # filesystems).
+          return if MacOS.version < :sonoma
+
+          SONOMA_FLAGS
+        end
+      end
+
+      prepend MacOSOverride
+    end
+  end
+end
--- 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/copy"

 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::Copy.with_attributes(self, dst)
    end
  end

--- a/Library/Homebrew/sorbet/rbi/upstream.rbi
+++ b/Library/Homebrew/sorbet/rbi/upstream.rbi
@ -2,3 +2,23 @@

 # This file contains temporary definitions for fixes that have
 # been submitted upstream to https://github.com/sorbet/sorbet.
+
+# https://github.com/sorbet/sorbet/pull/7959
+module FileUtils
+  sig {
+    params(
+      src:                T.any(File, String, Pathname, T::Array[T.any(File, String, Pathname)]),
+      dest:               T.any(String, Pathname),
+      preserve:           T.nilable(T::Boolean),
+      noop:               T.nilable(T::Boolean),
+      verbose:            T.nilable(T::Boolean),
+      dereference_root:   T::Boolean,
+      remove_destination: T.nilable(T::Boolean),
+    ).returns(T.nilable(T::Array[String]))
+  }
+  def self.cp_r(src, dest, preserve: nil, noop: nil, verbose: nil, dereference_root: true, remove_destination: nil)
+    # XXX: This comment is a placeholder to suppress `Style/EmptyMethod` lint.
+    # Simply compacting the method definition in a single line would in turn trigger
+    # `Layout/LineLength`, driving `brew style --fix` to an infinite loop.
+  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/test/spec_helper.rb
+++ b/Library/Homebrew/test/spec_helper.rb
@ -310,6 +310,7 @@ end

 RSpec::Matchers.define_negated_matcher :not_to_output, :output
 RSpec::Matchers.alias_matcher :have_failed, :be_failed
+RSpec::Matchers.define_negated_matcher :exclude, :include

 # Match consecutive elements in an array.
 RSpec::Matchers.define :array_including_cons do |*cons|
--- a/Library/Homebrew/test/utils/copy_spec.rb
+++ b/Library/Homebrew/test/utils/copy_spec.rb
@ -0,0 +1,146 @@
+# frozen_string_literal: true
+
+require "system_command"
+require "utils/copy"
+
+RSpec.describe Utils::Copy do
+  let(:path) { Pathname(Dir.mktmpdir) }
+  let(:source) { path/"source" }
+  let(:target) { path/"target" }
+
+  RSpec.shared_examples "copies files" do |method_name|
+    context "when the source is a regular file" do
+      before do
+        source.write "foo"
+        FileUtils.touch source, mtime: 42
+      end
+
+      it "copies the file and preserves its attributes" do
+        expect(target.exist?).to be(false)
+
+        described_class.public_send(method_name, source, target)
+
+        expect(target.file?).to be(true)
+        expect(target.read).to eq(source.read)
+        expect(target.mtime).to eq(source.mtime)
+      end
+    end
+
+    context "when the source is a list of files and the target is a directory" do
+      let(:source) { [path/"file1", path/"file2"] }
+      let(:target_children) { [target/"file1", target/"file2"] }
+
+      before do
+        source.each do |source|
+          source.write("foo")
+          FileUtils.touch source, mtime: 42
+        end
+        target.mkpath
+      end
+
+      it "copies the files and preserves their attributes" do
+        expect(target_children.map(&:exist?)).to all be(false)
+
+        described_class.public_send(method_name, source, target)
+
+        expect(target_children.map(&:file?)).to all be(true)
+        target_children.zip(source) do |target, source|
+          expect(target.read).to eq(source.read)
+          expect(target.mtime).to eq(source.mtime)
+        end
+      end
+    end
+  end
+
+  RSpec.shared_context "with macOS version" do |version|
+    before do
+      allow(MacOS).to receive(:version).and_return(MacOSVersion.new(version))
+    end
+  end
+
+  RSpec.shared_examples ".*with_attributes" do |method_name, fileutils_method_name|
+    context "when running on macOS Sonoma or later", :needs_macos do
+      include_context "with macOS version", "14"
+
+      include_examples "copies files", method_name
+
+      it "executes `cp` command with `-c` flag" do
+        expect(SystemCommand).to receive(:run!).with(
+          a_string_ending_with("cp"),
+          hash_including(args: include("-c").and(end_with(source, target))),
+        )
+
+        described_class.public_send(method_name, source, target)
+      end
+    end
+
+    context "when running on Linux or macOS Ventura or earlier" do
+      include_context "with macOS version", "13" if OS.mac?
+
+      include_examples "copies files", method_name
+
+      it "uses `FileUtils.#{fileutils_method_name}`" do
+        expect(SystemCommand).not_to receive(:run!)
+        expect(FileUtils).to receive(fileutils_method_name).with(source, target, hash_including(preserve: true))
+
+        described_class.public_send(method_name, source, target)
+      end
+
+      context "when `force_command` is set" do
+        it "executes `cp` command without `-c` flag" do
+          expect(SystemCommand).to receive(:run!).with(
+            a_string_ending_with("cp"),
+            hash_including(args: exclude("-c").and(end_with(source, target))),
+          )
+
+          described_class.public_send(method_name, source, target, force_command: true)
+        end
+      end
+    end
+  end
+
+  describe ".with_attributes" do
+    include_examples ".*with_attributes", :with_attributes, :cp
+  end
+
+  describe ".recursive_with_attributes" do
+    RSpec.shared_examples "copies directory" do
+      context "when the source is a directory" do
+        before do
+          FileUtils.mkpath source, mode: 0742
+          (source/"child").tap do |child|
+            child.write "foo"
+            FileUtils.touch child, mtime: 42
+          end
+        end
+
+        it "copies the directory recursively and preserves its attributes" do
+          expect(target.exist?).to be(false)
+
+          described_class.recursive_with_attributes(source, target)
+
+          expect(target.directory?).to be(true)
+          expect(target.stat.mode).to be(source.stat.mode)
+
+          [source/"child", target/"child"].tap do |source, target|
+            expect(target.file?).to be(true)
+            expect(target.read).to eq(source.read)
+            expect(target.mtime).to eq(source.mtime)
+          end
+        end
+      end
+    end
+
+    include_examples ".*with_attributes", :recursive_with_attributes, :cp_r
+
+    context "when running on macOS Sonoma or later", :needs_macos do
+      include_context "with macOS version", "14"
+      include_examples "copies directory"
+    end
+
+    context "when running on Linux or macOS Ventura or earlier" do
+      include_context "with macOS version", "13" if OS.mac?
+      include_examples "copies directory"
+    end
+  end
+end
--- 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/copy"
+
 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::Copy.with_attributes 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/copy"
+
 module UnpackStrategy
  # Strategy for unpacking directories.
  class Directory
@ -20,10 +22,10 @@ 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::Copy.recursive_with_attributes (child.directory? && !child.symlink?) ? "#{child}/." : child,
+                                              unpack_dir/child.basename,
+                                              force_command: true,
+                                              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/copy"
+
 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::Copy.with_attributes 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/copy"
+
 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::Copy.with_attributes 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/copy"
+
 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::Copy.with_attributes 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: strict
 # frozen_string_literal: true

+require "utils/copy"
+
 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::Copy.with_attributes 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/copy"
+
 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::Copy.with_attributes 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/copy"
+
 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::Copy.with_attributes path, unpack_dir/basename
      quiet_flags = verbose ? [] : ["-q"]
      system_command! "unzstd",
                      args:    [*quiet_flags, "-T0", "--rm", "--", unpack_dir/basename],
--- a/Library/Homebrew/utils/copy.rb
+++ b/Library/Homebrew/utils/copy.rb
@ -0,0 +1,58 @@
+# typed: true
+# frozen_string_literal: true
+
+require "extend/os/copy"
+require "fileutils"
+require "system_command"
+
+module Utils
+  # Helper functions for copying files.
+  module Copy
+    class << self
+      sig {
+        params(
+          source:        T.any(String, Pathname, T::Array[T.any(String, Pathname)]),
+          target:        T.any(String, Pathname),
+          force_command: T::Boolean,
+          sudo:          T::Boolean,
+          verbose:       T::Boolean,
+          command:       T.class_of(SystemCommand),
+        ).void
+      }
+      def with_attributes(source, target, force_command: false, sudo: false, verbose: false, command: SystemCommand)
+        if force_command || sudo || (flags = extra_flags)
+          command.run! "cp", args: ["-p", *flags, *source, target], sudo:, verbose:
+        else
+          FileUtils.cp source, target, preserve: true, verbose:
+        end
+
+        nil
+      end
+
+      sig {
+        params(
+          source:        T.any(String, Pathname, T::Array[T.any(String, Pathname)]),
+          target:        T.any(String, Pathname),
+          force_command: T::Boolean,
+          sudo:          T::Boolean,
+          verbose:       T::Boolean,
+          command:       T.class_of(SystemCommand),
+        ).void
+      }
+      def recursive_with_attributes(source, target, force_command: false, sudo: false, verbose: false,
+                                    command: SystemCommand)
+        if force_command || sudo || (flags = extra_flags)
+          command.run! "cp", args: ["-pR", *flags, *source, target], sudo:, verbose:
+        else
+          FileUtils.cp_r source, target, preserve: true, verbose:
+        end
+
+        nil
+      end
+
+      private
+
+      def extra_flags; end
+    end
+  end
+end