Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(config): When merging, replace rather than combining specific configuration keys #15066

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

arlosi
Copy link
Contributor

@arlosi arlosi commented Jan 14, 2025

In general, Cargo merges lists in configuration by concatenating them. However, sometimes the lists don't make sense for merging, such as a program and its arguments. We had the UnmergedStringList type that handled this case for merging environment variables, but it did not work for multiple configs.

  • Removes the UnmergedStringList type, which only worked for preventing merging of environment variables with configuration.
  • Adds a new function is_nonmergable_list which hard-codes which configuration keys contain lists that should not be merged.

Fixes #14906

@rustbot
Copy link
Collaborator

rustbot commented Jan 14, 2025

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added A-configuration Area: cargo config files and env vars S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 14, 2025
@epage
Copy link
Contributor

epage commented Jan 15, 2025

When we talked about this in the Cargo team meeting (2025-01-07), we were ok with this so long as we felt there was a proper way of fixing this later. One idea to do that was to delay merging until access. We're unsure how difficult that would be to implement.

@rustbot
Copy link
Collaborator

rustbot commented Jan 16, 2025

☔ The latest upstream changes (possibly 149aa21) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-configuration Area: cargo config files and env vars S-waiting-on-review Status: Awaiting review from the assignee but also interested parties.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Configuration merge doesn't make sense for credential-provider
4 participants