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

build: add rust-toolchain file #10607

Closed
wants to merge 2 commits into from
Closed

Conversation

andresilva
Copy link
Contributor

@andresilva andresilva commented Jan 7, 2022

This file will specify the current nightly version we are targeting and hopefully reduce the number of times we have to point people to use the correct compiler version. The shell.nix was updated to use this file. I have used the old format of rust-toolchain since the new TOML format is currently not supported by the rust nix overlay that we use in shell.nix.

this file will specify the current nightly version we are targeting
@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Jan 7, 2022
@andresilva
Copy link
Contributor Author

AFAICT this is the current version we use in CI.

@andresilva andresilva added B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders. labels Jan 7, 2022
@@ -0,0 +1,4 @@
[toolchain]
channel = "nightly-2021-10-29"
components = [ "rustfmt" ]
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want clippy in there?

Copy link
Contributor

Choose a reason for hiding this comment

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

I seem to have clippy by default so looks like we don't need to explicitly include it.

@gilescope
Copy link
Contributor

Works on my nix box. I am all for this as it reduces the barriers for external contributors.

Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

So compiles everything with the nightly compiler?

This is a changing behavior of the current way of building.

This file would also not be automatically changed when we add a new CI image.

Imo we should find a proper way on how we track the rust version entirely in git, aka dropping the installation of rust in the CI image for example and not start adding more and more locations that are loosely coupled.

@@ -0,0 +1,4 @@
[toolchain]
channel = "nightly-2021-10-29"
Copy link
Member

Choose a reason for hiding this comment

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

This isn't the version we use ci. This downloads its in every ci run.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I checked the CI logs to check for the version being used but blundered and copied the cargo nightly release date instead of rustc.

nightly-x86_64-unknown-linux-gnu (overridden by +toolchain on the command line)
rustc 1.58.0-nightly (46b8e7488 2021-11-07)

�[32;1m$ cargo +nightly --version�[0;m
cargo 1.58.0-nightly (94ca096af 2021-10-29)

@andresilva
Copy link
Contributor Author

So compiles everything with the nightly compiler?
This is a changing behavior of the current way of building.

Agreed, we probably need to settle on some standard for this. FWIW I'm in favor of just using one toolchain, i.e. either stable with RUSTC_BOOSTRAP hack, or just use nightly for everything.

This file would also not be automatically changed when we add a new CI image.

Imo we should find a proper way on how we track the rust version entirely in git, aka dropping the installation of rust in the CI image for example and not start adding more and more locations that are loosely coupled.

Ideally the CI image would just read from this file (or some other file that comes from the repo). As an optimization the CI image could have the latest version already downloaded, but when this file changes it should download it always (until the image gets updated).

I will close this for now.

@andresilva andresilva closed this Jan 11, 2022
@bkchr
Copy link
Member

bkchr commented Jan 11, 2022

Ideally the CI image would just read from this file (or some other file that comes from the repo).

That already worked with your change here.

I think we may could directly remove rust from the image. AFAIK all gha jobs also download rust in every run.

@xcaptain
Copy link
Contributor

Can substrate build on stable rust? I don't know why wasm-builder require a nightly version.
https://github.com/paritytech/substrate/tree/master/utils/wasm-builder

@bkchr
Copy link
Member

bkchr commented Jan 14, 2022

See the tracking issue: #1252

@bkchr bkchr deleted the andre/add-rust-toolchain-file branch January 14, 2022 07:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B0-silent Changes should not be mentioned in any release notes C1-low PR touches the given topic and has a low impact on builders.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants