Replace dtolnay/rust-toolchain with manual rustup

[Eliah Kagan (EliahKagan)]

This builds on #2337 by replacing all uses of dtolnay/rust-toolchain with rustup commands, as described in the "Plan" section there. See the commit message here (dc980b2) for considerable details including background, rationale, and why an alternative action was not as good of a choice.

I first attempted to do this change (within my fork) via a Copilot PR. An error occurred, but the generated prompt in EliahKagan#141 (if one expands the details) is potentially a useful alternative summary. The diff here is the same as what Copilot would've done.

Analogous to that release test of changes to release.yml in #2337, a release test achieved by cherry-picking the commit used there onto this branch was done in this release test. (That was before rebasing this, but the rebase was just to bring this ahead of the merge commit--it didn't change the contents of release.yml or anything else.) Based on that, this seems not to break release.yml (so it should work again once some production-suitable fix/workaround for #2338 is devised).

However, this PR remains a draft until I check that the expected security scanning alerts, for both CodeQL and Zizmor, have appeared in the Security tab due to #2337. (Most of them should then go away as a result of this PR.)

[Eliah Kagan (EliahKagan)]

The expected code scanning alerts, from both CodeQL and Zizmor, were created and are visible in the Security tab, as intended in #2337. That's the main thing I wanted to check before merging this (which is expected to close most of the alerts, since it fixes the condition it describes).

The failure here that I mentioned in #2339 (review) looks like it's an occasional network error and not due to any changes here.

Therefore, I think this is ready to merge once its tests pass.

[Copilot]

Pull request overview

This PR replaces all uses of the dtolnay/rust-toolchain GitHub Action with manual rustup commands to address security concerns. The changes maintain functional equivalence while eliminating dependencies on third-party actions for Rust toolchain installation.

• Converts all toolchain installations to use rustup update, rustup default, and rustup target add commands
• Moves environment variables to appropriate scopes (job-level or step-level) to support the manual installation approach
• Adds explicit shell: bash specifications where needed for cross-platform compatibility

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
.github/workflows/release.yml	Replaces two instances of dtolnay/rust-toolchain with manual rustup commands; moves environment variables from step-level to job-level in the installation job for proper scoping
.github/workflows/ci.yml	Replaces six instances of dtolnay/rust-toolchain with manual rustup commands; adds shell: bash where needed for Windows/multi-OS jobs; converts component installation from action parameters to rustup component add

[Eliah Kagan (EliahKagan)]

Adds explicit shell: bash specifications where needed for cross-platform compatibility

These are actually just to be able to run a script step of multiple commands that stops and reports failure on the first failing command. Maybe this reason for using step-level shell: bash isn't obvious and should be made clearer in some way.

[Sebastian Thiel (Byron)]

Adds explicit shell: bash specifications where needed for cross-platform compatibility

These are actually just to be able to run a script step of multiple commands that stops and reports failure on the first failing command. Maybe this reason for using step-level shell: bash isn't obvious and should be made clearer in some way.

If Copilot doesn't know that even, then I think it's very non-obvious. Even reading this I can't imagine why it has this behaviour except that it's something they explicitly put in to make using scripts easier, while having to keep the default behaviour backwards-compatible to not fail fast.

[Eliah Kagan (EliahKagan)]

If Copilot doesn't know that even, then I think it's very non-obvious.

I can put comments on every step-level shell: bash, much as we do for shell: bash when it is set at the workflow or job level. I think that should help both humans and LLMs.

I think we should also somehow make clearer (at least to humans reading the workflow file) why we're using different shells in the first place in some jobs, even when setting shell: bash at the job level would simplify commands, and even though we're using job-level shell: bash in some other jobs. There are good reasons to run cargo nextest from different shells depending on the operating system--bash on a Windows GHA runner is Git Bash, whose environment subtly differs, and we intend the test suite to work properly even when run from outside a Git Bash environment. But I don't think this is currently clear enough.

Even reading this I can't imagine why it has this behaviour except that it's something they explicitly put in to make using scripts easier, while having to keep the default behaviour backwards-compatible to not fail fast.

I realized I forgot to reply to something earlier about this. I've done so now; see #1363 (comment). In short, PowerShell doesn't quite have this feature, and also it makes different judgments from POSIX-compatble shells about what constitutes an error.

But it almost has it, such that the default behavior on CI for PowerShell should maybe be different from what it is currently. The situation is:

1. set -e itself is subject to huge mismatches between its actual effect and its intuitive effects, and many shells intentionally avoid replicating that design. This includes not only shells like PowerShell that are meant to be very different from Unix-style shells, but also shells like fish that are meant to be intuitive for users of other Unix-style shells.
2. PowerShell does have an $ErrorActionPreference variable. Setting $ErrorActionPreference = 'stop' fails fast on errors. When an external command fails, however, that's not considered an error in PowerShell. This is set in PowerShell script steps in GHA.
3. PowerShell doesn't even automatically return the exit status of the last command to have run. However, this can be done reliably by appending a line to the script. GHA runners append if ((Test-Path -LiteralPath variable:\LASTEXITCODE)) { exit $LASTEXITCODE } when making the PowerShell script file for a script step.
4. Some versions of PowerShell support $PSNativeCommandErrorActionPreference. If this is set to $true then something approximating set -e is achieved. I'm not sure how good the approximation is. I think this is still conisdered an experimental feautre, but I'm not sure. This would be usable in pwsh but not in powershell. But GitHub-hosted Windows runners have both. pwsh is the default on Windows when present.
5. The GitHub Actions documentation documents points (2) and (3). But it characterizes the effect of the former as, "Fail-fast behavior when possible." It seems to me that this is misleading, since $PSNativeCommandErrorActionPreference is not set even when doing so is possible. (But it could be that I'm overestimating the uplift that confers in practice for achieving fail-fast behavior across separate commands in a PowerShell script.)

The different bash and pwsh/powershell behavior on CI led to the bug in #1556 that was fixed in #1559, which was not caused by any change in #1363 but would perhaps have been found in review had I not forgotten to reply to #1363 (comment) there. (I was reminded recently of the bug fixed in #1559, because the more severe bug I introduced in #2337 and fixed in #2342 resembles it, though this recent bug did not relate to fail-fast behavior in scripts.)

[Sebastian Thiel (Byron)]

Thanks a lot for sharing, I learned a lot!