-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Remove libstdc++ version check error #126206
Conversation
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
…ck, r=<try> Remove libstdc++ version check error This keeps the error message from rust-lang#125411, but removes the `exit(1)` call. This PR is mostly a hotfix to unblock bootstrap benchmarks in rustc-perf. However, I think that it might be better to just print a warning, in general. If the ABI version does not match, the build might or might not work locally (as we can see on rustc-perf, where it works even if the reported ABI is 7). If it does not work (and **if** we can always recognize this during the LLVM wrapper build, instead of having some silent miscompilations), then the user will have to update their libstdc++ anyway, the error does not help them out on its own. So it should be enough to just provide a better error message, without blocking the build. But I'm not adamant on that, I just want to unblock bootstrap benchmarks until we can find a way to update libstdc++ on the collector machine. CC `@onur-ozkan` r? `@Mark-Simulacrum`
Makes sense, feel free to r=me. |
I'm generally sceptical of warnings that you can't silence and that aren't actually effective (i.e., maybe the right fix here is just to accept version 7 too if we think it works well enough). But for now we should fix the bug and iterate later, @bors r+ |
Oh, missed that you kicked off try.. @bors r- |
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
Finished benchmarking commit (f3d8cb4): comparison URL. Overall result: no relevant changes - no action neededBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. @bors rollup=never Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)This benchmark run did not return any relevant results for this metric. CyclesResults (primary -3.2%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: missing data |
Good, this fixed bootstrap. @bors r=Mark-Simulacrum |
☀️ Test successful - checks-actions |
Finished benchmarking commit (06194ca): comparison URL. Overall result: ✅ improvements - no action needed@rustbot label: -perf-regression Instruction countThis is a highly reliable metric that was used to determine the overall result at the top of this comment.
Max RSS (memory usage)Results (primary -4.4%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesThis benchmark run did not return any relevant results for this metric. Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: missing data |
This keeps the error message from #125411, but removes the
exit(1)
call.This PR is mostly a hotfix to unblock bootstrap benchmarks in rustc-perf.
However, I think that it might be better to just print a warning, in general. If the ABI version does not match, the build might or might not work locally (as we can see on rustc-perf, where it works even if the reported ABI is 7).
If it does not work (and if we can always recognize this during the LLVM wrapper build, instead of having some silent miscompilations), then the user will have to update their libstdc++ anyway, the error does not help them out on its own. So it should be enough to just provide a better error message, without blocking the build.
But I'm not adamant on that, I just want to unblock bootstrap benchmarks until we can find a way to update libstdc++ on the collector machine.
CC @onur-ozkan
r? @Mark-Simulacrum