-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Rollup of 3 pull requests #112282
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
Rollup of 3 pull requests #112282
Conversation
rust-lld is not located in the same directory as the other binaries that point to ../lib, but in a deeper directory in lib. So we need to point a few layers up for rust-lld to find the LLVM shared library without rustup's LD_LIBRARY_PATH overrides.
On Windows this is sometimes not the case, for reasons I can't track down. This works around the problem, although I'm not sure how to confirm we're not generating invalid build metrics in this case.
rust-lld: add rpath entry to the correct `lib` folder An explanation, for our linux rustup toolchain: - `lld` / `rust-lld` is built as a regular LLVM tool, but is not distributed via the `llvm-tools` component. It's distributed by default, like a regular rust binary, like cargo and rustc. The general expected setup is: binaries in `bin` and libraries in `lib`, so the rpath we use for a `bin/$executable` is `$ORIGIN/../lib`. - However, `rust-lld` is _not_ in the same location as our other executables (`$root/bin`), it's in `$root/lib/rustlib/$host/bin/`. The current rpath thus expects the LLVM's shared library to be in `$root/lib/rustlib/$host/lib/`. - That .so is only present in `$root/lib`, causing rust-lang#80703. (LLVM's shared library is also copied to `$root/lib/rustlib/$host/lib/` with the `llvm-tools` component, so it also was [a workaround for the issue](rust-lang#80703 (comment))) rustup's `LD_LIBRARY_PATH` overrides made this discrepancy invisible when we switched to `llvm.link-shared = true`, and this only showed up when running `rustc` or `rust-lld`'s executables directly. To fix this we could: - copy the .so to this expected location all the time, but that seems wasteful. - or, add an rpath entry when building LLD, which seems preferable to me (but I don't know if it could cause issues). This PR does the latter, tweaking how bootstrap builds LLD to point to the expected directory, and fixes rust-lang#80703. (Since this is related to P-high issues about switching to lld by default, I'll cc `@petrochenkov` to keep them updated.)
…lor-11, r=notriddle Migrate GUI colors test to original CSS color format Follow-up of rust-lang#111459. The update for `browser-ui-test` version is because for hex color conversions, it used a precision of 1 instead of 2, which was problematic. r? `@notriddle`
Don't require the output from libtest to be valid UTF-8 On Windows this is sometimes not the case, for reasons I can't track down (maybe related to localization? the bug report had a french locale). This works around the problem, although I'm not sure how to confirm we're not generating invalid build metrics in this case. Fixes rust-lang#112273.
@bors r+ rollup=never p=4 |
☀️ Test successful - checks-actions |
1 similar comment
☀️ Test successful - checks-actions |
📌 Perf builds for each rolled up PR:
previous master: 101fa903bb In the case of a perf regression, run the following command for each PR you suspect might be the cause: |
📌 Perf builds for each rolled up PR:
previous master: dc25fbe984 In the case of a perf regression, run the following command for each PR you suspect might be the cause: |
Finished benchmarking commit (101fa90): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression 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. CyclesResultsThis 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: 646.023s -> 646.397s (0.06%) |
Successful merges:
lib
folder #112247 (rust-lld: add rpath entry to the correctlib
folder)Failed merges:
if let Some()
that always matches to variable #112251 (rustdoc: convertif let Some()
that always matches to variable)r? @ghost
@rustbot modify labels: rollup
Create a similar rollup