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

(toolchain): Toolchain names must not be empty #2993

Merged
merged 1 commit into from
Oct 6, 2022

Conversation

kinnison
Copy link
Contributor

This fixes a problem where if you did toolchain link "" ...
then the toolchains dir was broken.

This fixes #2982

Signed-off-by: Daniel Silverstone dsilvers@digital-scurf.org

This fixes a problem where if you did `toolchain link "" ...`
then the `toolchains` dir was broken.

This fixes rust-lang#2982

Signed-off-by: Daniel Silverstone <dsilvers@digital-scurf.org>
Copy link
Member

@Rustin170506 Rustin170506 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@Rustin170506 Rustin170506 merged commit e40ca12 into rust-lang:master Oct 6, 2022
@asomers
Copy link
Contributor

asomers commented Jun 5, 2023

This actually broke CI for Nix. Basically, we had some tasks doing rustup toolchain install +$TOOLCHAIN; cargo +$TOOLCHAIN ..., while other tasks relied on the default toolchain that was included in the Docker image. Those tasks left TOOLCHAIN blank, and did cargo +"" .... I guess we can't do that anymore.

@Rustin170506
Copy link
Member

rustup toolchain install +$TOOLCHAIN;

Is this a typo? Because I think rustup toolchain install +1.70.0 should not work at all.

@Rustin170506
Copy link
Member

Those tasks left TOOLCHAIN blank, and did cargo +"" .... I guess we can't do that anymore.

If it is always blank, how about removing it and using cargo ....

@asomers
Copy link
Contributor

asomers commented Jun 6, 2023

Is this a typo? Because I think rustup toolchain install +1.70.0 should not work at all.

Yes, that's right. I mean rustup toolchain install $TOOLCHAIN.

If it is always blank, how about removing it and using cargo ....

It isn't always blank, because that code is shared by multiple tasks. I'm fixing it by ensuring that each task only installs a single toolchain, so I won't need to specify a toolchain with cargo.

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.

rustup toolchain link "" rust/build/x86_64-unknown-linux-gnu/stage1 deletes all toolchains
3 participants