Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

unpack_strategy/directory: try preserving hardlinks #18497

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

cho-m
Copy link
Member

@cho-m cho-m commented Oct 3, 2024

  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes? Here's an example.
  • Have you successfully run brew style with your changes locally?
  • Have you successfully run brew typecheck with your changes locally?
  • Have you successfully run brew tests with your changes locally?

Try running cp -al to preserve hardlinks on both macOS and Linux.

If that fails, fall back on cp -a which can preserve hardlinks on Linux (but not macOS) assuming target filesystem supports them.


Originally tried preserving hardlinks in #13154 but the situation with macOS commands was a bit of a pain (cp didn't work well, rsync was slow and had mtime issues, cpio was a pain to use and could lead to excessive memory as need to stdin every file, etc.).

Exploring this again to figure out a general solution for compacting bottle/install sizes in #18478. Given we now only build bottles for Ventura onward, the limitation on cp -l is less of an issue.

I think fallback on older macOS should be quick as cp will fail right away on illegal options, e.g.

cp -z
cp: illegal option -- z

There is a slow path if cp -a fails and falls back to cp -pR where:

  • In case of macOS, both would fail. Though I consider this similar to exception handling where this is a rare case and slowness is okay.
  • In case of Linux, there is an inconvenient point where -a (implicit -R) continues on failure so it adds overhead of adding then removing files. Again should be rare situation as Linux filesystems would usually support hardlinks. Not sure if possible but maybe could create a scenario of install brew on a USB drive.

Specific example is again trino but this time the bottle which has hardlinks:

Before:

==> Summary
🍺  /opt/homebrew/Cellar/trino/459: 6,310 files, 3.6GB
==> Running `brew cleanup trino`...
Disable this behaviour by setting HOMEBREW_NO_INSTALL_CLEANUP.
Hide these hints with HOMEBREW_NO_ENV_HINTS (see `man brew`).
==> Installation times
trino                    16.562 s

After:

==> Summary
🍺  /opt/homebrew/Cellar/trino/459: 6,310 files, 859.9MB
==> Running `brew cleanup trino`...
Disable this behaviour by setting HOMEBREW_NO_INSTALL_CLEANUP.
Hide these hints with HOMEBREW_NO_ENV_HINTS (see `man brew`).
==> Installation times
trino                    10.458 s

@cho-m cho-m force-pushed the preserve-hardlinks branch 2 times, most recently from 7e1bf89 to d805d10 Compare October 3, 2024 22:16
Try running `cp -al` to preserve hardlinks on both macOS and Linux.

If that fails, fall back on `cp -a` which can preserve hardlinks on
Linux (but not macOS) assuming target filesystem supports them.
@cho-m cho-m marked this pull request as ready for review October 4, 2024 00:32
@cho-m cho-m marked this pull request as draft October 4, 2024 01:09
@cho-m cho-m marked this pull request as ready for review October 4, 2024 01:12
Copy link
Member

@MikeMcQuaid MikeMcQuaid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @cho-m! Let's try as-is and can iterate more on master.

Library/Homebrew/unpack_strategy/directory.rb Show resolved Hide resolved
@MikeMcQuaid MikeMcQuaid merged commit c142f31 into master Oct 4, 2024
27 checks passed
@MikeMcQuaid MikeMcQuaid deleted the preserve-hardlinks branch October 4, 2024 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants