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

Cargo does not use rust-toolchain.toml of separate project when executed within cargo run #12292

Closed
whitevegagabriel opened this issue Jun 20, 2023 · 12 comments
Labels
A-rustup Area: rustup interaction C-bug Category: bug S-triage Status: This issue is waiting on initial triage.

Comments

@whitevegagabriel
Copy link

whitevegagabriel commented Jun 20, 2023

Problem

I am using a rust script to build a variety of separate subprojects.

One of these subprojects declares a rust-toolchain.toml that uses "nightly" rust.

When I manually enter the directory, I see that nightly rust is being used.
When I use my rust script via cargo run, I see that stable rust is being used.

Seems that it loads rust-toolchain at invocation and never again.

Steps

  1. Create a build script with the following contents
fn main() {
    let root_dir = path::PathBuf::from(
        env::var("CARGO_MANIFEST_DIR").expect("Must be run via Cargo to establish root directory"),
    );

    let mut subproj_dir = root_dir;
    subproj_dir.push("subproj");

    let output = process::Command::new("sh")
        .arg("-c")
        .arg("cargo --version")
        .current_dir(subproj_dir)
        .output()
        .unwrap();

    println!("out: {}", from_utf8(&output.stdout).unwrap());
    println!("err: {}", from_utf8(&output.stderr).unwrap());
}
  1. Create a subproject in a folder called subproj with its own Cargo.toml, as well as a rust-toolchain.toml with the following contents
[toolchain]
channel = "nightly"
  1. From the top-level project, cargo run

Expected:
Should see the version is *-nightly

Actual:
Version was non-nightly

Possible Solution(s)

No response

Notes

Executing the build script binary (and hardcoding the subproject directory) results in no issues.

Version

cargo 1.70.0 (ec8a8a0ca 2023-04-25)
release: 1.70.0
commit-hash: ec8a8a0cabb0e0cadef58902470f6c7ee7868bdc
commit-date: 2023-04-25
host: x86_64-pc-windows-msvc
libgit2: 1.6.3 (sys:0.17.0 vendored)
libcurl: 8.0.1-DEV (sys:0.4.61+curl-8.0.1 vendored ssl:Schannel)
os: Windows 10.0.22621 (Windows 10 Pro) [64-bit]
@whitevegagabriel whitevegagabriel added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Jun 20, 2023
@weihanglo
Copy link
Member

If I understand correctly, rustup set RUSTUP_TOOLCHAIN environment variable for the proxy process once the toolchain is determined. It is kinda a sticky-fashion to ensure every build is under the same toolchain.

You could add a line of println!("{:?}", std::env::var("RUSTUP_TOOLCHAIN").unwrap()); in your build script to verify the behavior.

@weihanglo
Copy link
Member

I don't exactly know what you attempted to do, but there is an upcoming change that might affect things around rustup-cargo interaction. Just FYI.

From 1.71.0 changelog:

❗ Optimized the usage under rustup. When Cargo detects it will run rustc pointing a rustup proxy, it'll try bypassing the proxy and use the underlying binary directly. There are assumptions around the interaction with rustup and RUSTUP_TOOLCHAIN. However, it's not expected to affect normal users. #11917

@whitevegagabriel
Copy link
Author

Yeah, RUSTUP_TOOLCHAIN has the value "stable-x86_64-pc-windows-msvc" within the build script, as expected.

I think the problem is that this value should change to "nightly-x86_64-pc-windows-msvc" within the subprocess that executes cargo for my subproject that has declared its rust-toolchain.toml to use "nightly".

Interestingly enough, this problem does not occur on my Ubuntu or macOS machines, only on Windows (msvc).

@whitevegagabriel
Copy link
Author

Happy to provide more details if my "reproduction steps" are still too unclear.

@weihanglo
Copy link
Member

Thanks for the reply. The reproducer is pretty clear. However, I feel like there nothing is Cargo can do for this use case.

  • Cargo knows nothing about rustup (or “pretend” it doesn't know). RUSTUP_TOOLCHAIN is set by rustup for the sticky-fashion mentioned above.
  • If we are going to change that, it could be a breaking change for people based on this behavior to ensure the build with the same toolchain.

What you could do is, maybe, unsetting RUSTUP_TOOLCHAIN by your own. It might have unexpected consequence though.

See rustup overrides for more.

@whitevegagabriel
Copy link
Author

I appreciate you taking the time to look into this.

I think part of the reason why I raised this issue is that its a discrepancy between different platforms. This issue only occurs on Windows.

If a project has declared a rust-toolchain.toml (as mentioned in the rustup overrides you linked), then that config should be used regardless of whether cargo was run from bash or from a Command in rust, and regardless of platform (unless otherwise documented).

@weihanglo
Copy link
Member

I think part of the reason why I raised this issue is that its a discrepancy between different platforms. This issue only occurs on Windows.

Not really. I can reproduce it on Linux and macOS as well.

If you cannot reproduce it on other platforms, check where the invoked cargo is installed from.

If a project has declared a rust-toolchain.toml (as mentioned in the rustup overrides you linked), then that config should be used regardless of whether cargo was run from bash or from a Command in rust, and regardless of platform (unless otherwise documented).

No. The child process inherits RUSTUP_TOOLCHAIN from the parent process, which has the highest precedence over rust-toolchain.toml. The toolchain wouldn't be in use if there is a RUSTUP_TOOLCHAIN set.

@whitevegagabriel
Copy link
Author

Oh, very interesting!

In that case, I need to do some research on why I'm seeing the discrepancy... I'll do some research and report back.

@whitevegagabriel
Copy link
Author

So, looks like that at my time of reproducing on other platforms, I had unintentionally left a (for debugging purposes) .env("RUSTUP_TOOLCHAIN", "nightly"), which caused the build to pass on non-windows.

In other words, resetting RUSTUP_TOOLCHAIN had (as you warned), unexpected consequences in that it worked on other platforms, but not Windows.

Anyway, if that's expected, then I think my issue has been sufficiently addressed.
I'll probably just end up invoking the build script binary directly instead of via cargo run

@weihanglo
Copy link
Member

Okay then. I'll close this. If you find something very suspicious as a bug, let us know. Thank you :)

@whitevegagabriel
Copy link
Author

For posterity:
Clearing the var RUSTUP_TOOLCHAIN had no effect because for the sub-invocation, the cargo command was linked to .rustrup/toolchains/*toolchain*/bin/cargo.exe. (on macOS and Ubuntu, it was linked to .cargo/bin/cargo)

A workaround that is working for me (for now) is to clear RUSTUP_TOOLCHAIN, then directly invoke .cargo/bin/cargo.exe

@ehuss
Copy link
Contributor

ehuss commented Jun 21, 2023

A fix for Windows' behavior will be available in the next release of rustup via rust-lang/rustup#3178. Unfortunately it will be opt-in at first, but I hope the following release could turn it on by default.

@weihanglo weihanglo added the A-rustup Area: rustup interaction label Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-rustup Area: rustup interaction C-bug Category: bug S-triage Status: This issue is waiting on initial triage.
Projects
None yet
Development

No branches or pull requests

3 participants