-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Fix verification script and extended tests due to rustup
changes
#14990
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
rustup
changes
@@ -117,8 +117,11 @@ test_source_distribution() { | |||
|
|||
# build and test rust | |||
|
|||
# install the needed version of rust defined in rust-toolchain.toml | |||
rustup toolchain install |
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.
This is the recommended fix from rustup
This reverts commit 0ed0e0a.
.github/workflows/extended.yml
Outdated
|
||
jobs: | ||
# Check crate compiles and base cargo check passes | ||
linux-build-lib: | ||
name: linux build test | ||
runs-on: ubuntu-latest | ||
container: |
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.
this makes this job consistent with all the other CI jobs that use the rust container
I think this PR now has the right fix. Next steps are:
I'll try and check later tonight but I have to run for a family event |
Here is an example CI run of the extended tests running cleanly: |
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!
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.
Thank you 💯
Which issue does this PR close?
Rationale for this change
rustup
previously would automatically install the toolchain (version of rust) thatwas needed if it wasn't already installed.
However, the recently released rustup version changes this behavior
https://github.com/rust-lang/rustup/blob/f00c3d1fbcbe8d3ae2411e63ca906bc9b69e43d1/CHANGELOG.md?plain=1#L9-L17
Thus to ensure we have the correct toolchain installed, we need to run
rustup toolchain install
What changes are included in this PR?
rustup toolchain install
per release notesAre these changes tested?
Are there any user-facing changes?