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

Simplify setup.{sh,bat} and work around rust-lang/rustup#2601. #332

Closed
wants to merge 1 commit into from

Conversation

eddyb
Copy link
Contributor

@eddyb eddyb commented Dec 9, 2020

rustup component add will install the toolchain if it isn't already, and will respect rust-toolchain, so hardcoding the value from rust-toolchain into setup.sh and setup.bat isn't necessary.

It also works around rust-lang/rustup#2601, which I've seen a handful of people hit (including myself), usually due to IDE integration installing the rust-src component using rustup component add, before ./setup.sh is ran.

The inclusion of rust-std is a second level of workaround, it somehow avoids the problem described in rust-lang/rustup#2601 (comment), where running the new setup.sh/setup.bat for the same toolchain that the old setup.sh/setup.bat was already ran (and it executed rustup toolchain install ... --component rust-src, putting the components list in a broken/suboptimal state).

More specifically, installing rust-std (even on its own) makes rustup decide to remove the (wrong) rust-src-$host component and install the (correct) rust-src one, if the former happens to be installed.
(The two versions of rust-src are the same component "package", only their names in lib/rustlib/components differ)

@eddyb eddyb requested a review from khyperia as a code owner December 9, 2020 07:47
@eddyb
Copy link
Contributor Author

eddyb commented Dec 9, 2020

@khyperia mentioned we've gone back & forth on this, and it looks like the short form was suggested in #139 (comment), but then later reverted in d9838a0 and I don't know why.

If there is a bug with rustup component add that I haven't managed to reproduce yet during my rust-lang/rustup#2601 testing, I want to know what it is before we merge this or do anything else with rustup.

@eddyb eddyb marked this pull request as draft December 9, 2020 11:04
@eddyb
Copy link
Contributor Author

eddyb commented Dec 9, 2020

Oh, #139 getting merged and d9838a0 were very close together, and I believe I found the context on Discord.

Looks like for @repi rustup component add wasn't respecting rust-toolchain? I haven't ran into that myself.
This is what happens when you don't have the toolchain installed (with the setup.sh from this PR):

$ rustup toolchain uninstall nightly-2020-11-24
info: uninstalling toolchain 'nightly-2020-11-24-x86_64-unknown-linux-gnu'
info: toolchain 'nightly-2020-11-24-x86_64-unknown-linux-gnu' uninstalled
$ ./setup.sh
info: syncing channel updates for 'nightly-2020-11-24-x86_64-unknown-linux-gnu'
info: latest update on 2020-11-24, rust version 1.50.0-nightly (d9a105fdd 2020-11-23)

And when the toolchain exists, I can use verbose mode to see it downloads the right version:

$ rustup component remove rust-src
info: removing component 'rust-src'
$ rustup -v component add rust-std rust-src
verbose: read metadata version: '12'
info: component 'rust-std' for target 'x86_64-unknown-linux-gnu' is up to date
info: downloading component 'rust-src'
verbose: downloading file from: 'https://static.rust-lang.org/dist/2020-11-24/rust-src-nightly.tar.xz'

@repi
Copy link
Contributor

repi commented Dec 9, 2020

Think it was simply failing in CI before I did revert that and went back on installing explicit version. But may work now so that would be good!

@eddyb eddyb marked this pull request as ready for review December 9, 2020 11:27
@eddyb
Copy link
Contributor Author

eddyb commented Dec 9, 2020

Current main uses rustup component add on CI that looks like it hasn't changed since #63.

So I don't see how setup.sh/setup.bat could affect CI? Is there CI logic outside of that ci.yaml file?

@eddyb
Copy link
Contributor Author

eddyb commented Dec 9, 2020

Looks like the whole rust-src bug is still going to be a problem once we switch to TOML rust-toolchain rust-lang/rustup#2601 (comment) - but potentially less so, assuming nothing runs rustup component add rust-src directly (RLS probably won't, it checks first).

Also, rust-std can't be used as a workaround with the TOML because the workaround only causes rustup to fix itself when ran in a separate rustup invocation than the one which installed the broken rust-src-$host.

@eddyb
Copy link
Contributor Author

eddyb commented Dec 9, 2020

Looks like TOML works with RLS, but only because of a coincidence: rust-lang/rustup#2601 (comment)

I guess I'll go the full TOML route, and I've opened NixOS/nixpkgs#106443 so that I can have a TOML-capable rustup.

EDIT: I guess that means suggesting my local changes to #284.

@eddyb eddyb closed this Dec 9, 2020
@khyperia khyperia removed their request for review January 11, 2021 10:00
@eddyb eddyb deleted the better-setup branch November 25, 2023 08:44
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.

2 participants