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

Merged configuration lists have unexpected precedence ordering #12506

Closed
arlosi opened this issue Aug 15, 2023 · 3 comments · Fixed by #12515
Closed

Merged configuration lists have unexpected precedence ordering #12506

arlosi opened this issue Aug 15, 2023 · 3 comments · Fixed by #12515
Labels
A-configuration Area: cargo config files and env vars A-rustflags Area: rustflags C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.

Comments

@arlosi
Copy link
Contributor

arlosi commented Aug 15, 2023

Problem

When merging TOML lists from configuration, the order does not follow the precedence order described in the documentation. The documentation does not explicitly specify the order for merging lists other than "Arrays will be joined together".

The docs state

If a key is specified in multiple config files, the values will get merged together. Numbers, strings, and booleans will use the value in the deeper config directory taking precedence over ancestor directories, where the home directory is the lowest priority. Arrays will be joined together.

The --config option may be specified multiple times, in which case the values are merged in left-to-right order, using the same merging logic that is used when multiple configuration files apply. Configuration values specified this way take precedence over environment variables, which take precedence over configuration files.

I expected a merged list would be (or the reverse of this):

  • command line
  • environment
  • deeper config
  • shallower config

Instead, the order appears to be:

  • deeper config
  • shallower config
  • command line
  • environment

Steps

The following script illustrates the current situation.

mkdir -p experiment/.cargo/
echo "test=['file']" > experiment/.cargo/config.toml
mkdir -p experiment/nested/.cargo/
echo "test=['nested-file']" > experiment/nested/.cargo/config.toml

pushd experiment/nested
CARGO_TEST=env-var cargo -Zunstable-options --config 'test=["first --config"]' --config 'test=["second --config"]' config get test
popd
rm -r experiment
test = ["nested-file", "file", "first --config", "second --config", "env-var"]

If a non-list is used, the precedence of these values is:

second --config
first --config
env-var
nested-file
file

Possible Solution(s)

Merge the lists with highest precedence first. The output of the above bash script should be:

test = ["second --config", "first --config", "env-var", "nested-file", "file"]

An alternative would be to merge them with lowest precedence first.

test = ["file", "nested-file", "env-var", "first --config", "second --config"]

Notes

No response

Version

cargo 1.73.0-nightly (7e9de3f4e 2023-08-13)
release: 1.73.0-nightly
commit-hash: 7e9de3f4ec3708f500bec142317895b96131e47c
commit-date: 2023-08-13
host: x86_64-unknown-linux-gnu
libgit2: 1.6.4 (sys:0.17.2 vendored)
libcurl: 8.2.1-DEV (sys:0.4.65+curl-8.2.1 vendored ssl:OpenSSL/1.1.1u)
ssl: OpenSSL 1.1.1u  30 May 2023
os: Ubuntu 22.04 (jammy) [64-bit]
@arlosi arlosi added C-bug Category: bug S-triage Status: This issue is waiting on initial triage. labels Aug 15, 2023
@weihanglo
Copy link
Member

For build.rustflags, the proposed solution might not be desirable, as flags in right overwrites those in right (if I remember correctly).

@weihanglo weihanglo added A-configuration Area: cargo config files and env vars S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted. A-rustflags Area: rustflags and removed S-triage Status: This issue is waiting on initial triage. labels Aug 15, 2023
@arlosi
Copy link
Contributor Author

arlosi commented Aug 16, 2023

I implemented both possible orders, and putting high precedence last requires the fewest test changes and works better with rustflags per @weihanglo's comment.

high precedence last: c611fed52
high precedence first: b1522d7b8

@weihanglo
Copy link
Member

Related issue: #8128.

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 A-rustflags Area: rustflags C-bug Category: bug S-needs-design Status: Needs someone to work further on the design for the feature or fix. NOT YET accepted.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants