Configuration merge doesn't make sense for credential-provider

[kornelski]

Problem

When merging multiple config.toml files, Cargo concatenates arrays. For the registries.….credential-provider setting this merging is counter-productive, because it constructs an invalid command.

Steps

When there's more than one applicable cargo.toml file that contains:

[registries.custom]
credential-provider = ["cargo:token-from-stdout", "command", "arguments"]

after merging, it becomes:

[registries.custom]
credential-provider = ["cargo:token-from-stdout", "command", "arguments", "cargo:token-from-stdout", "command", "arguments"]

and results in Cargo running command arguments cargo:token-from-stdout command arguments, which contains wrong arguments, and can fail in very confusing ways.

It's particularly easy to end up with exact duplicates of a custom registry configuration when configs are injected by build tools or projects have their own copy in addition to per-user or per-host configs.

Possible Solution(s)

The credential-provider field should be replaced, not concatenated, when merging config files.

Notes

No response

Version

cargo 1.83.0 (5ffbef321 2024-10-29)

[arlosi]

There are a few different config fields affected by this same issue.

• registries.*.credential-provider
• target.*.runner
• host.runner
• credential-alias.*
• doc.browser

We have an UnmergedStringList type that's used for cases like this, but it only works when merging configuration from environment variables with non-environment sources. It doesn't work for two config files as mentioned here.

Unfortunately, the configuration merging between files happens while all the data is still in TOML form, so Cargo is unaware of which fields are UnmergedStringLists, so we can't apply per-type logic when merging.

Options to fix this:

1. Change the config system to defer config value merging to when the config is queried, so we can apply the type-specific (UnmergedStringList) logic. Each config source would be stored separately, and merging would happen at the config::get() call.
2. Remove the UnmergedStringList type and use an explicit list of which configuration keys are not to be merged. The existing merging logic would then reference this explicit list to determine whether to combine or override lists.

[tanvincible]

@rustbot claim