Skip to content
This repository has been archived by the owner on Jan 13, 2025. It is now read-only.

Consider using a rust-toolchain.toml file #25603

Closed
ruuda opened this issue May 27, 2022 · 5 comments
Closed

Consider using a rust-toolchain.toml file #25603

ruuda opened this issue May 27, 2022 · 5 comments
Assignees
Labels
stale [bot only] Added to stale content; results in auto-close after a week.

Comments

@ruuda
Copy link
Contributor

ruuda commented May 27, 2022

Problem

Solana depends intimately on the particular rustc version used. One example of this was solana-labs/rbpf@5394026, where before that change in rbpf was merged, Solana would segfault when compiled with the wrong rustc version.

However, for the default approach of building a Rust project (cargo build), rustup uses the system-wide default toolchain, not the one suitable for the particular Solana release. Fortunately, the right Rust version is recorded in the repository:

stable_version=1.60.0
However, it’s in a nonstandard location, and it’s not used by rustup’s cargo proxy binary.

Proposed Solution

Add a rust-toolchain or rust-toolchain.toml file to pin the stable toolchain using rustup’s standard mechanism. The existing ci/rust-version.sh can still manage the nightly toolchain. As far as I am aware, there is no standardized way to manage multiple toolchains.

@mvines, you seem to most often update the Rust version, what do you think?

@mvines
Copy link
Contributor

mvines commented May 27, 2022

sounds like a good idea!

@KirillLykov
Copy link
Contributor

KirillLykov commented Dec 18, 2022

The plan is to split one huge PR into several smaller:

  1. Add ./cargo-nightly, use it instead of ./cargo introduce cargo-nightly #29319
  2. Add toolchain file, take version from it take rust version from toolchain file #29320
  3. Get rid of ./cargo in (depends on 1 and 2)
  1. Do the same replacement for github workflows Simplfiy scripts using ./cargo after git workflow changes #29105

@yihau
Copy link
Contributor

yihau commented Dec 22, 2022

I'm thinking should we use the pattern below to replace ./cargo instead of only cargo 🤔

source ci/rust-version.sh stable

... (omit)

cargo +"$rust_stable"

the concern is that if someone set rustup override, I think the priority will be higher than cargo-toolchain.toml.

@KirillLykov
Copy link
Contributor

I think the main idea of using cargo build is that it is simple and expected procedure. If a user has set up rust version using overrides, I would assume that he/she knows what he/she is doing and it is up to developer.

@github-actions github-actions bot added the stale [bot only] Added to stale content; results in auto-close after a week. label Dec 28, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jan 5, 2024
@steviez
Copy link
Contributor

steviez commented Jan 5, 2024

Switching this from "not planned" to "closed"; we still depend on the custom cargo script for some aspects (ie running fmt and clippy with nightly), but, we do have a rust-toolchain.toml file as of #29320 so the basic case of just building the repo is supported.

Any further work to try to reduce usage of this scripts can probably take place in new PR's / issue(s)

@steviez steviez closed this as completed Jan 5, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
stale [bot only] Added to stale content; results in auto-close after a week.
Projects
None yet
Development

No branches or pull requests

5 participants