Gracefully handle errors during a build. #8247
Merged
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
|
r? @Eh2406 (rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
Seams reasonable to me. |
src/cargo/core/compiler/job_queue.rs
Outdated
| @@ -651,25 +635,33 @@ impl<'cfg> DrainState<'cfg> { | |||
| // successful and otherwise wait for pending work to finish if it failed | |||
| // and then immediately return. | |||
| let mut error = None; | |||
| // CAUTION! From here on out, do not use `?`. Every error must be handled | |||
There was a problem hiding this comment.
Could this loop get factored into a function that doesn't return Result to statically prevent usage of ??
There was a problem hiding this comment.
Updated, WDYT? I assume Result will never be changed to implement Try such that it calls result.ok() to convert a Result to Option automatically?
|
@bors: r+ Looks great to me! I highly doubt that auto-conversion will ever be added, but even so we've got a nice comment explaining to be vigilant anyway |
|
📌 Commit 5e561ae has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-azure |
bors
added a commit
to rust-lang-ci/rust
that referenced
this pull request
May 20, 2020
Update cargo 9 commits in cb06cb2696df2567ce06d1a39b1b40612a29f853..500b2bd01c958f5a33b6aa3f080bea015877b83c 2020-05-08 21:57:44 +0000 to 2020-05-18 17:12:54 +0000 - Handle LTO with an rlib/cdylib crate type (rust-lang/cargo#8254) - Gracefully handle errors during a build. (rust-lang/cargo#8247) - Update `im-rc` to 15.0.0 (rust-lang/cargo#8255) - Fix `cargo update` with unused patch. (rust-lang/cargo#8243) - Rephrased error message for disallowed sections in virtual workspace (rust-lang/cargo#8200) - Ignore broken console output in some situations. (rust-lang/cargo#8236) - Expand error message to explain that a string was found (rust-lang/cargo#8235) - Add context to some fs errors. (rust-lang/cargo#8232) - Move SipHasher to an isolated module. (rust-lang/cargo#8233)
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.
If there are certain errors like EPIPE during a build, Cargo runs the risk of hanging if the compiler emits too many messages. This happens because Cargo now uses a bounded queue for compiler messages. However, if the main loop exits while the threads are running, there is nothing to drain the queue, and thus the threads will block indefinitely.
The solution here is to be extra careful of how errors are handled in the main loop. All errors are now treated roughly the same (report the error, allow the queue to continue to drain).
I've also tweaked things so the first error is reported, not the last.
Closes #8245