CARGO env variable does not get set when running commands

[wmmc88]

Describe The Bug

The README describes

When defined with scripts (as opposed to commands), the CARGO environment variable will be defined for the requested toolchain.

Although this says it doesn't define it for commands, I think this should so that the behavior matches with running via rustup or cargo +toolchain.

To Reproduce

build.rs:

use std::path::PathBuf;
use std::env;

fn main() {
    panic!("{:#?}", env::var("CARGO").map(PathBuf::from).unwrap());
}

Makefile.toml:

[tasks.nightly-build]
toolchain = "nightly"
extend = "build"

These commands panic with stable toolchain path:
cargo build
rustup run stable cargo build
cargo make build
cargo make nightly-build

These commands panic with nightly toolchain path:
cargo +nightly build
rustup run nightly cargo build

The issue here is that cargo make nightly-build should print the nightly toolchain path. Something in cargo-make is preventing it from doing that.

[sagiegurari]

its not cargo make. if i remember this was mentioned before and it was due to cargo behavior itself.

[wmmc88]

its not cargo make. if i remember this was mentioned before and it was due to cargo behavior itself.

Do you have a workaround for this?

Also the reason I think this is cargo make is because when running rustup run nightly cargo build directly, i get the expected behavior of using nightly toolchain, but when cargo-make executes rustup run nightly cargo build it seems to use nightly toolchain but the env vars are all set to stable toolchain

[wmmc88]

Ok after lots of debugging, this is what i came up with:

1. Despite printing the stable toolchain path for CARGO, it is still using the nightly toolchain. I verified this by using nightly-only features. the reason nightly toolchain is still used is because rustup currently prepends the nightly toolchain to the PATH when running rustup run nightly cargo build. Since rustup also sets RUSTC to rustc, it correctly still uses the nightly toolchain, but i believe this will break on the next rustup release: Change default for RUSTUP_WINDOWS_PATH_ADD_BIN rust-lang/rustup#3703. After the next release, the stable toolchain will be selected when running cargo make nightly-build.

2. So why is CARGO pointing to stable? its because when cargo make nightly-build is invoked, it uses the default toolchain (for me that's stable).

This is the flow:

1. cargo make nightly-build
2. rustup redirects to stable toolchain path
3. cargo sets CARGO env var to stable path per cargo docs
4. stable cargo calls cargo-make
5. cargo-make calls rustup run nightly cargo build which redirects to the nightly cargo
6. cargo sees CARGO is set to stable toolchain path so it does not change the value, and just forwards it per cargo docs
7. nightly cargo compiles the code with nightly toolchain since per cargo docs since cargo doesn't actually use the CARGO variable
8. CARGO as read by the build script still points to stable

I think what needs to happen here is that because of this recursive rustup invocation, cargo-make needs to either clear or explicitly set CARGO. Right now, it only clears RUSTC, RUSTDOC, and RUSTFLAGS, and only explicitly sets CARGO on scripts (and not commands). This matches previous recommendation found in a related thread in rustup

I'm going to test these changes locally and put up a PR if it works