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

github_packages: create hard link instead of copying #15553

Merged
merged 1 commit into from Jun 19, 2023

Conversation

carlocab
Copy link
Member

  • 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?

Some bottles are quite large, and the copies can make us run out of
space rather quickly. Let's try to avoid that by using hard links
instead of copies.

One case worth thinking about that I don't handle here: what if the source and destination live in different file systems?

Some bottles are quite large, and the copies can make us run out of
space rather quickly. Let's try to avoid that by using hard links
instead of copies.
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.

Seems fine as long as it's tested/verified to produce the same end result (because this changes what we're actually uploading).

@Bo98
Copy link
Member

Bo98 commented Jun 16, 2023

One case worth thinking about that I don't handle here: what if the source and destination live in different file systems?

Is there a nice way to check for this? Sounds like it could be a conditional if it's not too complex.

But it sounds like both might(?) be relative to the working directory which should mean it is always the same file system, and thus not a problem.

@carlocab
Copy link
Member Author

carlocab commented Jun 17, 2023

Seems fine as long as it's tested/verified to produce the same end result (because this changes what we're actually uploading).

Could you elaborate on what you mean by this? Because one of my reasons for this change is that doing this doesn't change what we're uploading.

That said, I did try to look at what brew pr-upload --dry-run does before and after this change (see the old and new directories respectively:

Details

❯ exa -la -T
drwx------    - carlocab 17 Jun 20:40 .
drwxr-xr-x    - carlocab 17 Jun 20:45 ├── new
.rw-r--r--  519 carlocab 17 Jun 20:40 │  ├── brew-test-bot.xml
.rw-r--r--    0 carlocab 17 Jun 20:40 │  ├── skipped_or_failed_formulae-arm64_big_sur.txt
.rw-r--r--    0 carlocab 17 Jun 20:40 │  ├── skipped_or_failed_formulae-arm64_monterey.txt
.rw-r--r--    0 carlocab 17 Jun 20:40 │  ├── skipped_or_failed_formulae-arm64_ventura.txt
.rw-r--r--    0 carlocab 17 Jun 20:40 │  ├── skipped_or_failed_formulae-big_sur.txt
.rw-r--r--    0 carlocab 17 Jun 20:40 │  ├── skipped_or_failed_formulae-monterey.txt
.rw-r--r--    0 carlocab 17 Jun 20:40 │  ├── skipped_or_failed_formulae-ventura.txt
.rw-r--r--    0 carlocab 17 Jun 20:40 │  ├── skipped_or_failed_formulae-x86_64_linux.txt
drwxr-xr-x    - carlocab 17 Jun 20:45 │  ├── trurl--0.8
drwxr-xr-x    - carlocab 17 Jun 20:45 │  │  ├── blobs
drwxr-xr-x    - carlocab 17 Jun 20:45 │  │  │  └── sha256
.rw-r--r-- 3.0k carlocab 17 Jun 20:45 │  │  │     ├── 0d9042dfab01685867438615c02049d198c910cfb1c5c292e557fc8c2bcb6d59
.rw-r--r--  222 carlocab 17 Jun 20:45 │  │  │     ├── 0e4bab362d76690ba1527bb59e3473d24f092b01aa3a3a64a9f4ebbfa80cc6f1
.rw-r--r--  19k carlocab 17 Jun 20:40 │  │  │     ├── 01b3a92be04c378ca91e2af21b57ff4a566c73a6063b39484246931024d54544
.rw-r--r--  19k carlocab 17 Jun 20:40 │  │  │     ├── 1d858b704c8dd22820c2b067c6845075d0c25c12053a8512514e04f7a90e4ed2
.rw-r--r--  19k carlocab 17 Jun 20:40 │  │  │     ├── 3a52f96f3a26f0fa142d51a6a4846eaf4576c08662078806e41b8dcb185ca076
.rw-r--r-- 1.9k carlocab 17 Jun 20:45 │  │  │     ├── 3df6bf196fe6c75ba99e8d613f15946c0416079c28ffd6c882963ae12a0b81cf
.rw-r--r-- 3.0k carlocab 17 Jun 20:45 │  │  │     ├── 6ac5a40f086a1a441984b3686b0385f0b3021005958e6790181e6773a7469b0f
.rw-r--r--  228 carlocab 17 Jun 20:45 │  │  │     ├── 6f410717526149986a6742f4ddadad67e73eac7add336edc6acf92d38f72b93e
.rw-r--r--  222 carlocab 17 Jun 20:45 │  │  │     ├── 39eb9d832fab24c98bfa032ffff6580ec96e68f531ee92bd55e475b8efabe2c1
.rw-r--r--  222 carlocab 17 Jun 20:45 │  │  │     ├── 59be91e9bdfa81cfeee7ea37d7cc4e6cd73e7ae337d82f7499d007b5b67e1fbd
.rw-r--r-- 1.9k carlocab 17 Jun 20:45 │  │  │     ├── 87173cd2b977c6798c9b8e34f0192898bcf957328869e7695722a127058401e4
.rw-r--r--  22k carlocab 17 Jun 20:40 │  │  │     ├── 9789427f3efa65ee8794685624d56839e7c1602690fd19dbc1d4295e795806d3
.rw-r--r-- 3.0k carlocab 17 Jun 20:45 │  │  │     ├── 511051588c5fe4147c80062246ae947b31a57b2f9244866995c8b5d72a71c158
.rw-r--r--  222 carlocab 17 Jun 20:45 │  │  │     ├── 50913523775d0123d07d96973c5303d29b3152516e38e8f4bb6900e91e97cd5a
.rw-r--r--  13k carlocab 17 Jun 20:45 │  │  │     ├── a489ec8f8cb90953cf0c7c7c12dab5a3bb6d957384d209e9bf31129ceb6247e8
.rw-r--r--  18k carlocab 17 Jun 20:40 │  │  │     ├── a723fa5853378509b04ae2d731d121ebac0165188ad1b05e11c8641a051aa78b
.rw-r--r--  222 carlocab 17 Jun 20:45 │  │  │     ├── aaee74a01c1492f55a56a5625ee56a414f0637da075903c56acb907378e4d25c
.rw-r--r--  222 carlocab 17 Jun 20:45 │  │  │     ├── b7ceea304b97a39afbe1addefcbc5693fc550277cd4b4a67a8c0c92ac5376602
.rw-r--r-- 3.0k carlocab 17 Jun 20:45 │  │  │     ├── c5a6de9123c382114be9c2125e72b24ad2ddd96a63b2ac8cf5ca0ed1f130edd8
.rw-r--r--  18k carlocab 17 Jun 20:40 │  │  │     ├── c73dec1c919874c01b4fc7de405d6e0ff9977d27fe27208f8a74888905717813
.rw-r--r-- 3.6k carlocab 17 Jun 20:45 │  │  │     ├── ef744b0181a1405d0431e5378edd42504de6cc70718c8e49beba160a71b4fbe3
.rw-r--r--  18k carlocab 17 Jun 20:40 │  │  │     └── f5b8f4f64143dde416e56f23b4457a503f626dc7debbd6f9de72082c645ede4b
.rw-r--r--  314 carlocab 17 Jun 20:45 │  │  ├── index.json
.rw-r--r--   35 carlocab 17 Jun 20:45 │  │  └── oci-layout
.rw-r--r-- 3.6k carlocab 17 Jun 20:40 │  ├── trurl--0.8.arm64_big_sur.bottle.json
.rw-r--r--  19k carlocab 17 Jun 20:40 │  ├── trurl--0.8.arm64_big_sur.bottle.tar.gz
.rw-r--r-- 3.6k carlocab 17 Jun 20:40 │  ├── trurl--0.8.arm64_monterey.bottle.json
.rw-r--r--  18k carlocab 17 Jun 20:40 │  ├── trurl--0.8.arm64_monterey.bottle.tar.gz
.rw-r--r-- 1.5k carlocab 17 Jun 20:40 │  ├── trurl--0.8.arm64_ventura.bottle.json
.rw-r--r--  18k carlocab 17 Jun 20:40 │  ├── trurl--0.8.arm64_ventura.bottle.tar.gz
.rw-r--r-- 3.6k carlocab 17 Jun 20:40 │  ├── trurl--0.8.big_sur.bottle.json
.rw-r--r--  19k carlocab 17 Jun 20:40 │  ├── trurl--0.8.big_sur.bottle.tar.gz
.rw-r--r-- 3.6k carlocab 17 Jun 20:40 │  ├── trurl--0.8.monterey.bottle.json
.rw-r--r--  19k carlocab 17 Jun 20:40 │  ├── trurl--0.8.monterey.bottle.tar.gz
.rw-r--r-- 1.5k carlocab 17 Jun 20:40 │  ├── trurl--0.8.ventura.bottle.json
.rw-r--r--  18k carlocab 17 Jun 20:40 │  ├── trurl--0.8.ventura.bottle.tar.gz
.rw-r--r-- 4.5k carlocab 17 Jun 20:40 │  ├── trurl--0.8.x86_64_linux.bottle.json
.rw-r--r--  22k carlocab 17 Jun 20:40 │  └── trurl--0.8.x86_64_linux.bottle.tar.gz
drwxr-xr-x    - carlocab 17 Jun 20:44 └── old
.rw-r--r--  519 carlocab 17 Jun 20:40    ├── brew-test-bot.xml
.rw-r--r--    0 carlocab 17 Jun 20:40    ├── skipped_or_failed_formulae-arm64_big_sur.txt
.rw-r--r--    0 carlocab 17 Jun 20:40    ├── skipped_or_failed_formulae-arm64_monterey.txt
.rw-r--r--    0 carlocab 17 Jun 20:40    ├── skipped_or_failed_formulae-arm64_ventura.txt
.rw-r--r--    0 carlocab 17 Jun 20:40    ├── skipped_or_failed_formulae-big_sur.txt
.rw-r--r--    0 carlocab 17 Jun 20:40    ├── skipped_or_failed_formulae-monterey.txt
.rw-r--r--    0 carlocab 17 Jun 20:40    ├── skipped_or_failed_formulae-ventura.txt
.rw-r--r--    0 carlocab 17 Jun 20:40    ├── skipped_or_failed_formulae-x86_64_linux.txt
drwxr-xr-x    - carlocab 17 Jun 20:44    ├── trurl--0.8
drwxr-xr-x    - carlocab 17 Jun 20:44    │  ├── blobs
drwxr-xr-x    - carlocab 17 Jun 20:44    │  │  └── sha256
.rw-r--r-- 3.0k carlocab 17 Jun 20:44    │  │     ├── 0d9042dfab01685867438615c02049d198c910cfb1c5c292e557fc8c2bcb6d59
.rw-r--r--  222 carlocab 17 Jun 20:44    │  │     ├── 0e4bab362d76690ba1527bb59e3473d24f092b01aa3a3a64a9f4ebbfa80cc6f1
.rw-r--r--  19k carlocab 17 Jun 20:44    │  │     ├── 01b3a92be04c378ca91e2af21b57ff4a566c73a6063b39484246931024d54544
.rw-r--r--  19k carlocab 17 Jun 20:44    │  │     ├── 1d858b704c8dd22820c2b067c6845075d0c25c12053a8512514e04f7a90e4ed2
.rw-r--r--  19k carlocab 17 Jun 20:44    │  │     ├── 3a52f96f3a26f0fa142d51a6a4846eaf4576c08662078806e41b8dcb185ca076
.rw-r--r-- 1.9k carlocab 17 Jun 20:44    │  │     ├── 3df6bf196fe6c75ba99e8d613f15946c0416079c28ffd6c882963ae12a0b81cf
.rw-r--r-- 3.0k carlocab 17 Jun 20:44    │  │     ├── 6ac5a40f086a1a441984b3686b0385f0b3021005958e6790181e6773a7469b0f
.rw-r--r--  228 carlocab 17 Jun 20:44    │  │     ├── 6f410717526149986a6742f4ddadad67e73eac7add336edc6acf92d38f72b93e
.rw-r--r--  222 carlocab 17 Jun 20:44    │  │     ├── 39eb9d832fab24c98bfa032ffff6580ec96e68f531ee92bd55e475b8efabe2c1
.rw-r--r--  222 carlocab 17 Jun 20:44    │  │     ├── 59be91e9bdfa81cfeee7ea37d7cc4e6cd73e7ae337d82f7499d007b5b67e1fbd
.rw-r--r-- 1.9k carlocab 17 Jun 20:44    │  │     ├── 87173cd2b977c6798c9b8e34f0192898bcf957328869e7695722a127058401e4
.rw-r--r--  22k carlocab 17 Jun 20:44    │  │     ├── 9789427f3efa65ee8794685624d56839e7c1602690fd19dbc1d4295e795806d3
.rw-r--r-- 3.0k carlocab 17 Jun 20:44    │  │     ├── 511051588c5fe4147c80062246ae947b31a57b2f9244866995c8b5d72a71c158
.rw-r--r--  222 carlocab 17 Jun 20:44    │  │     ├── 50913523775d0123d07d96973c5303d29b3152516e38e8f4bb6900e91e97cd5a
.rw-r--r--  13k carlocab 17 Jun 20:44    │  │     ├── a489ec8f8cb90953cf0c7c7c12dab5a3bb6d957384d209e9bf31129ceb6247e8
.rw-r--r--  18k carlocab 17 Jun 20:44    │  │     ├── a723fa5853378509b04ae2d731d121ebac0165188ad1b05e11c8641a051aa78b
.rw-r--r--  222 carlocab 17 Jun 20:44    │  │     ├── aaee74a01c1492f55a56a5625ee56a414f0637da075903c56acb907378e4d25c
.rw-r--r--  222 carlocab 17 Jun 20:44    │  │     ├── b7ceea304b97a39afbe1addefcbc5693fc550277cd4b4a67a8c0c92ac5376602
.rw-r--r-- 3.0k carlocab 17 Jun 20:44    │  │     ├── c5a6de9123c382114be9c2125e72b24ad2ddd96a63b2ac8cf5ca0ed1f130edd8
.rw-r--r--  18k carlocab 17 Jun 20:44    │  │     ├── c73dec1c919874c01b4fc7de405d6e0ff9977d27fe27208f8a74888905717813
.rw-r--r-- 3.6k carlocab 17 Jun 20:44    │  │     ├── ef744b0181a1405d0431e5378edd42504de6cc70718c8e49beba160a71b4fbe3
.rw-r--r--  18k carlocab 17 Jun 20:44    │  │     └── f5b8f4f64143dde416e56f23b4457a503f626dc7debbd6f9de72082c645ede4b
.rw-r--r--  314 carlocab 17 Jun 20:44    │  ├── index.json
.rw-r--r--   35 carlocab 17 Jun 20:44    │  └── oci-layout
.rw-r--r-- 3.6k carlocab 17 Jun 20:40    ├── trurl--0.8.arm64_big_sur.bottle.json
.rw-r--r--  19k carlocab 17 Jun 20:40    ├── trurl--0.8.arm64_big_sur.bottle.tar.gz
.rw-r--r-- 3.6k carlocab 17 Jun 20:40    ├── trurl--0.8.arm64_monterey.bottle.json
.rw-r--r--  18k carlocab 17 Jun 20:40    ├── trurl--0.8.arm64_monterey.bottle.tar.gz
.rw-r--r-- 1.5k carlocab 17 Jun 20:40    ├── trurl--0.8.arm64_ventura.bottle.json
.rw-r--r--  18k carlocab 17 Jun 20:40    ├── trurl--0.8.arm64_ventura.bottle.tar.gz
.rw-r--r-- 3.6k carlocab 17 Jun 20:40    ├── trurl--0.8.big_sur.bottle.json
.rw-r--r--  19k carlocab 17 Jun 20:40    ├── trurl--0.8.big_sur.bottle.tar.gz
.rw-r--r-- 3.6k carlocab 17 Jun 20:40    ├── trurl--0.8.monterey.bottle.json
.rw-r--r--  19k carlocab 17 Jun 20:40    ├── trurl--0.8.monterey.bottle.tar.gz
.rw-r--r-- 1.5k carlocab 17 Jun 20:40    ├── trurl--0.8.ventura.bottle.json
.rw-r--r--  18k carlocab 17 Jun 20:40    ├── trurl--0.8.ventura.bottle.tar.gz
.rw-r--r-- 4.5k carlocab 17 Jun 20:40    ├── trurl--0.8.x86_64_linux.bottle.json
.rw-r--r--  22k carlocab 17 Jun 20:40    └── trurl--0.8.x86_64_linux.bottle.tar.gz

We can also check the output of diff -r:

❯ diff -r new/trurl--0.8/ old/trurl--0.8/; echo $?
0
❯ diff -r old new; echo $?
0

However, the new directory is substantially smaller:

❯ du -hc -d1
404K    ./old
260K    ./new
664K    .
664K    total

This should mean that nothing important has changed other than the size of the working directory. But let me know if I should be looking out for anything else.

Is there a nice way to check for this? Sounds like it could be a conditional if it's not too complex.

I haven't looked, but I was thinking the same thing too.

But it sounds like both might(?) be relative to the working directory which should mean it is always the same file system, and thus not a problem.

Yes, you're right, it does look like everything happens in the current working directory so the hard links should be fine.

@MikeMcQuaid
Copy link
Member

That said, I did try to look at what brew pr-upload --dry-run does before and after this change (see the old and new directories respectively:

As you've checked: that's more than enough for me, thanks!

@MikeMcQuaid MikeMcQuaid merged commit 6763d11 into Homebrew:master Jun 19, 2023
24 checks passed
@MikeMcQuaid
Copy link
Member

Thanks again @carlocab!

@carlocab carlocab deleted the s/cp/ln branch June 19, 2023 13:07
@github-actions github-actions bot added the outdated PR was locked due to age label Jul 20, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated PR was locked due to age
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants