Skip to content

Commit

Permalink
unpack_strategy/directory: use mv for nested unpack
Browse files Browse the repository at this point in the history
`mv` should preserve hardlinks and allow faster unpack on the same
filesystem. A secondary pass is done with `cp` to copy over attributes
onto any existing directories.

We only run this for nested unpacks as most direct Directory strategy
usage is for repositories where moving files breaks existing code.

There are also potential user issues that could be due to Apple's
`cp -l` on specific macOS versions. Can consider re-adding `cp -l` with
improved fallback handling if there is noticeable advantages.
  • Loading branch information
cho-m committed Oct 19, 2024
1 parent 6f17b06 commit 9f79991
Show file tree
Hide file tree
Showing 3 changed files with 52 additions and 26 deletions.
4 changes: 2 additions & 2 deletions Library/Homebrew/test/unpack_strategy/directory_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@
expect(unpack_dir/"folderSymlink").to be_a_symlink
end

it "preserves hardlinks" do
strategy.extract(to: unpack_dir)
it "preserves hardlinks with move enabled" do
described_class.new(path, move: true).extract(to: unpack_dir)
expect((unpack_dir/"file").stat.ino).to eq (unpack_dir/"hardlink").stat.ino
end

Expand Down
2 changes: 1 addition & 1 deletion Library/Homebrew/unpack_strategy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ def extract_nestedly(to: nil, basename: nil, verbose: false, prioritize_extensio
FileUtils.chmod "u+w", path, verbose:
end

Directory.new(tmp_unpack_dir).extract(to:, verbose:)
Directory.new(tmp_unpack_dir, move: true).extract(to:, verbose:)
end
end

Expand Down
72 changes: 49 additions & 23 deletions Library/Homebrew/unpack_strategy/directory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,33 +16,59 @@ def self.can_extract?(path)
path.directory?
end

sig {
params(
path: T.any(String, Pathname),
ref_type: T.nilable(Symbol),
ref: T.nilable(String),
merge_xattrs: T::Boolean,
move: T::Boolean,
).void
}
def initialize(path, ref_type: nil, ref: nil, merge_xattrs: false, move: false)
super(path, ref_type:, ref:, merge_xattrs:)
@move = move
end

private

sig { override.params(unpack_dir: Pathname, basename: Pathname, verbose: T::Boolean).void }
def extract_to_dir(unpack_dir, basename:, verbose:)
path_children = path.children
return if path_children.empty?

existing = unpack_dir.children

# We run a few cp attempts in the following order:
#
# 1. Start with `-al` to create hardlinks rather than copying files if the source and
# target are on the same filesystem. On macOS, this is the only cp option that can
# preserve hardlinks but it is only available since macOS 12.3 (file_cmds-353.100.22).
# 2. Try `-a` as GNU `cp -a` preserves hardlinks. macOS `cp -a` is identical to `cp -pR`.
# 3. Fall back on `-pR` to handle the case where GNU `cp -a` failed. This may happen if
# installing into a filesystem that doesn't support hardlinks like an exFAT USB drive.
cp_arg_attempts = ["-a", "-pR"]
cp_arg_attempts.unshift("-al") if path.stat.dev == unpack_dir.stat.dev

cp_arg_attempts.each do |arg|
args = [arg, *path_children, unpack_dir]
must_succeed = print_stderr = (arg == cp_arg_attempts.last)
result = system_command("cp", args:, verbose:, must_succeed:, print_stderr:)
break if result.success?

FileUtils.rm_r(unpack_dir.children - existing)
move_to_dir(unpack_dir, verbose:) if @move

path.each_child do |child|
system_command! "cp",
args: ["-pR", (child.directory? && !child.symlink?) ? "#{child}/." : child,
unpack_dir/child.basename],
verbose:
end
end

# Move files and non-conflicting directories from `path` to `unpack_dir`
#
# @raise [RuntimeError] if moving a non-directory over an existing directory or vice versa
sig { params(unpack_dir: Pathname, verbose: T::Boolean).void }
def move_to_dir(unpack_dir, verbose:)
path.find(ignore_error: false) do |src|
next if src == path

dst = unpack_dir/src.relative_path_from(path)
if dst.exist?
dst_real_dir = dst.directory? && !dst.symlink?
src_real_dir = src.directory? && !src.symlink?

Check warning on line 58 in Library/Homebrew/unpack_strategy/directory.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/unpack_strategy/directory.rb#L58

Added line #L58 was not covered by tests
# Avoid trying to move a non-directory over an existing directory or vice versa.
# This is similar to `cp` which fails with errors like 'cp: <dst>: Is a directory'.
# However, unlike `cp`, this will fail early rather than at then end.
raise "Cannot move directory #{src} to non-directory #{dst}" if src_real_dir && !dst_real_dir
raise "Cannot move non-directory #{src} to directory #{dst}" if !src_real_dir && dst_real_dir
# Defer writing over existing directories. Handle this later on to copy attributes
next if dst_real_dir

FileUtils.rm(dst, verbose:)

Check warning on line 67 in Library/Homebrew/unpack_strategy/directory.rb

View check run for this annotation

Codecov / codecov/patch

Library/Homebrew/unpack_strategy/directory.rb#L67

Added line #L67 was not covered by tests
end

FileUtils.mv(src, dst, verbose:)
Find.prune
end
end
end
Expand Down

0 comments on commit 9f79991

Please sign in to comment.