Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Explicit definition of required rust toolchain #11307

Open
andresilva opened this issue Apr 27, 2022 · 6 comments
Open

Explicit definition of required rust toolchain #11307

andresilva opened this issue Apr 27, 2022 · 6 comments
Labels
J1-meta A specific issue for grouping tasks or bugs of a specific category.

Comments

@andresilva
Copy link
Contributor

andresilva commented Apr 27, 2022

Currently we require a nightly toolchain to build the runtime (#1252). As part of CI we are currently using a nightly compiler for building the runtime and a stable compiler for the client code. It is not always obvious which nightly is needed to build Substrate at any given point (it's a common question in the support channels). Additionally, every time we need to bump the nightly version we need to ask the CI team to update the docker images otherwise the builds fail.

IMO we should introduce a rust-toolchain file that is supported by rustup (https://rust-lang.github.io/rustup/overrides.html#the-toolchain-file). We could still have CI docker images that come with a rust toolchain preloaded as a performance optimization (to avoid always downloading the toolchain), but this would guarantee that we wouldn't need to bother CI team to update the image whenever we need to bump the nightly (they could do it asynchronously since it's just a performance optimization).

This has one problem though: the rustup toolchain file only allows defining one toolchain to be used, whereas we currently use two toolchains. We could get around this by either: building everything with nightly (behavior change from what we do now), or using a stable toolchain with RUSTC_BOOTSTRAP=1 (this is how we currently build our production runtimes AFAIK). The fact that users would have to setup that environment variable might be troublesome though (or we can introduce direnv to this project :)

(I had previously created a PR for this: #10607)

@andresilva andresilva added the J1-meta A specific issue for grouping tasks or bugs of a specific category. label Apr 27, 2022
@pepyakin
Copy link
Contributor

pepyakin commented Apr 27, 2022

The fact that users would have to setup that environment variable might be troublesome though (or we can introduce direnv to this project :)

There were ideas of using the xtask approach, specifically for building the runtime in post native-optimization world (for context about that world refer to #7288 paritytech/polkadot-sdk#62).

Maybe that thing could provide a proper interface hiding the RUSTC_BOOTSTRAP thing.

@andresilva
Copy link
Contributor Author

We could also make wasm-builder inject the RUSTC_BOOTSTRAP environment variable.

@ggwpez
Copy link
Member

ggwpez commented Apr 30, 2022

I am also in favor of adding a rust-toolchain file since that could reduce the number of people that experience compile errors and open issues for it.
Maybe we can use one of the overrides for the wasm code? Setting the RUSTUP_TOOLCHAIN in the wasm builder or using a directory override for it: https://rust-lang.github.io/rustup/overrides.html ?

@bkchr
Copy link
Member

bkchr commented Apr 30, 2022

While I'm for moving the control about the rust version to use into the repo instead of having the CI file defining this, I'm not convinced to introduce any kind of further magic. RUSTC_BOOTSTRAP is a hack, by the compiler team. We could use it in our CI, I'm fine with this, but not setting this by default by the wasm-builder.

The rust-toolchain will also not fix issues by people experiencing build problems, because they probably depend on Substrate and their this toolchain file has no influence. Meaning they can still use some newer rust version that is maybe breaking something.

@TriplEight
Copy link
Contributor

TriplEight commented Jul 14, 2022

Good idea, thanks.

A short explainer about how CI automatically updates Rust versions currently:
On the example of CI image for Substrate. The job rebuilds the images nightly. While stable is installed in a base image and not pinned to an exact version, we've moved to pinning the nightly due to there's a code to update every time.
Then ci-linux:staging being scanned for vulnerabilities and tested against substrate and polkadot. If everything's green, it's promoted to production.

Of course we could read some particular (nightly) version from i.e. substrate repo and build a staging image. This would advance the automation, since now the entire procedure of updating a new Rust version can be done by someone who updates the broken code.

@athei
Copy link
Member

athei commented Mar 13, 2023

I think we should just close this once we are using the same stable toolchain for both client and runtime: #13580 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
J1-meta A specific issue for grouping tasks or bugs of a specific category.
Projects
None yet
Development

No branches or pull requests

7 participants
@andresilva @pepyakin @athei @bkchr @ggwpez @TriplEight and others