cargo_rustc: remove workaround for fixed upstream issue #4581
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tamird
referenced
this pull request
Oct 5, 2017
This commit is an architectural change inside of Cargo itself in the way that it handles the output format of builds. Previously when a build start, all existing directories and files would be renamed to `old-foo` folders. The build would then `rename` all files back into the right location as they were seen as fresh and needed for the build. The benefit of a system such as this is a rock-solid guarantee that the build tree contains exactly what it would if we were to start the build from a totally clean directory each time. There are some downsides, however: * In #800, it was discovered that this method has an unfortunate interaction with Docker. Docker apparently will mount many filesystems which `rename` will not work across. * I have seen countless flaky failures on windows due to an attempt to remove a file that was still in use somehow. I've never been able to truly track down why these failures are happening, however. The new system for managing output files is to build up a list of all known files at the start of a build, whitelist any necessary files when the build is being prepared, and then wipe out all unknown files right before the build begins. This is not quite as close to the guarantee as the benefits reaped before because on the second build all build files will still be in their final output locations, they may just get updated as part of the build as well. This seems like an acceptable compromise, however. Closes #800
|
While I think it's worth considering removing this block, I'm pretty certain it won't fix rust-lang/rust#44515 because this code path shouldn't get hit during testing, the compiler-bulitins crate should be fresh and avoid hitting rustc at all |
|
OK - now that rust-lang/rust#44515 is merged, this can be evaluated on its own. |
|
The comment in the middle I think is why this can't be removed. That was added after the first comment was originally added. |
|
@alexcrichton updated to keep the part added in 4b82fdc. |
Fixed in rust-lang/rust#25411. Also, the removed code is implicated in test failures observed in rust-lang/rust#44515.
|
@bors: r+ |
|
📌 Commit 8b68e59 has been approved by |
bors
added a commit
that referenced
this pull request
Oct 10, 2017
cargo_rustc: remove workaround for fixed upstream issue Fixed in rust-lang/rust#25411. Also, the removed code is implicated in test failures observed in rust-lang/rust#44515. r? @alexcrichton
|
☀️ Test successful - status-appveyor, status-travis |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixed in rust-lang/rust#25411. Also, the
removed code is implicated in test failures observed in
rust-lang/rust#44515.
r? @alexcrichton