Setting target.runner via --config twice concatenates lists

[RalfJung]

Consider the following invocation:

cargo +nightly run --target "x86_64-unknown-linux-gnu" --config "target.'cfg(all())'.runner=['run']" --config "target.'cfg(all())'.runner=['run']"

This fails saying

error: could not execute process `run run target/x86_64-unknown-linux-gnu/debug/xargo` (never executed)

I think there is a bug here: cargo tried to execute run run ..., i.e., it concatenated the two configs I gave! I would have expected the second config to overwrite the first, similar to what happens since #8629 when the runner is set both in cargo.toml and via the env var.

[Rustin170506]

@rustbot claim

[Rustin170506]

I think this is a feature, not a bug.

/// A config type that is a program to run.
///
/// This supports a list of strings like `['/path/to/program', 'somearg']`
/// or a space separated string like `'/path/to/program somearg'`.
///
/// This expects the first value to be the path to the program to run.
/// Subsequent values are strings of arguments to pass to the program.
///
/// Typically you should use `ConfigRelativePath::resolve_program` on the path
/// to get the actual program.
#[derive(Debug, Clone)]
pub struct PathAndArgs {
    pub path: ConfigRelativePath,
    pub args: Vec<String>,
}

So cargo treats the second run as an argument to the first run.

[RalfJung]

But when would that be what you want?
I might have set one runner for all targets and then another runner for a specific target. It shouldn't just concatenate those lists.

When using the env var CARGO_RUNNER, it correctly overwrites the value it would otherwise use. That used to also concatenate, but in #8629 the old behavior was deemed incorrect.

[Rustin170506]

Could you try cargo run --config "target.'cfg(all())'.runner=['run1']" --config "target.'cfg(cfg(not(target_os = "none")))'.runner=['run2']" --config "target.'cfg(all())'.runner=['run3']"?

I think it will use the last one and overwrite the second one.

Also you can try cargo run -config "target.'cfg(cfg(not(target_os = "none")))'.runner=['run2']" --config "target.'cfg(all())'.runner=['run3']"

It will use the last one.

[epage]

I believe this is being released on stable in 2 days which will likely limit what we can change about this

[Mark-Simulacrum]

There's still room for any patches, but please communicate closely with me (over Zulip, most likely) if the Cargo team would like to adjust things.

[Rustin170506]

For now, cargo run --config "target.'cfg(all())'.runner=['run1']" --config "target.'cfg(all())'.runner=['run3']"
equal to

[target.'cfg(all())']
runner = ["run1","run3"]

I'm not sure if should we change the behavior.

[Rustin170506]

If you try to add this config and also pass the CLI args, it will be concatenated.

[target.'cfg(all())']
runner = ["run1","run2"]

cargo run --config "target.'cfg(all())'.runner=['run3']" --config "target.'cfg(all())'.runner=['run4']"

You will get error: could not execute process run1 run2 run3 run4 target/debug/cargo (never executed).

And if you use the config and env variable, it won't be concatenated.

So maybe we should fix it, at least don't concatenate the .cargo/config and the CLI args. I am not sure does it is expected.

[RalfJung]

I believe this is being released on stable in 2 days which will likely limit what we can change about this

This is long stable via .cargo/config.toml, --config is just an easier way to describe these examples.

#8629 changed stable behavior AFAIK?

[RalfJung]

Oh interesting, this is rejected in config.toml:

[target.'cfg(all())']
runner = ["run1"]

[target.'cfg(all())']
runner = ["run1"]

So it is somewhat strange that it is accepted in --config.

However, if I have this in my ~/.cargo/config.toml

[target.'cfg(all())']
runner = ["run1"]

and this in my local .cargo/config.toml

[target.'cfg(all())']
runner = ["run1"]

Then it also merges the lists. So --config is consistent with having this set it different config files. This is not about --config.

It is about how cargo "merges" the value when it has a value set in different sources. Currently it is inconsistent:

• if one source is toml and the other is the env var, "merging" means "the later value overwrites the other ones"
• if both sources are toml, "merging" means "the later value gets appended after the previous ones"

I would expect "merging" to treat the env var as just another source, and do merging in the same way as it is done elsewhere. And for this specific case of the runner, IMO "merging" should mean to use the last value. Concatenating the lists makes sense for things like rustflags, but it doesn't make sense for runners.

You will get error: could not execute process run1 run2 run3 run4 target/debug/cargo (never executed).

There is no run2 in any of your settings, I am confused?

[Rustin170506]

There is no run2 in any of your settings, I am confused?

Oops, I posted the out-of-date config and command. updated.