Skip to content
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

build.rs: print rustc stderr if exit status != 0 #3180

Merged
merged 1 commit into from
Apr 4, 2023

Conversation

jsha
Copy link
Contributor

@jsha jsha commented Apr 1, 2023

I was trying to run benchmarks locally with rustc-perf and found that many of them failed to build with a message from libc's build.rs "Failed to get rustc version." I made this change locally to help debug, and I think it would be generally useful. In my case it quickly revealed that rustc was failing to find libLLVM and so rustc --version was emitting nothing on stdout.

I think this may have been part of what was intended with #3000 and might help debug rust-lang/crater#663.

@rustbot
Copy link
Collaborator

rustbot commented Apr 1, 2023

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @JohnTitor (or someone else) soon.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue

@JohnTitor
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Apr 3, 2023

📌 Commit 88e27b8 has been approved by JohnTitor

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Apr 3, 2023

⌛ Testing commit 88e27b8 with merge 557f644...

bors added a commit that referenced this pull request Apr 3, 2023
build.rs: print rustc stderr if exit status != 0

I was trying to run benchmarks locally with rustc-perf and found that many of them failed to build with a message from libc's build.rs "Failed to get rustc version." I made this change locally to help debug, and I think it would be generally useful. In my case it quickly revealed that rustc was failing to find libLLVM and so `rustc --version` was emitting nothing on stdout.

I think this may have been part of what was intended with #3000 and might help debug rust-lang/crater#663.
@bors
Copy link
Contributor

bors commented Apr 3, 2023

💔 Test failed - checks-actions

build.rs Outdated
@@ -181,6 +181,11 @@ fn rustc_minor_nightly() -> (u32, bool) {
.output()
.ok()
.expect("Failed to get rustc version");
if !output.status.success() {
let stderr = std::string::String::from_utf8_lossy(output.stderr.as_slice());
eprintln!("failed to run rustc: {stderr}");
Copy link
Member

Choose a reason for hiding this comment

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

This syntax doesn't work on old rustc.

@jsha jsha force-pushed the print-rustc-error-output branch from 88e27b8 to 52808ce Compare April 3, 2023 20:47
@JohnTitor
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Apr 3, 2023

📌 Commit 52808ce has been approved by JohnTitor

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Apr 3, 2023

⌛ Testing commit 52808ce with merge f3d7607...

bors added a commit that referenced this pull request Apr 3, 2023
build.rs: print rustc stderr if exit status != 0

I was trying to run benchmarks locally with rustc-perf and found that many of them failed to build with a message from libc's build.rs "Failed to get rustc version." I made this change locally to help debug, and I think it would be generally useful. In my case it quickly revealed that rustc was failing to find libLLVM and so `rustc --version` was emitting nothing on stdout.

I think this may have been part of what was intended with #3000 and might help debug rust-lang/crater#663.
@bors
Copy link
Contributor

bors commented Apr 3, 2023

💔 Test failed - checks-actions

@jsha
Copy link
Contributor Author

jsha commented Apr 3, 2023

It looks like the nightly freebsd tests are failing with:

---- t::test_cmsg_nxthdr stdout ----
thread 't::test_cmsg_nxthdr' panicked at 'misaligned pointer dereference: address must be a multiple of 0x4 but is 0x7fffdf9fa4d1', libc-test/test/cmsg.rs:74:25
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    t::test_cmsg_nxthdr

Which I suspect is unrelated to this change.

@JohnTitor
Copy link
Member

Note that you saw the build on PRs, not bors. The bors' build is here: https://cirrus-ci.com/build/5602401026048000
The FreeBSD failure happens on your PR because your branch is old, it's fixed on master so rebasing should resolve it.
The actual error is here (you can find the link from bors comment): https://github.com/rust-lang/libc/actions/runs/4601182601/jobs/8128728585#step:4:237

It's a network failure which means we should: @bors retry

@bors
Copy link
Contributor

bors commented Apr 4, 2023

⌛ Testing commit 52808ce with merge b9f0372...

@bors
Copy link
Contributor

bors commented Apr 4, 2023

☀️ Test successful - checks-actions, checks-cirrus-freebsd-12, checks-cirrus-freebsd-13, checks-cirrus-freebsd-14
Approved by: JohnTitor
Pushing b9f0372 to master...

@bors bors merged commit b9f0372 into rust-lang:master Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants