Separate rust toolchains from global install

[doubledup]

As mentioned in #916 we need to be more explicit about separating the dev environment's rustup installation and about how we select Rust toolchains.

Using the .toml extension in .cargo is the preferred form, eg. .cargo/config.toml: https://doc.rust-lang.org/cargo/reference/config.html

[codecov]

Codecov Report

Patch and project coverage have no change.

Comparison is base (6707c33) 75.40% compared to head (a7fc57a) 75.40%.
Report is 5 commits behind head on main.

❗ Current head a7fc57a differs from pull request most recent head 5ad14d8. Consider uploading reports for the commit 5ad14d8 to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #918   +/-   ##
=======================================
  Coverage   75.40%   75.40%           
=======================================
  Files          40       40           
  Lines        1736     1736           
  Branches       74       74           
=======================================
  Hits         1309     1309           
  Misses        407      407           
  Partials       20       20           

Flag	Coverage Δ
rust	76.02% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[yrong]

Seems codes above are just ignored in

snowbridge/smoketest/.gitignore

Lines 14 to 16 in 599f149

	src/parachains/bridgehub.rs
	src/parachains/assethub.rs
	src/parachains/relaychain.rs

so may not require fmt.

Btw: In previous commit in PR#915 there is a workaround here
https://github.com/Snowfork/snowbridge/blob/c2ae10a22140fe0a110d78c6f54251fa8e0d5109/hooks/pre-commit#L16-L35 for rustfmt by adding another rust-toolchain.toml.nightly into project and switch between the two toolchains as required.

Then no need to explict add nightly-2023-07-31 multiple times into each command. But it seems a bit tricky so I'm also fine with the current explict way.

[doubledup]

I think the formatting is just in case we need to read the code when writing smoke tests - you're right that there's no need for it, but it'll help a bit when writing tests.

There isn't really a good path for our toolchain management right now. Swapping out toolchain files means we need a separate command to set the toolchain before every set of Rust commands. We're removing the toolchain file after the standup discussion, but now users could modify their toolchain and see build behaviour change without a change in the repo. This is fixed by rerunning scripts/init.sh, but ideally we would commit a stable and a nightly version (with relevant components & targets) that would be used automatically.

The installation behaviour in rustup show will likely be replaced by a subcommand for explicit installation at some point:
rust-lang/rustup#2686 (in particular this comment)
rust-lang/rustup#1397

[yrong]

There isn't really a good path for our toolchain management right now. Swapping out toolchain files means we need a separate command to set the toolchain before every set of Rust commands

Yes, agree. We're doing some workaround here to make both toolchains to work in same repo and It's a bit tricky. TBH I prefer we make things simple and stick to nightly version for the dev setup.

I would assume actually Parity encourage the use of nightly rust and their docker image to build production release is nigthly also. https://github.com/paritytech/scripts/blob/f901682d2d1b2fa5ed046864f36c2da481532843/dockerfiles/ci-linux/Dockerfile#L7

We can still manually build binary with stable rust to check if it works(I think it should work as long as we're not using some nightly specific feature).

[doubledup]

Vincent's mentioned that we should use toolchain files instead, as they ensure that the version is always tracked in the repo. I've added a nightly toolchain file as in the commit you linked. We'll just need to keep an eye out for rustup updates when we update nixpkgs.

[doubledup]

The Dockerfile mentions that the default is stable: https://github.com/paritytech/scripts/blob/f901682d2d1b2fa5ed046864f36c2da481532843/dockerfiles/ci-linux/Dockerfile#L27
(the default is stable until another toolchain is selected: https://rust-lang.github.io/rustup/concepts/channels.html#working-with-nightly-rust)

So they're probably only using nightly for rustfmt, which is what we're aiming for afaik. Stable has fewer surprises, so it's a better bet for production builds.

[yrong]

The Dockerfile mentions that the default is stable

Yes, you're right as defined here
https://github.com/paritytech/scripts/blob/f901682d2d1b2fa5ed046864f36c2da481532843/dockerfiles/base-ci-linux/Dockerfile#L66

[yrong]

LGTM!

[doubledup]

Tried adding an alias for rustfmt, but they don't work in Nix shells: NixOS/nix#1598
(seems this also applies to flakes)

[vgeddes]

Looks good now!