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
labels
This comment has been minimized.
This comment has been minimized.
It would also be nice if we could test it on actual CI builders.
Feel free to add a builder to try or PR CI (whichever works, try might be needed for docker caches to work, at least once).
In general I'm personally happy to approve this once it's working as an experiment -- if we run into problems with it (e.g., flakiness, too long of a builder, etc.) then we can reconsider. I think if it works the relatively wide dependency graph and yet stable builds such are sufficient benefits to make Fuchsia a worthwhile sample for now, though I expect long-term we'll want to avoid blessing any particular project (e.g., by making this a distributed problem via a Crater-like solution). One option there might be to use a self-hosted GH runner which perhaps would even be run by third parties, but marked as optional in our builds, or similar. I'm personally not yet convinced that Crater is a good fit due to the non-scalable human triage element in it today.
Hopefully I did this right..
@bors try
bors added a commit to rust-lang-ci/rust that referenced this pull request
Build Fuchsia in CI This is very much in a draft state but I wanted to put it up now to get early feedback. It would also be nice if we could test it on actual CI builders. Fittingly, it is failing right now due to discovering an ICE in clippy, probably more fallout from recent type system changes. Other recent regressions this would have caught include - rust-lang#117455 and rust-lang#117493 - rust-lang#117602 This ends up not sharing very much at all with cargotest. Fuchsia has its own tool to manage checkouts and its own build system. What it requires is a fully "install"ed toolchain with a host and fuchsia target. We share logic from the dist-various-2 builder to build the fuchsia target. Right now this runs clippy and skips linking a bunch of targets, since most issues we catch are in the frontend. In theory we could probably get the build CPU time down quite a bit with this approach, but right now some linked targets are creeping into the dependencies anyway and we don't have a good way of preventing that yet. The approach is basically to get a checkout at a pinned commit and then run a [script](https://fuchsia-review.git.corp.google.com/c/fuchsia/+/943833/6/scripts/rust/build_fuchsia_from_rust_ci.sh) at a predetermined location. I would like to update that pin every few weeks. For now we cherry-pick some build changes but that's temporary. Partial checkouts are used to minimize clone time, but we don't filter out prebuilt packages. r? `@Mark-Simulacrum` Based on discussion in [this Zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/Putting.20Fuchsia.20in.20crater).
This comment was marked as off-topic.
This comment was marked as off-topic.
☀️ Try build successful - checks-actions
Build commit: 17146a2 (17146a280ee06a5870ba05d2032e4bb2c7b9d1ad)
This comment has been minimized.
This comment has been minimized.
bors added a commit to rust-lang-ci/rust that referenced this pull request
Build Fuchsia in CI This is very much in a draft state but I wanted to put it up now to get early feedback. It would also be nice if we could test it on actual CI builders. Fittingly, it is failing right now due to discovering an ICE in clippy, probably more fallout from recent type system changes. Other recent regressions this would have caught include - rust-lang#117455 and rust-lang#117493 - rust-lang#117602 This ends up not sharing very much at all with cargotest. Fuchsia has its own tool to manage checkouts and its own build system. What it requires is a fully "install"ed toolchain with a host and fuchsia target. We share logic from the dist-various-2 builder to build the fuchsia target. Right now this runs clippy and skips linking a bunch of targets, since most issues we catch are in the frontend. In theory we could probably get the build CPU time down quite a bit with this approach, but right now some linked targets are creeping into the dependencies anyway and we don't have a good way of preventing that yet. The approach is basically to get a checkout at a pinned commit and then run a [script](https://fuchsia-review.git.corp.google.com/c/fuchsia/+/943833/6/scripts/rust/build_fuchsia_from_rust_ci.sh) at a predetermined location. I would like to update that pin every few weeks. For now we cherry-pick some build changes but that's temporary. Partial checkouts are used to minimize clone time, but we don't filter out prebuilt packages. r? `@Mark-Simulacrum` Based on discussion in [this Zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/Putting.20Fuchsia.20in.20crater).
☀️ Try build successful - checks-actions
Build commit: c1cb7ea (c1cb7eaca115bb89ab34af966e96de83837e9180)
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This should be ready now, and hopefully passing now that #117979 is in.
@bors try
I decided to go with "integration" as the builder name, since it's essentially an integration test over the Fuchsia codebase. Open to suggestions though.
I'm personally not yet convinced that Crater is a good fit due to the non-scalable human triage element in it today.
I see what you mean when it comes to release runs. When running crater on individual PRs I think it would be useful. Maybe later on we can come up with a way to do this besides "add these lines to ci.yml and do a try run".
One option there might be to use a self-hosted GH runner which perhaps would even be run by third parties, but marked as optional in our builds, or similar.
Yeah, I think something like that would be good to have eventually!
This comment was marked as outdated.
This comment was marked as outdated.
bors added a commit to rust-lang-ci/rust that referenced this pull request
Build Fuchsia in CI This is very much in a draft state but I wanted to put it up now to get early feedback. It would also be nice if we could test it on actual CI builders. Fittingly, it is failing right now due to discovering an ICE in clippy (looks like fixed in rust-lang/rust-clippy#11760), probably more fallout from recent type system changes. Other recent regressions this would have caught include - rust-lang#117455 and rust-lang#117493 - rust-lang#117602 This ends up not sharing very much at all with cargotest. Fuchsia has its own tool to manage checkouts and its own build system. What it requires is a fully "install"ed toolchain with a host and fuchsia target. We share logic from the dist-various-2 builder to build the fuchsia target. Right now this runs clippy and skips linking a bunch of targets, since most issues we catch are in the frontend. In theory we could probably get the build CPU time down quite a bit with this approach, but right now some linked targets are creeping into the dependencies anyway and we don't have a good way of preventing that yet. The approach is basically to get a checkout at a pinned commit and then run a [script](https://fuchsia-review.git.corp.google.com/c/fuchsia/+/943833/6/scripts/rust/build_fuchsia_from_rust_ci.sh) at a predetermined location. I would like to update that pin every few weeks. For now we cherry-pick some build changes but that's temporary. Partial checkouts are used to minimize clone time, but we don't filter out prebuilt packages. r? `@Mark-Simulacrum` Based on discussion in [this Zulip thread](https://rust-lang.zulipchat.com/#narrow/stream/242791-t-infra/topic/Putting.20Fuchsia.20in.20crater).
Some changes occurred in src/tools/cargo resolved
cc @ehuss
I bumped the Fuchsia commit, and we no longer need any cherry picks to build it. Also updated the PR description.
Locally this builds fine, so I don't think another try is needed.
Rebased and still passing locally.
Rebased, bumped Fuchsia and squashed. Still passing locally.
Rebased and bumped Fuchsia; still passing locally.
| os: ubuntu-20.04-4core-16gb | ||
| env: {} | ||
| - name: x86_64-gnu-integration | ||
| os: ubuntu-20.04-16core-64gb |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: We should try to reduce builder sizes where it's not adding value. Historically 16 core builders rarely make things that much faster -- once this lands and we collect a few dozen builds we can re-evaluate, perhaps at the next bump point for the pinned commit.
and removed S-waiting-on-review
labels
Finished benchmarking commit (c722d51): comparison URL.
Overall result: no relevant changes - no action needed
@rustbot label: -perf-regression
Instruction count
This benchmark run did not return any relevant results for this metric.
Max RSS (memory usage)
Results
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.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
0.4% | [0.4%, 0.4%] | 1 |
| Regressions ❌ (secondary) |
- | - | 0 |
| Improvements ✅ (primary) |
-1.7% | [-2.5%, -0.6%] | 3 |
| Improvements ✅ (secondary) |
- | - | 0 |
| All ❌✅ (primary) | -1.1% | [-2.5%, 0.4%] | 4 |
Cycles
Results
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.
| mean | range | count | |
|---|---|---|---|
| Regressions ❌ (primary) |
- | - | 0 |
| Regressions ❌ (secondary) |
- | - | 0 |
| Improvements ✅ (primary) |
-0.4% | [-0.4%, -0.4%] | 1 |
| Improvements ✅ (secondary) |
-5.8% | [-5.8%, -5.8%] | 1 |
| All ❌✅ (primary) | -0.4% | [-0.4%, -0.4%] | 1 |
Binary size
This benchmark run did not return any relevant results for this metric.
Bootstrap: 669.584s -> 667.911s (-0.25%)
Artifact size: 314.15 MiB -> 314.09 MiB (-0.02%)
When I try to run this image locally, it always hangs during the jiri update step. Is there anything special one needs to do to run this image?
Nevermind, after adding -v to that command it's clear that it's not hanging, it's just downloading something like 2GB of data.
@nikic Yeah sorry, it's a big checkout with lots of prebuilts. We could look into minimizing the number of prebuilts downloaded if it's prohibitively large.