Skip to content

rustbuild: with --no-fail-fast, report the specific commands that failed #44680

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

Merged
merged 1 commit into from
Sep 19, 2017

Conversation

infinity0
Copy link
Contributor

I'm not sure this is the most elegant way of doing it, I'm still a bit of a rust noob. I tried Vec<Command> and keeping Cell instead of RefCell but couldn't fight my way past the borrow errors, this was the first arrangement that I could make work.

@rust-highfive
Copy link
Contributor

r? @alexcrichton

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

@infinity0
Copy link
Contributor Author

r? @Mark-Simulacrum

cc @cuviper

@Mark-Simulacrum
Copy link
Member

Looks good to me. @bors r+ p=1 (regression)

@bors
Copy link
Collaborator

bors commented Sep 18, 2017

📌 Commit 435a0ea has been approved by Mark-Simulacrum

@@ -68,8 +68,8 @@ impl fmt::Display for TestKind {
fn try_run_expecting(build: &Build, cmd: &mut Command, expect: BuildExpectation) {
if !build.fail_fast {
if !build.try_run(cmd, expect) {
let failures = build.delayed_failures.get();
build.delayed_failures.set(failures + 1);
let mut failures = &mut build.delayed_failures.borrow_mut();
Copy link
Member

@kennytm kennytm Sep 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Could not build bootstrap. Please remove the &mut.

[00:01:54] error: variable does not need to be mutable
[00:01:54]   --> /checkout/src/bootstrap/check.rs:71:17
[00:01:54]    |
[00:01:54] 71 |             let mut failures = &mut build.delayed_failures.borrow_mut();
[00:01:54]    |                 ^^^^^^^^^^^^
[00:01:54]    |
[00:01:54] note: lint level defined here
[00:01:54]   --> /checkout/src/bootstrap/lib.rs:116:9
[00:01:54]    |
[00:01:54] 116| #![deny(warnings)]
[00:01:54]    |         ^^^^^^^^
[00:01:54]    = note: #[deny(unused_mut)] implied by #[deny(warnings)]
[00:01:54] 
[00:01:54] error: variable does not need to be mutable
[00:01:54]   --> /checkout/src/bootstrap/check.rs:86:17
[00:01:54]    |
[00:01:54] 86 |             let mut failures = &mut build.delayed_failures.borrow_mut();
[00:01:54]    |                 ^^^^^^^^^^^^
[00:01:54] 
[00:01:54] error: aborting due to 2 previous errors
[00:01:54] 
[00:01:54] error: Could not compile `bootstrap`.

@@ -83,8 +83,8 @@ fn try_run(build: &Build, cmd: &mut Command) {
fn try_run_quiet(build: &Build, cmd: &mut Command) {
if !build.fail_fast {
if !build.try_run_quiet(cmd) {
let failures = build.delayed_failures.get();
build.delayed_failures.set(failures + 1);
let mut failures = &mut build.delayed_failures.borrow_mut();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both done.

@alexcrichton
Copy link
Member

@bors: r=Mark-Simulacrum

@bors
Copy link
Collaborator

bors commented Sep 18, 2017

📌 Commit 8f25497 has been approved by Mark-Simulacrum

@bors
Copy link
Collaborator

bors commented Sep 18, 2017

⌛ Testing commit 8f25497 with merge 06bb0e0...

bors added a commit that referenced this pull request Sep 18, 2017
rustbuild: with --no-fail-fast, report the specific commands that failed

I'm not sure this is the most elegant way of doing it, I'm still a bit of a rust noob. I tried `Vec<Command>` and keeping `Cell` instead of `RefCell` but couldn't fight my way past the borrow errors, this was the first arrangement that I could make work.
@bors
Copy link
Collaborator

bors commented Sep 19, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: Mark-Simulacrum
Pushing 06bb0e0 to master...

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