CARGO is not Cargo when build script is invoked through cargo used as a library

[jonhoo]

Problem

When a program using cargo as a library initiates a build, the CARGO environment variable to build scripts is set to be the path to the current executable, even though that command may look nothing like Cargo. This, in turn, means that build scripts invoked as part as such builds may fail if they try to use CARGO as though it were Cargo, like for example cbindgen does:
https://github.com/eqrion/cbindgen/blob/93c06c5c9d319f481788c9670700097b4e46d270/src/bindgen/cargo/cargo_metadata.rs#L233-L235

Steps

#!/bin/bash

set -euo pipefail

rm -rf cargo-build-rs
mkdir cargo-build-rs
cd cargo-build-rs

cargo new cargo-something
pushd cargo-something
echo 'cargo = "0.57"' >> Cargo.toml
cat >src/main.rs <<EOF
use cargo::core::compiler::{BuildConfig, CompileMode};
use cargo::core::resolver::{CliFeatures};
use cargo::core::Workspace;
use cargo::ops::{CompileFilter, CompileOptions, Packages};
use cargo::Config;

fn main() {
  // This is the smallest working emulation of `cargo build` I could come up with.
  let mut config = Config::default().unwrap();
  config
    .configure(0, false, None, false, false, false, &None, &[], &[])
    .unwrap();
  let root = cargo::util::important_paths::find_root_manifest_for_wd(config.cwd()).unwrap();
  let ws = Workspace::new(&root, &config).unwrap();
  let build_config = BuildConfig::new(&config, None, &[], CompileMode::Build).unwrap();
  let cli_features = CliFeatures::from_command_line(&[], false, true).unwrap();
  let compile_opts = CompileOptions {
      build_config,
      cli_features,
      spec: Packages::from_flags(false, Vec::new(), Vec::new()).unwrap(),
      filter: CompileFilter::from_raw_arguments(
          false,
	  Vec::new(),
          false,
          Vec::new(),
          false,
          Vec::new(),
          false,
          Vec::new(),
          false,
          false,
      ),
      target_rustdoc_args: None,
      target_rustc_args: None,
      local_rustdoc_args: None,
      rustdoc_document_private_items: false,
      honor_rust_version: true,
  };
  cargo::ops::compile(&ws, &compile_opts).unwrap();
}
EOF
cargo build
popd

cargo new --lib has-buildrs
cd has-buildrs
cat >build.rs <<EOF
fn main() {
    println!("{}", std::env::var("CARGO").unwrap());
    assert!(false);
}
EOF
env PATH="$PWD/../cargo-something/target/debug:$PATH" cargo something

Yields

~/cargo-build-rs/cargo-something/target/debug/cargo-something

Note that a command like $CARGO metadata (like cbindgen executes) would fail in this case, since cargo-something != cargo .

Possible Solution(s)

This is a tough one. In the particular case where the binary invoking a build through cargo-as-a-library is a cargo external subcommand, perhaps the initiating cargo invocation could set CARGO (by some other name) that then gets picked up transparently by the build logic. But in the more general case, it seems like this needs to be settable through cargo::Config somewhere so that library users at least have a way to correct CARGO if they do invoke a build.

Notes

No response

Version

cargo 1.56.0 (4ed5d137b 2021-10-04)
release: 1.56.0
commit-hash: 4ed5d137baff5eccf1bae5a7b2ae4b57efad4a7d
commit-date: 2021-10-04

[jonhoo]

It could be that one significant improvement here is to make the logic that sets CARGO should propagate an existing CARGO if it's already set. That way, if cargo-something is invoked as cargo something, the CARGO that the "outer" Cargo sets (which is truly Cargo) will carry through to the inner build script. It won't fix cases where Cargo isn't involved in the outer execution, but I think that's certainly the common case.

[jonhoo]

With #10115 closed (which I think is reasonable), I think it's even more important to figure out a solution to this, ideally using a single mechanism to address both cases.

[jonhoo]

To leave breadcrumbs to related issues, a very specific (but also slightly different) instantiation of this problem is #10113. Ideally a solution addresses both use-cases.

[jonhoo]

Also, just for convenience for readers of this issue, the place where Cargo discovers the current Cargo executable is

cargo/src/cargo/util/config/mod.rs

Line 406 in 3ff0443

pub fn cargo_exe(&self) -> CargoResult<&Path> {

It's used here for external authentication commands:

cargo/src/cargo/ops/registry/auth.rs

Line 126 in f955ddc

.env("CARGO", config.cargo_exe()?)

Here for build scripts:

cargo/src/cargo/core/compiler/compilation.rs

Line 316 in 1ac43cf

cmd.env(crate::CARGO_ENV, cargo_exe);

And here for fingerprinting:

cargo/src/cargo/core/compiler/fingerprint.rs

Line 728 in 25c5ced

let current = if key == CARGO_ENV {

[jonhoo]

I also just realized that Cargo already sets CARGO when it invokes subcommands:

cargo/src/bin/cargo/main.rs

Lines 193 to 195 in c71f344

	let cargo_exe = config.cargo_exe()?;
	let mut cmd = ProcessBuilder::new(&command);
	cmd.env(cargo::CARGO_ENV, cargo_exe).args(args);

So I think there's a reasonable argument for saying that Cargo should prefer re-using an existing value for CARGO over overwriting it with what current_exe returns.

[jonhoo]

I filed #11285 to fix this by forwarding CARGO if already set