Skip to content
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

chore: complete updates to build-images from msrv bump #12320

Merged
merged 3 commits into from
Feb 28, 2025

Conversation

TomAFrench
Copy link
Member

The rust MSRV got bumped in #12298 but we didn't catch that we're still using a stale devbox image due to the check in bootstrap.sh not being updated properly. CI's passing due to cargo automatically installing the proper toolchain at runtime.

I've completed the relevant changes to fix the build image / ./bootstrap.sh check in this PR but I'm not up to date on how pushing an update to devbox should be done (or if CI still automatically pushes to dockerhub anymore).

@charlielye @ludamad Could one of you help me with fixing this?

@TomAFrench
Copy link
Member Author

TomAFrench commented Feb 27, 2025

Sidenote on this, any thoughts on having ./bootstrap.sh read from noir/noir-repo/rust-toolchain.toml to ensure that it's in sync? Happy to roll this change into this PR.

@charlielye
Copy link
Contributor

Sidenote on this, any thoughts on having ./bootstrap.sh read from noir/noir-repo/rust-toolchain.toml to ensure that it's in sync? Happy to roll this change into this PR.

Interesting. So the check toolchain function would take the version from rust-toolchain? Think that makes sense.

How often are we expected to bump the rust version? because it is a pain... But as per README.md in the build-images, someone with the dockerhub password (i can do it), can run ./bootstrap.sh ci. Then I need to rebuild and update the AMI's we use in CI. It's not done automatically in CI yet.

@TomAFrench
Copy link
Member Author

How often are we expected to bump the rust version? because it is a pain...

I'm wanting to avoid doing it unnecessarily as well. We've followed the process of just bumping our MSRV in response to major dependencies bumping theirs (as this prevents us from publishing our crates). This should be less of an issue in future due to rust stabilising the msrv-aware resolver in the 2024 edition.

@TomAFrench
Copy link
Member Author

I've tested CI with a local build of the new image and everything is building correctly. Let me know if there's anything extra that needs to be done before we publish a new image.

@TomAFrench
Copy link
Member Author

@guipublic I spoke with Charlie on this today and we agreed that we're going to loosen the checks in bootstrap.sh so that it just prints a warning when there's a mismatch on the rust version as long as rustup is installed.

Let's get this PR merged and update the sync PR so we can unblock the sync.

@TomAFrench TomAFrench requested a review from guipublic February 28, 2025 13:39
@guipublic guipublic merged commit 51f16d4 into master Feb 28, 2025
6 checks passed
@guipublic guipublic deleted the tf/update-bootstrap.sh branch February 28, 2025 14:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants