-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
ci: add a runner for vanilla LLVM 16 #110242
Conversation
(rustbot has picked a reviewer for you, use r? to override) |
This will need to coordinate with #110142 as well for the exact builder config. |
Yeah. We'll want an 8 core builder here too, probably. Otherwise this looks fine to me, r=me. |
@@ -11,7 +11,6 @@ RUN apt-get update && apt-get install -y --no-install-recommends \ | |||
file \ | |||
curl \ | |||
ca-certificates \ | |||
python2.7 \ |
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.
both Dockerfiles have an outdated comment about how they use both Python 2 and 3
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.
yes, please use python2 in at least one of the test runners.
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.
@jyn514 mingw-check-tidy
still uses it -- should be good enough?
# NOTE: intentionally uses python2 for x.py so we can test it still works. |
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.
Do we block on that in a full bors merge? I was under the impression it only ran on PRs.
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.
Hmm, I suppose that is just a pr
job. So, I can add something back to llvm-15 if you want, but the new llvm-16 builder is on ubuntu:23.04
which doesn't have python2 at all. Or we can find some other builder as the guinea pig, or maybe it's time to soft-retire python2 and stop worrying about it...
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.
Let's find some other builder to be the guinea pig. I see src/ci/docker/host-x86_64/dist-x86_64-illumos/Dockerfile is using 18.04, would that work?
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.
It seems like we're barely testing python2 compat at all, given that x.py
re-execs when it sees python3 available, but oh well. I've added it to dist-x86_64-illumos
as suggested.
5685cd9
to
209c6f6
Compare
Like rust-lang#107044, this will let us track compatibility with LLVM 16 going forward, especially after we eventually upgrade our own to the next. This also drops `tidy` here and in `x86_64-gnu-llvm-15`, syncing with that change in rust-lang#106085.
209c6f6
to
6fe2406
Compare
@bors r=Mark-Simulacrum |
☀️ Test successful - checks-actions |
Finished benchmarking commit (5fe3528): comparison URL. Overall result: ❌✅ regressions and improvements - ACTION NEEDEDNext Steps: If you can justify the regressions found in this perf run, please indicate this with @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)ResultsThis 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. |
Completely unrelated change. It's at least cool to see a list of some of the current benchmark noise. @rustbot label: +perf-regression-triaged |
Like #107044, this will let us track compatibility with LLVM 16 going
forward, especially after we eventually upgrade our own to the next.
This also drops
tidy
here and inx86_64-gnu-llvm-15
, syncing withthat change in #106085.