search

rust-lang / rustup

The Rust toolchain installer
https://rust-lang.github.io/rustup/
Apache License 2.0
6.18k stars 892 forks source link

Allow specifying multiple toolchains in `rust-toolchain.toml` file, with first as default, with auto-installation for all. #3546

Open CinchBlue opened 11 months ago

CinchBlue commented 11 months ago

Problem you are trying to solve

When running a project, often times, one wants to use nightly features from rustfmt, while also keeping a stable version for builds + deploys.

Solution you'd like

Allow a second key called [[other-toolchain]] to specify non-default toolchains. This should be backwards-compatible with the current rust-toolchain format:

# this is for builds
[toolchain]
channel = "1.70.0"
targets = [ "wasm32-unknown-unknown", "aarch64-linux-android" ]

# this is for rustfmt
[[other-toolchain]]
channel = "nightly-2023-11-26"

Notes

No response

rami3l commented 11 months ago

@CinchBlue Hi, thanks for filing this issue!

I share your usage and your concern (I even have cargo +nightly fmt configured as the formatter in my IDE), but I'm not quite sure if the solution you've proposed aligns with rust-toolchain.toml's intended role.

@rbtcollins @djc what do you think?

djc commented 11 months ago

This proposal doesn't make sense to me, as it's not obvious how or when the other-toolchain gets selected.

rami3l commented 11 months ago

This proposal doesn't make sense to me, as it's not obvious how or when the other-toolchain gets selected.

Let me clarify this use case a bit: the intention is to tell users to install nightly only for running cargo +nightly fmt. This is useful because cargo +stable fmt will emit a bunch of warnings in some projects with a non-trivial rustfmt config, and will happily refuse to format any format-able positions considered as nightly-only (by rustfmt, not by rustc).

My personal suggestion for @CinchBlue is this: configure a cargo +nightly fmt check in your CI (it's recommended to have cargo fmt checks in any non-trivial Rust repo anyway), so that whoever wants to make non-trivial changes to your code base will at some point install a nightly toolchain themself to avoid red lights in CI.

djc commented 11 months ago

Let me clarify this use case a bit: the intention is to tell users to install nightly only for running cargo +nightly fmt. This is useful because cargo +stable fmt will emit a bunch of warnings in some projects with a non-trivial rustfmt config, and will happily refuse to format any format-able positions considered as nightly-only (by rustfmt, not by rustc).

I understand this much. What I don't understand is what the other-toolchain entry is supposed to mean/do/trigger.

rami3l commented 11 months ago

I understand this much. What I don't understand is what the other-toolchain entry is supposed to mean/do/trigger.

Oh, nothing more than informing the user that this other-toolchain will be installed but not activated, if I understand it correctly.

My reasoning is that since there's no ideal way of expressing "we want +nightly to be the default just for cargo fmt", this should not be implemented as suggested. On the other hand, if there were such a way already, nightly should already have been installed by rustup for such intentions, exactly like when you run rustup default nightly without installing nightly.

rbtcollins commented 11 months ago

If someone wants to design a system tied into the proxies, so that 'cargo fmt' and 'rustfmt' both run the correct toolchain for this multi-toolchain case, then I think we can go through a mini RFP process to figure out all the interactions.

As it is I share @djc 's concern : this extra key is not coupled to any user behaviour - it is no different to documentation, except for this eager-install aspect, and I'm not sure that implicit eager-install is a good idea - outside of disconnected use, I don't understand the benefit today. And the cost is likely substantial.

We are, already, on a slow path to remove implicit install completely.

CinchBlue commented 11 months ago

My personal suggestion for @CinchBlue is this: configure a cargo +nightly fmt check in your CI (it's recommended to have cargo fmt checks in any non-trivial Rust repo anyway), so that whoever wants to make non-trivial changes to your code base will at some point install a nightly toolchain themself to avoid red lights in CI.

This is what I was doing before, but if you tell people to run that, you'll will inevitably end up with multiple versions of nightly running around per-person, meaning there will be a tug-of-war over which cargo +nightly fmt is correct, as it can differ between arbitrary nightly versions. (Yes, this is something that has been directly plaguing me and a few other people for months.)

The question is where do you "canonize" the correct Rust toolchain to "pin" for a given repo? Right now, it is rust-toolchain.toml, yet it only allows pinning 1. Allowing to at least pin 2 would be useful, but there is no reason I shouldn't be able to allow people to pin 10 so we can run CI targets + cross-compile targets for MSVs locally too.

Right now, I manually maintain a Makefile/justfile command to pin the version using a script variable to substitute in before all cargo commands. But this is still manual, and is not ideal for something that "just works."

rami3l commented 11 months ago

Right now, I manually maintain a Makefile/justfile command to pin the version using a script variable to substitute in before all cargo commands. But this is still manual, and is not ideal for something that "just works."

@CinchBlue I understand your disappointment here and I fully recognize that the current situation should be improved. That's why I said it is a personal suggestion: it's a workaround and I'm not satisfied with sticking to it.

However, I think we still need some more thinking in terms of feature design. It certainly cannot be shipped to end users in its current state.

CinchBlue commented 11 months ago

However, I think we still need some more thinking in terms of feature design. It certainly cannot be shipped to end users in its current state.

What is needed to get this to a workable, shippable state? Are there prior issues, threads, or lists I can see or that can be drafted here with discussion?

djc commented 11 months ago

I'm not aware of any prior issues/threads/lists for this, but maybe others have more context.

I'm open to the idea that there should be options to enable configuring multiple toolchains in rust-toolchain.toml. (Another use case that seems interesting is to have an msrv toolchain which can evolve independently over time.) However, I think that would basically require redesigning rust-toolchain.toml -- in a backward-compatible way -- to cope with an arbitrary number of toolchains, each of which define an "alias" that can be used with the proxies which is only in scope if the proxy is executed from a directory that is a dependent of the directory the rust-toolchain.toml.

@rbtcollins I think this basically aligns with your notes in https://github.com/rust-lang/rustup/issues/3546#issuecomment-1831292725? Any thoughts/suggestions/caveats/feedback?

CinchBlue commented 11 months ago

Also, you can reproduce inconsistencies with this setup:

$ rustup show -v
Default host: aarch64-unknown-linux-gnu
rustup home:  /home/ubuntu/.rustup

installed toolchains
--------------------

nightly-2023-11-24-aarch64-unknown-linux-gnu
rustc 1.76.0-nightly (a1a37735c 2023-11-23)

nightly-2023-11-26-aarch64-unknown-linux-gnu
rustc 1.76.0-nightly (f5dc2653f 2023-11-25)
# .rustfmt.toml
# Which version of the formatting rules to use. Version::One is backwards-compatible with Rustfmt 1.0.
# Other versions are only backwards compatible within a major version number.
version = "Two"
# Set a specific version of cargo-fmt for the CI
# required_version = "1.7.0"
edition = "2021"

unstable_features = true
format_generated_files = false

group_imports = "StdExternalCrate"
reorder_imports = true
imports_granularity = "Crate"

indent_style = "Block"

Running cargo +nightly-2023-11-26 vs. cargo +nightly-2023-11-24 produces a tug-of-war over formatting despite having the same general settings in the rustfmt configuration file.

RivenSkaye commented 11 months ago

As I've explained more in-depth in #3577 (closed as duplicate), another use case for this would be to allow switching between e.g. -windows-gnu and -windows-msvc toolchains. It currently locks you in to whichever the default selection is on first run while both toolchains are a valid option for any given channel + version/date combination.

As for using it, rustup will resolve e.g. +nightly-msvc to either a currently installed nightly-<...>-msvc or the latest available nightly-<host-triplet>-msvc toolchains (in that order). Even with a toolchain file, it will currently not resolve +msvc. This could be changed to checking if a toolchain file is present and using the specified information there to resolve it through +<channel>-msvc following the existing logical path. If no toolchain file is present, it'd be free to error in the same way it does currently (error: toolchain 'msvc' is not installed).

My initial proposed fix was adding an entry to the [toolchain] table, though I'm not averse to the solutions proposed in this issue either. Specifically to match the "main" table, I proposed a backends array, which would also work with most of the solutions here. Not sure if it's an issue for people running on other multi-backend platforms such as Linux(-gnu|-musl), but I think it'd also make it easier to keep projects tested across a larger variety of platforms if it's not required to manually specify them all, but instead manage it in the toolchain file for both dev and CI

djc commented 11 months ago

I'm not sure exactly what you mean by the backends array -- it seems like adding yet another concept which doesn't sound attractive to me.

rami3l commented 11 months ago

@rami3l the problem there lies in either statically linking a lot of MinGW stuff for it to work on Windows, resulting in much bigger binaries, or having to make sure MSVC can find and process MinGW headers for GNU builds. The latter wouldn't be much of an issue, if a rust-toolchain.toml file would cause it to always default to MSVC. But considering my system is set to default to GNU, I get the bloaty result.

Originally posted by @RivenSkaye in https://github.com/rust-lang/rustup/issues/3577#issuecomment-1851421775

I think you're referring to the problem of compiling to multiple Windows target triples. I think this section of the user guide might help with your situation?

You don't need to switch toolchains to support all windows targets though; a single toolchain supports all four x86 windows targets:

$ rustup target add x86_64-pc-windows-msvc
$ rustup target add x86_64-pc-windows-gnu
$ rustup target add i686-pc-windows-msvc
$ rustup target add i686-pc-windows-gnu

I don't use Windows personally, but I believe you can do all of your development work within the gnu toolchain. Just pass the correct triple to cargo with --target= and you should be good to go. AFAIK that's what people do when they need musl binaries on Linux: they stick to the gnu toolchain and add a musl target to that toolchain.

RivenSkaye commented 11 months ago

I'm not sure exactly what you mean by the backends array -- it seems like adding yet another concept which doesn't sound attractive to me.

That was my initial proposed fix in the other issue; similar in idea to the proposed [[other-toolchain]] here, with the biggest difference being that the fix proposed here would allow for fully customizing one other toolchain instead of allowing for several backends that'd inherit everything from the already existing selectors. Hence the general idea is perfectly compatible with what I initially suggested so it shouldn't need to introduce the extra concept.

I don't use Windows personally, but I believe you can do all of your development work within the gnu toolchain. Just pass the correct triple to cargo with --target= and you should be good to go. AFAIK that's what people do when they need musl binaries on Linux: they stick to the gnu toolchain and add a musl target to that toolchain.

Last I checked, using the GNU toolchain ended up linking in the usual MinGW libraries even on MSVC targets resulting in much bigger binaries. Running new compiles and comparisons to check rn

RivenSkaye commented 11 months ago

Well, seems like that is an issue of the past. I'm actually getting slightly smaller binary outputs now with a negligible difference for real-world cases, which made for a pleasant surprise today. That leaves just the angles mentioned in the original question and the additional pinning multiple for CI and collaboration. Sorry for the interruptions here.

For reference, the last time I checked was just before the last big GCC update hit MSYS2/MinGW64 so it has been a while.

rami3l commented 11 months ago

@RivenSkaye No worries, glad to hear that your problem is resolved!

ghost commented 10 months ago

Mmh i have just run into this issue myself and reading this gave me an idea: Instead of specifying multiple toolchains globally, what if it was allowed to override the toolchain per component? I think using nightly rustfmt together with stable rust is perfectly valid, so i was thinking about something along the lines of

[toolchain]
channel = "stable" # this is essentially the default toolchain

# channel for this component isn't specified, will use stable
[[toolchain.components]]
name = "clippy"

# channel is nightly, so nightly rust will be used for this component
[[toolchain.components]]
name = "rustfmt"
channel = "nightly"
target-san commented 9 months ago

I imagined this more like named toolchains. Reserved names like "stable", "nightly-YYYY-MM-DD" etc. aren't allowed. Smth like this:

# Default toolchain
[toolchain]
channel = "stable"
# Additional toolchain
[[toolchain.lint]]
channel = "nightly-2024-02-07"

Then, it will be used this way:

# Will use pinned nightly for toolchain "lint"
cargo +lint fmt
CinchBlue commented 9 months ago

I like this idea. I think the key name should be not the same as toolchain.

target-san commented 9 months ago

@CinchBlue The name's just a stub. Not even sure that the same key can be used as both simple entry and "list" declaration.

juntyr commented 8 months ago

I would also really love a way to specify multiple toolchains (with different names, one being the unnamed default) that are all installed (or the named ones could even be lazily installed when the name is first used). In my usecase, I have a workspace that uses stable but builds a few WASM modules using nightly since I need to use -Z build-std with them to build std with panic_immediate_abort (I'm not actually using any nightly features).

rbtcollins commented 8 months ago

@djc msrv is already specified in Cargo.toml; I think something like a virtual toolchain that consults that and then uses that toolchain would be better - otherwise we'd have no source of truth for it.

@juntyr for your case it seems like a rust-toolchain that specifies a nightly version that is stable for your needs, and then build everything with it, would meet your current needs.

For everyone here - the path forward depends on someone sitting down and writing a spec for the desired behaviour.

It needs to take into consideration the following impacts:

juntyr commented 8 months ago

@juntyr for your case it seems like a rust-toolchain that specifies a nightly version that is stable for your needs, and then build everything with it, would meet your current needs.

Thank you for considering my use case! While only using nightly certainly is a way to build the full project using just one toolchain, in this particular project I only want to use stable features (and e.g. https://github.com/rust-lang/rust/issues/120804 has shown that just using a nightly tool chain can produce spooky side effects when some dependency crate detects it and enables nightly features based on it). Aside from the proposed multi-toolchain toml config, other solutions to my use case would be (a) a way to build everything with the nightly compiler but to deny any crate from enabling nightly features [is there a way to do this?] or (b) to link a prebuilt std with immediate panic abort so I can continue using just a stable compiler. Perhaps you know if either of those is already possible?