-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Fix fix_path_deps test error. #6236
Conversation
r? @matklad (rust_highfive has picked a reviewer for you, use r? to override) |
This test has been failing a fair bit: https://ci.appveyor.com/project/rust-lang-libs/cargo/builds/17275967/job/a3g5pmwknyk7xiiq I can't repro locally, but this should work. There's a little race between which side says Checking vs Fixing first. An alternate solution might be to print diff --git a/src/cargo/core/compiler/job_queue.rs b/src/cargo/core/compiler/job_queue.rs
index 1bb654d1..5ee957d7 100644
--- a/src/cargo/core/compiler/job_queue.rs
+++ b/src/cargo/core/compiler/job_queue.rs
@@ -400,6 +400,12 @@ impl<'a> JobQueue<'a> {
let res = job.run(fresh, &JobState { tx: my_tx.clone() });
my_tx.send(Message::Finish(key, res)).unwrap();
};
+
+ if !build_plan {
+ // Print out some nice progress information
+ self.note_working_on(config, &key, fresh)?;
+ }
+
match fresh {
Freshness::Fresh => doit(),
Freshness::Dirty => {
@@ -407,11 +413,6 @@ impl<'a> JobQueue<'a> {
}
}
- if !build_plan {
- // Print out some nice progress information
- self.note_working_on(config, &key, fresh)?;
- }
-
Ok(())
} |
Ah yeah I think we probably want to guarantee that we print out the result first, want to go ahead and apply the alternative patch to ensure that status messages go out before we spawn work? |
2cfb2f1
to
52f45b5
Compare
Sure! Updated with the alternate approach. I agree it is better, I was just concerned of unexpected consequences. |
@bors: r+ |
📌 Commit 52f45b5 has been approved by |
⌛ Testing commit 52f45b5 with merge c067ab0f6b80962f6845e1866571fd88605aeba6... |
💔 Test failed - status-travis |
@bors: retry |
☀️ Test successful - status-appveyor, status-travis |
Fix fix_path_deps again. The previous fix in #6236 wasn't enough to make this test not fail. On Travis MacOS it fails about 5% of the time. Essentially the first "FIXING" line is sometimes delayed until after the second "CHECKING" line. The "FIXING" message is sent async to the diagnostic server, so I think it's just a race that doesn't have a good way to enforce. Oddly, I can only repro this on Travis. Error seen at: https://travis-ci.org/rust-lang/cargo/jobs/450956693
No description provided.