-
Notifications
You must be signed in to change notification settings - Fork 13.6k
Ship correct Cranelift library in its dist component #145207
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
Conversation
@bors try |
[WIP] Fix Cranelift dist try-job: dist-x86_64-linux
I'm relatively sure that the second commit fixes the issue, but I'll let the try build finish to see if the UI test fails. If yes, then I'll rerun it to see if it works with the second commit. |
rustbot has assigned @albertlarsan68. Use |
r? jieyouxu |
This comment has been minimized.
This comment has been minimized.
💔 Test failed (CI). Failed jobs:
|
Some changes occurred in src/tools/opt-dist cc @Kobzol |
@bors try |
Ship correct Cranelift library in its dist component try-job: dist-x86_64-linux
This comment has been minimized.
This comment has been minimized.
💔 Test failed (CI). Failed jobs:
|
85610e6
to
1329f91
Compare
@bors try |
Ship correct Cranelift library in its dist component try-job: dist-x86_64-linux
This comment has been minimized.
This comment has been minimized.
💔 Test failed (CI). Failed jobs:
|
1329f91
to
6893e69
Compare
@bors try |
Ship correct Cranelift library in its dist component try-job: dist-x86_64-linux
Ok, should be ready now. @rustbot ready |
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.
Thanks
@bors r+ rollup=never (dist) |
I'll bump the priority here, because the queue is short, and we could still make today's nightly, to allow people to use Cranelift on latest nightly again. @bors p=1 |
If we're going to do that, same prio as rollups: |
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing fce0e74 (parent) -> 6355cd3 (this PR) Test differencesShow 5 test diffsStage 1
Stage 2
Additionally, 2 doctest diffs were found. These are ignored, as they are noisy. Job group index
Test dashboardRun cargo run --manifest-path src/ci/citool/Cargo.toml -- \
test-dashboard 6355cd39c81e9699b1925c58d2ed3165bcab1715 --output-dir test-dashboard And then open Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
Finished benchmarking commit (6355cd3): 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)Results (primary 3.2%, secondary -2.8%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.3%, secondary 0.6%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 464.993s -> 463.68s (-0.28%) |
The first commit adds a post-dist UI test to check that Cranelift can be used with the extracted dist x64 Linux archive.
The original codegen copy logic in the Cranelift dist step was a bit redundant, and I didn't notice in #144787 that it's copying the codegen backend from the build compiler's sysroot, rather than the target compiler's sysroot. The second commit modifies the logic to directly access the built codegen file (there is no need to search for it in the compiler's sysroot, in fact when you run just
x dist rustc_codegen_cranelift
, it shouldn't "taint" the sysroot with the codegen backend! Which it did before #144787) and copy it to the tarball under a normalized name. Thus we get around any similar potential issues in the future, and make previously implicit logic more explicit.This also fixes running just
x dist rustc_codegen_cranelift
without enablingcranelift
inrust.codegen-backends
, which should have been enabled by #144787, but it didn't work fully, because the dist step tried to copy the codegen backend from the compiler's sysroot, but it didn't contain the codegen backend if it was not enabled byrust.codegen-backends
.Fixes: #145201
try-job: dist-x86_64-linux