Clobbering of environment variables during recursive invocations of cargo

[ratmice]

Problem

sometimes it would be convenient to do things like cargo run within .cargo/config.toml

[target.wasm32-wasip2]
runner = "cargo run -p my_runner"

So that you can avoid a separate install process for the test runner.

The caveat of this is that cargo run overwrites a bunch of environment variables that initially set by the cargo process that invoked the runner.

Proposed Solution

For cases such as this it would be nice to have an cargo run --env-var-prefix foo_ -p my_runner
this would then set variables like foo_OUT_DIR etc.

Perhaps a better way would be to just have a rarely used Cargo.toml field to specify an env var prefix, so
my_runner/Cargo.toml can use that, it would then be expected that my_runner/build.rs take part in
understanding that this crate always uses that env prefix. That seems more acceptable to me anyways.

Notes

No response

[epage]

The caveat of this is that cargo run overwrites a bunch of environment variables that initially set by the cargo process that invoked the runner.

Could you provide more details on the use case and what kind of environment variables you are needing access to that you lose? If you could provide a complete but minimal reproduction case, that would be a big help.

[ratmice]

The environment variable is CARGO_MANIFEST_DIR, and OUT_DIR if the runner has a build.rs.
I will try and come up with a minimal reproducer.

The use case is that we don't want to require cargo install --path foo_runner before building on a target that requires the runner which is in .config/cargo.toml.

By specifying cargo run -p foo_runner instead we could avoid the needed preinstall phase (if we could avoid the envs clashing)

[epage]

Ok, so by --env-var-prefix, you mean to say that any inherited env variable should also be passed down to the process started by cargo run with that specified prefix, so there would be a way to access CARGO_MANIFEST_DIR (as foo_CARGO_MANIFEST_DIR) without conflicting with what variables are normally set by cargo run on the process it launches.

The solution is relatively simple.

The use case is reasonable but the solution seems relatively niche and I wonder if there are other, longer term ways of resolving it and what that means for the short term. For example, we want to find ways to migrate project config to manifests (#12738) which includes aliases. We have artifact dependencies as a way for build scripts to run dependencies (and maybe even be build scripts, see #14903). If that was also applied to aliases / tasks / having runnable dependencies, then maybe there is a way for us to do similar for runners. The runner would then be accessing a binary and running it directly, rather than calling cargo run.

Unsure how well the name communicates the intent, especially if we add other flags like --env-set so you can pass environment variables to the process being run without affecting cargo.

[ratmice]

Yeah, you basically have the problem spot-on.

I was thinking the other way around, that cargo --env-var-prefix foo_ instead of renaming inherited environment variables with foo_ prefix would actually use the foo_CARGO_MANIFEST_DIR for the foo_runner package.
There is a slight problem with that way because if foo_runner has a build.rs which assumes CARGO_MANIFEST_DIR, the environment variables change depending on how run was invoked. Your approach where we forward existing cargo variables with the prefix fixes that.

The other alternative was to specify env-var-prefix in foo_runner/Cargo.toml so that it's build.rs always gets foo_CARGO_MANIFEST_DIR and thus never clashes with the default, and you don't have to specify it in cargo run
I find this last option perhaps the simplest because it can just be another optional Cargo.toml field.

Edit:
I forgot to say, I will read through your links, and ponder what you say!

[epage]

The other alternative was to specify env-var-prefix in foo_runner/Cargo.toml so that it's build.rs always gets foo_CARGO_MANIFEST_DIR and thus never clashes with the default, and you don't have to specify it in cargo run

Prefixing what cargo sets, and not what it inherits, feels like a cleaner approach.

The challenge with this is that there are a lot of helper libraries for build scripts that will read the variables directly and it would be a major shift in the ecosystem to get them to support custom environment variables. This will be exacerbated by #14903 which would mean project's may no longer have a build.rs but a dependency on a package that acts as their build.rs.

Since the intent is to access the parent process's cargo variables, this gets weird if the parent process also set an env prefix. Now you don't have a commonly accepted term to look up.

[ratmice]

Indeed, I think either approach would seem to work fine though (ignoring all the build script issues),

I just meant when I was thinking of the name --env-var-prefix I was thinking prefixing what we set, not prefixing what we inherit. The difficulty with prefixing what we inherit is coming up with a good name that conveys it, and the documentation is a little bit more difficult to convey what it does, since it is very niche.

But prefix-what-we-inherit is probably the most backwards compatible

[ratmice]

The reproducer that you asked for https://github.com/ratmice/cargo_issue_15350.git

It isn't the minimalist but is decently small, since it depends on e.g. wasmtime and the wasm32- wasip2 target.
And all the boiler plate after the assert! can kind of be ignored.

[ratmice]

FWIW, On the project I was working on it has been decided that cargo install preinstall phase is acceptable, so I personally don't need this feature. But I'll leave it up to the cargo project to decide whether or not this is a use case that they want to support.

That said, I had two ideas the avoid the entire environment variable clobbering:

One is based upon crate-type:

If we had a crate-type tool which is basically an alias for bin except run only passes env vars during build the phase of run and omits them when performing exec.

Second option is similar, except instead of being based on a crate type, it adds a new command

cargo exec -p runner

Where exec is similar to run except it would only pass environment variables during the build phase, and then exec the bin crate without cargo environment.

I think this last option manages to side-step a lot of problems with the fewest number of new concepts.