cargo

cargo copied to clipboard

What does this PR try to resolve?

Fixes #10992

Behaviour changes: After this commit, config files loaded via CLI take priority over env vars. Config files loaded via config-include feature also get a higher priority over env vars, if the first config file is being loaded via CLI.

Cargo knows why it loads a specific config file with WhyLoad enum, and store in the field of Definition::Cli(…). With this additional data, config files loaded via --config <path> get a Definition::Cli(Some(…)). In contrast, --config <value> with ordinary values become Definition::Cli(None).

The error message of the Definition::Cli(Some(…)) is identical to Definition::Path(…), so we won't lose any path information to debug.

How should we test and review this PR?

Several tests are added in config-cli and config-include test suites.

To check the precedence, set up a project with an extra config file, such as

# Expect to have a target-dir named `foo`
CARGO_BUILD_TARGET_DIR='env' cargo c --config 'build.target-dir = "cli"' --config .cargo/foo.toml

# Inside .cargo/foo.toml
[build]
target-dir = "foo"

Additional information

This is a bit hacky IMO. I don't like leaking the path info to Definition::Cli. However, it is really tricky to provide information for deserialization before values get merged.

r? @ehuss

(rust-highfive has picked a reviewer for you, use r? to override)

Thanks!

@bors r+

:pushpin: Commit e0502e4e78335a32f7c905db78ff05c20e2b9867 has been approved by ehuss

It is now in the queue for this repository.

:hourglass: Testing commit e0502e4e78335a32f7c905db78ff05c20e2b9867 with merge 0c29d43b9a54e6c30d51465ca365c50344ed59a1...

:sunny: Test successful - checks-actions Approved by: ehuss Pushing 0c29d43b9a54e6c30d51465ca365c50344ed59a1 to master...