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

Use same-file to avoid unnecessary hard links #4390

Merged
merged 1 commit into from
Aug 10, 2017

Conversation

alexcrichton
Copy link
Member

This is targeted at removing the need for a workaround in rust-lang/rust#39518,
allowing the main rust build system to move back to hard links which should be
much more efficient.

This is targeted at removing the need for a workaround in rust-lang/rust#39518,
allowing the main rust build system to move back to hard links which should be
much more efficient.
@rust-highfive
Copy link

r? @matklad

(rust_highfive has picked a reviewer for you, use r? to override)

@Mark-Simulacrum
Copy link
Member

Looks good to me. Sorry for not getting to this myself.

@alexcrichton
Copy link
Member Author

No worries! I was halfway through thinking about the implementation and how to do it when I remembered the crate already existed :)

@matklad
Copy link
Member

matklad commented Aug 10, 2017

bors r+

@matklad
Copy link
Member

matklad commented Aug 10, 2017

@bors r+

@bors
Copy link
Contributor

bors commented Aug 10, 2017

📌 Commit 599db09 has been approved by matklad

@bors
Copy link
Contributor

bors commented Aug 10, 2017

⌛ Testing commit 599db09 with merge 8720d63...

bors added a commit that referenced this pull request Aug 10, 2017
Use `same-file` to avoid unnecessary hard links

This is targeted at removing the need for a workaround in rust-lang/rust#39518,
allowing the main rust build system to move back to hard links which should be
much more efficient.
@matklad
Copy link
Member

matklad commented Aug 10, 2017

In theory, it might be fun to think about comparing the contents of files as well, for potentially more fine grained caching, a-la compiler's red/green split: it might be the case that although inputs change, the result is the same.

@bors
Copy link
Contributor

bors commented Aug 10, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: matklad
Pushing 8720d63 to master...

@bors bors merged commit 599db09 into rust-lang:master Aug 10, 2017
@alexcrichton alexcrichton deleted the read-hard-links branch August 10, 2017 14:03
@alexcrichton
Copy link
Member Author

Heh I was actually just thinking about that yesterday as well! Definitely seems like a plausible idea!

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Sep 10, 2017
The `copy` function historically in rustbuild used hard links to speed up the
copy operations that it does. This logic was backed out, however, in rust-lang#39518 due
to a bug that only showed up on Windows, described in rust-lang#39504. The cause
described in rust-lang#39504 happened because Cargo, on a fresh build, would overwrite
the previous artifacts with new hard links that Cargo itself manages.

This behavior in Cargo was fixed in rust-lang/cargo#4390 where it no longer
should overwrite files on fresh builds, opportunistically leaving the filesystem
intact and not touching it.

Hopefully this can help speed up local builds by doing fewer copies all over the
place!
bors added a commit to rust-lang/rust that referenced this pull request Sep 10, 2017
rustbuild: Switch back to using hard links

The `copy` function historically in rustbuild used hard links to speed up the
copy operations that it does. This logic was backed out, however, in #39518 due
to a bug that only showed up on Windows, described in #39504. The cause
described in #39504 happened because Cargo, on a fresh build, would overwrite
the previous artifacts with new hard links that Cargo itself manages.

This behavior in Cargo was fixed in rust-lang/cargo#4390 where it no longer
should overwrite files on fresh builds, opportunistically leaving the filesystem
intact and not touching it.

Hopefully this can help speed up local builds by doing fewer copies all over the
place!
@ehuss ehuss added this to the 1.21.0 milestone Feb 6, 2022
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.

6 participants