Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is "Overrides" section of "The rustup book" correct? #4150

Open
smoelius opened this issue Jan 10, 2025 · 5 comments
Open

Is "Overrides" section of "The rustup book" correct? #4150

smoelius opened this issue Jan 10, 2025 · 5 comments

Comments

@smoelius
Copy link
Contributor

The "Overrides" section of "The rustup book" says that shorthand notation should have the highest priority. However, my experiments suggest RUSTUP_TOOLCHAIN has higher priority.

I created this program, which I called a:

use std::process::{Command, ExitStatus};

fn main() {
    let _: ExitStatus = Command::new("rustup")
        .args(["+nightly", "show"])
        .status()
        .unwrap();
}

Running it with target/debug/a, I see this:

$ target/debug/a 
...
active toolchain
----------------

nightly-x86_64-unknown-linux-gnu (overridden by +toolchain on the command line)
rustc 1.86.0-nightly (824759493 2025-01-09)

However, running it with RUSTUP_TOOLCHAIN=stable target/debug/a, I see this:

$ RUSTUP_TOOLCHAIN=stable target/debug/a
...
active toolchain
----------------

stable-x86_64-unknown-linux-gnu (environment override by RUSTUP_TOOLCHAIN)
rustc 1.84.0 (9fc6b4312 2025-01-07)

Am I doing something wrong? Or have I misinterpreted the docs?

@rami3l
Copy link
Member

rami3l commented Jan 10, 2025

@smoelius Hmmm, just tried this on my machine:

> rustup --version
rustup 1.28.0 :: 1.27.1+556 (568c482a6 2025-01-07)
info: This is the version for the rustup toolchain manager, not the rustc compiler.
info: The currently active `rustc` version is `rustc 1.84.0 (9fc6b4312 2025-01-07)`
> RUSTUP_TOOLCHAIN=nightly rustup show
Default host: aarch64-apple-darwin
rustup home:  /Users/rami3l/.rustup

installed toolchains
--------------------
stable-aarch64-apple-darwin (default)
nightly-aarch64-apple-darwin (active)
nightly-2023-09-30-aarch64-apple-darwin

active toolchain
----------------
name: nightly-aarch64-apple-darwin
active because: overridden by environment variable RUSTUP_TOOLCHAIN
installed targets:
  aarch64-apple-darwin
> RUSTUP_TOOLCHAIN=nightly rustup +stable show
Default host: aarch64-apple-darwin
rustup home:  /Users/rami3l/.rustup

installed toolchains
--------------------
stable-aarch64-apple-darwin (active, default)
nightly-aarch64-apple-darwin
nightly-2023-09-30-aarch64-apple-darwin

active toolchain
----------------
name: stable-aarch64-apple-darwin
active because: overridden by +toolchain on the command line
installed targets:
  aarch64-apple-darwin

... same thing for RUSTUP_TOOLCHAIN=nightly sh -c 'rustup show', for example.

I'm not sure if I have changed this behavior or anything... 🤔
Anyway, here's the code:

rustup/src/config.rs

Lines 528 to 557 in 6fa21c2

fn find_override_config(&self) -> Result<Option<(OverrideCfg, ActiveReason)>> {
let override_config: Option<(OverrideCfg, ActiveReason)> =
// First check +toolchain override from the command line
if let Some(ref name) = self.toolchain_override {
let override_config = name.resolve(&self.get_default_host_triple()?)?.into();
Some((override_config, ActiveReason::CommandLine))
}
// Then check the RUSTUP_TOOLCHAIN environment variable
else if let Some(ref name) = self.env_override {
// Because path based toolchain files exist, this has to support
// custom, distributable, and absolute path toolchains otherwise
// rustup's export of a RUSTUP_TOOLCHAIN when running a process will
// error when a nested rustup invocation occurs
Some((name.clone().into(), ActiveReason::Environment))
}
// Then walk up the directory tree from 'path' looking for either the
// directory in the override database, or a `rust-toolchain{.toml}` file,
// in that order.
else if let Some((override_cfg, active_reason)) = self.settings_file.with(|s| {
self.find_override_from_dir_walk(&self.current_dir, s)
})? {
Some((override_cfg, active_reason))
}
// Otherwise, there is no override.
else {
None
};
Ok(override_config)
}

@smoelius
Copy link
Contributor Author

Thanks very much for your reply, @rami3l.

Regarding this test:

RUSTUP_TOOLCHAIN=nightly rustup +stable show

I get different results between stable and master. In brief, the "The rustup book" seems to describe master's behavior, but not stable's.

Here is what I see in more detail:

$ rustup --version
rustup 1.27.1 (54dd3d00f 2024-04-24)
info: This is the version for the rustup toolchain manager, not the rustc compiler.
info: The currently active `rustc` version is `rustc 1.84.0 (9fc6b4312 2025-01-07)`
$ RUSTUP_TOOLCHAIN=nightly rustup +stable show
...
active toolchain
----------------

nightly-x86_64-unknown-linux-gnu (environment override by RUSTUP_TOOLCHAIN)
rustc 1.86.0-nightly (824759493 2025-01-09)
$ rustup --version
rustup 1.28.0 :: 1.27.1+558 (6fa21c2c4 2025-01-10)
info: This is the version for the rustup toolchain manager, not the rustc compiler.
info: The currently active `rustc` version is `rustc 1.84.0 (9fc6b4312 2025-01-07)`
$ RUSTUP_TOOLCHAIN=nightly rustup +stable show
...
active toolchain
----------------
name: stable-x86_64-unknown-linux-gnu
active because: overridden by +toolchain on the command line
installed targets:
  i686-unknown-linux-gnu
  wasm32-unknown-unknown
  x86_64-unknown-linux-gnu

@djc
Copy link
Contributor

djc commented Jan 10, 2025

So if the rustup book indeed documents the master branch, it would probably be good to add an admonition about this change in context? I think changes in behavior are probably rare enough that it's not really helpful to maintain parallel published versions of the book. @smoelius would you be able to submit a PR for this?

@rami3l
Copy link
Member

rami3l commented Jan 10, 2025

So if the rustup book indeed documents the master branch, it would probably be good to add an admonition about this change in context? I think changes in behavior are probably rare enough that it's not really helpful to maintain parallel published versions of the book. @smoelius would you be able to submit a PR for this?

@djc @smoelius I just checked the blame of that page; it came from over 4 years ago without any changes. If anything, I believe this is a fixed regression instead of a breakage.

@djc
Copy link
Contributor

djc commented Jan 10, 2025

Hah, fair enough.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants