-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix: Fix reporting of build script errors #10024
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 reporting of build script errors #10024
Conversation
| if let Some(err) = data.error() { | ||
| has_errors = true; | ||
| stdx::format_to!(buf, "{:#}\n", err); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was semi-intentional -- if cargo check fails because you have a type error in non-build.rs code, we should consider that as a working workspace. I think this is a non-issue given that we use RUSTC_WRAPPER trick, but it makes sense to re-check that you don't get partially loaded workspace notification
just becaues borrow checked doesn't favor you today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This -- the absense of errore reporting for the cases wheer cargo check run succesfully, but returned a non-zero status
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this still seems to work correctly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although rust-analyzer does not seem to recover when there is a build error in a build script, even after clicking on that item after fixing the issue
|
r=me |
|
bors r+ |

r? @matklad (mostly to double-check that the redundant code I removed was, in fact, redundant)
Fixes #9864
Fixes #10023