diff --git a/Library/Homebrew/cask/artifact/moved.rb b/Library/Homebrew/cask/artifact/moved.rb index b6fe5c893f975..d22e4c76c08a0 100644 --- 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 @@ def move(adopt: false, force: false, verbose: false, predecessor: nil, reinstall 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 @@ def move(adopt: false, force: false, verbose: false, predecessor: nil, reinstall 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 @@ def move_back(skip: false, force: false, adopt: false, command: nil, **options) 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 diff --git a/Library/Homebrew/extend/os/copy.rb b/Library/Homebrew/extend/os/copy.rb new file mode 100644 index 0000000000000..59dcc7da706f0 --- /dev/null +++ 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? diff --git a/Library/Homebrew/extend/os/mac/utils/copy.rb b/Library/Homebrew/extend/os/mac/utils/copy.rb new file mode 100644 index 0000000000000..f262524c8a4c1 --- /dev/null +++ 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 diff --git a/Library/Homebrew/extend/pathname.rb b/Library/Homebrew/extend/pathname.rb index ec649fe0e42cf..d0defa91d4c68 100644 --- a/Library/Homebrew/extend/pathname.rb +++ b/Library/Homebrew/extend/pathname.rb @@ -6,6 +6,7 @@ require "metafiles" require "extend/file/atomic" require "system_command" +require "utils/copy" module DiskUsageExtension sig { returns(Integer) } @@ -226,7 +227,7 @@ def cp_path_sub(pattern, replacement) else dst.dirname.mkpath dst = yield(self, dst) if block_given? - FileUtils.cp(self, dst) + Utils::Copy.with_attributes(self, dst) end end diff --git a/Library/Homebrew/sorbet/rbi/upstream.rbi b/Library/Homebrew/sorbet/rbi/upstream.rbi index a00487f95aced..25724349e31e8 100644 --- 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 diff --git a/Library/Homebrew/test/cask/artifact/app_spec.rb b/Library/Homebrew/test/cask/artifact/app_spec.rb index 95cc8d9a8b446..e33d6732221d4 100644 --- a/Library/Homebrew/test/cask/artifact/app_spec.rb +++ b/Library/Homebrew/test/cask/artifact/app_spec.rb @@ -350,8 +350,9 @@ 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)) diff --git a/Library/Homebrew/test/spec_helper.rb b/Library/Homebrew/test/spec_helper.rb index 4287f45a1c340..ee82b05ad84f3 100644 --- a/Library/Homebrew/test/spec_helper.rb +++ b/Library/Homebrew/test/spec_helper.rb @@ -310,6 +310,7 @@ 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| diff --git a/Library/Homebrew/test/utils/copy_spec.rb b/Library/Homebrew/test/utils/copy_spec.rb new file mode 100644 index 0000000000000..9d9ee33c1f577 --- /dev/null +++ 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 diff --git a/Library/Homebrew/unpack_strategy/bzip2.rb b/Library/Homebrew/unpack_strategy/bzip2.rb index af1248c466f53..30ae554dff824 100644 --- 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 @@ def self.can_extract?(path) 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], diff --git a/Library/Homebrew/unpack_strategy/directory.rb b/Library/Homebrew/unpack_strategy/directory.rb index 90b5ade18e772..b8c841932ac07 100644 --- 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 @@ def self.can_extract?(path) 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 diff --git a/Library/Homebrew/unpack_strategy/gzip.rb b/Library/Homebrew/unpack_strategy/gzip.rb index 2ece0befdc456..c343a173a526e 100644 --- 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 @@ def self.can_extract?(path) 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], diff --git a/Library/Homebrew/unpack_strategy/lzip.rb b/Library/Homebrew/unpack_strategy/lzip.rb index 668aa4fdcfc18..f676e12509f84 100644 --- 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 @@ def dependencies 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], diff --git a/Library/Homebrew/unpack_strategy/lzma.rb b/Library/Homebrew/unpack_strategy/lzma.rb index d529e2de4c4f3..85c0898aa5dcb 100644 --- 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 @@ def dependencies 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], diff --git a/Library/Homebrew/unpack_strategy/uncompressed.rb b/Library/Homebrew/unpack_strategy/uncompressed.rb index fdde716dcbf4c..ab2ba36fbf15c 100644 --- 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/copy" + module UnpackStrategy # Strategy for unpacking uncompressed files. class Uncompressed @@ -22,7 +24,7 @@ def extract_nestedly(to: nil, basename: nil, verbose: false, prioritize_extensio 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 diff --git a/Library/Homebrew/unpack_strategy/xz.rb b/Library/Homebrew/unpack_strategy/xz.rb index 7ac0ceb4e6c38..77d4e85da765f 100644 --- 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 @@ def dependencies 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], diff --git a/Library/Homebrew/unpack_strategy/zstd.rb b/Library/Homebrew/unpack_strategy/zstd.rb index f8cdacb0abec3..4c2571df2d53c 100644 --- 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 @@ def dependencies 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], diff --git a/Library/Homebrew/utils/copy.rb b/Library/Homebrew/utils/copy.rb new file mode 100644 index 0000000000000..7833e69675d43 --- /dev/null +++ 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