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

use rustup override set to select toolchain #33

Merged

Conversation

fprasx
Copy link
Contributor

@fprasx fprasx commented Jan 12, 2024

Using rustup override set will override a local rust-toolchain.toml file while rustup default will not.

Resolves #31

Using rustup override set will override a local rust-toolchain.toml file
while rustup default will not.
@robjtede
Copy link
Contributor

I'd like to contrive a test (that would fail on current main) before merging this. I can do it or you can have a crack if you want.

@fprasx
Copy link
Contributor Author

fprasx commented Jan 12, 2024

Hmm, I don't want to spend too much time on this if you have time, because you'll definitely be more efficient than I am (I'm not great with yaml or bash). My initial idea is to do something like:

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 521e3f6..0e959d9 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -12,10 +12,11 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        rust:
-          # Test with toolchain file override
-          - null
+        use-toolchain-file:
+          - true
+          - false

+        rust:
           # Test that the sparse registry check works.
           # 1.66 and 1.67 don't support stable sparse registry.
           - "1.66"
@@ -32,7 +33,7 @@ jobs:

       # Test toolchain file support
       - name: Write rust-toolchain.toml
-        if: matrix.rust == null
+        if: matrix.use-toolchain-file == true
         shell: bash
         run: |
           cat <<EOF >>rust-toolchain.toml
@@ -51,7 +52,11 @@ jobs:
           components: clippy

       - name: Check ${{'${{steps.toolchain.outputs.rustc-version}}'}}
-        run: echo '${{steps.toolchain.outputs.rustc-version}}'
+        run: |
+          realversion=$(rustc --version)
+          [ "$realversion" != {{steps.toolchain.outputs.rustc-version}} ] || \
+            { echo "toolchain file not overridden: found $realversion"; exit 1}

       - name: Check ${{'${{steps.toolchain.outputs.cargo-version}}'}}
         run: echo '${{steps.toolchain.outputs.cargo-version}}'

Basically you do every test case with and without a toolchain file, but check that the resulting active toolchain is always the desired one. This is a little bad though because it would immediately double CI usage.

@robjtede
Copy link
Contributor

robjtede commented Jan 12, 2024

That's a good start; thanks! I'll get it sorted today then.

@obi1kenobi
Copy link
Contributor

Thank you both! 🚀

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@robjtede robjtede merged commit bcda41b into actions-rust-lang:main Jan 13, 2024
@jonasbb
Copy link
Member

jonasbb commented Jan 13, 2024

Thank you @fprasx for submitting the PR and thanks @robjtede, for adding the tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

toolchain file not properly overriden
4 participants