Config enhancements. #7649
Merged
Config enhancements. #7649
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(rust_highfive has picked a reviewer for you, use r? to override) |
rust-highfive
added
the
S-waiting-on-review
|
☔ The latest upstream changes (presumably #7699) made this pull request unmergeable. Please resolve the merge conflicts. |
|
📌 Commit 7f14aeaaba4165d0465bc212b95b7dd5b01f7f8e has been approved by |
bors
added
S-waiting-on-bors
|
@bors retry |
|
@bors r=alexcrichton |
|
📌 Commit d2b25cf3a6026fc487b21bdbd4ec1296318f2196 has been approved by |
|
⌛ Testing commit d2b25cf3a6026fc487b21bdbd4ec1296318f2196 with merge 137e18f617bc21be68882dfd7b71375cfbbd3506... |
|
💔 Test failed - checks-azure |
bors
added
S-waiting-on-review
|
@bors r=alexcrichton |
|
📌 Commit ce7fd68 has been approved by |
bors
added
S-waiting-on-bors
bors
added a commit
that referenced
this pull request
Dec 19, 2019
Config enhancements. This is a collection of changes to config handling. I intended to split this into separate PRs, but they all built on one another so I decided to do it as one. However, I can still split this up if desired. High level overview: - Refactorings, mainly to remove `pub` from `Config::get_table` and use serde API instead. - Add `--config` CLI option. - Add config `include` to include other files. This makes some progress on #5416. Closes #6699. This makes a minor user-visible change in regards to `StringList` types. If an array is specified in a config as a list, and also as an env var, they will now be merged. Previously the environment variable overrode the file value. But if it is a string, then it won't join (env var takes precedence). I can probably change this, but I'm not sure if the old behavior is desired, or if it should merge all the time? **Future plans** This lays the groundwork for some more changes: - Work on #7253 (`debug-assertions` and `debug` fails in environment vars). I have some ideas to try. - Consider removing use of `get_list` for `paths`, and use a `Vec<ConfigRelativePath>`. This will require some non-trivial changes to how `ConfigSeqAccess` works. This is one of the last parts that does not use the serde API. - Possibly change `[source]` to load config values in a lazy fashion. This will unlock the ability to use environment variables with source definitions (like CARGO_SOURCE_CRATES_IO_REPLACE_WITH). - Possibly change `[profile]` to load config profiles in a lazy fashion. This will make it easier to use environment variables with profiles, particularly with arbitrarily named profiles. - Possibly remove the case-sensitive environment variables in `-Zadvanced-env`. I think they are just too awkward, and prone to problems. Instead, drive people towards using `--config` instead of env vars. - Add support for TOML tables in env vars (like `CARGO_PROFILES={my-profile={opt-level=1}})`). I started implementing it, but then looking at the use cases, it didn't seem as useful as I initially thought. However, it's still an option to try. **Refactoring overview** - `[source]` table now uses the serde API. - `[target]` table now uses the serde API. This is complicated since the 'cfg()' entries are different from the triple entries. The 'cfg()' tables are loaded separately, and are accessed from `Config::target_cfgs`. Otherwise, it just uses `config.get` of the specific target.TRIPLE. - Moved the target config stuff into `config/target.rs`. - Various changes to make this work: - Added `PathAndArgs` type which replaces `config.get_path_and_args`. - Changed `ConfigKey` to track the key parts as a list (instead of a string). This fixes an issue where quoted keys weren't handled properly (like `[foo.'a.b'.bar]`). This also seems to make a little more sense (it was joining parts into a string only to immediately call `split` on it). Changed various APIs to take a `ConfigKey` object instead of a string to avoid that splitting behavior. - `ValueDeserializer` now pre-computes the `Definition` so that it can provide a better error message when a value fails to deserialize. Overall, there shouldn't be significant user-visible changes. Some error messages have changed and warnings have been added for some ignored keys. `-Zadvanced-env` now works for source and target tables, though I'm not really happy with that feature.
|
☀️ Test successful - checks-azure |
This was referenced Dec 19, 2019
bors
added a commit
that referenced
this pull request
Jan 6, 2020
Fix CARGO_TARGET_triple_LINKER environment variable. #7649 caused an unfortunate regression where the `CARGO_TARGET_triple_LINKER` environment variable stopped working. I did not realize `serde(flatten)` caused serde to switch to `deserialize_map` which does not support environment variables. The solution here is to essentially revert back to how the `[target]` table used to be loaded by loading each key individually. This also removes the `ar` field which is not used by `rustc`.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a collection of changes to config handling. I intended to split this into separate PRs, but they all built on one another so I decided to do it as one. However, I can still split this up if desired.
High level overview:
pubfromConfig::get_tableand use serde API instead.--configCLI option.includeto include other files.This makes some progress on #5416.
Closes #6699.
This makes a minor user-visible change in regards to
StringListtypes. If an array is specified in a config as a list, and also as an env var, they will now be merged. Previously the environment variable overrode the file value. But if it is a string, then it won't join (env var takes precedence). I can probably change this, but I'm not sure if the old behavior is desired, or if it should merge all the time?Future plans
This lays the groundwork for some more changes:
debug-assertionsanddebugfails in environment vars). I have some ideas to try.get_listforpaths, and use aVec<ConfigRelativePath>. This will require some non-trivial changes to howConfigSeqAccessworks. This is one of the last parts that does not use the serde API.[source]to load config values in a lazy fashion. This will unlock the ability to use environment variables with source definitions (like CARGO_SOURCE_CRATES_IO_REPLACE_WITH).[profile]to load config profiles in a lazy fashion. This will make it easier to use environment variables with profiles, particularly with arbitrarily named profiles.-Zadvanced-env. I think they are just too awkward, and prone to problems. Instead, drive people towards using--configinstead of env vars.CARGO_PROFILES={my-profile={opt-level=1}})). I started implementing it, but then looking at the use cases, it didn't seem as useful as I initially thought. However, it's still an option to try.Refactoring overview
[source]table now uses the serde API.[target]table now uses the serde API. This is complicated since the 'cfg()' entries are different from the triple entries. The 'cfg()' tables are loaded separately, and are accessed fromConfig::target_cfgs. Otherwise, it just usesconfig.getof the specific target.TRIPLE.config/target.rs.PathAndArgstype which replacesconfig.get_path_and_args.ConfigKeyto track the key parts as a list (instead of a string). This fixes an issue where quoted keys weren't handled properly (like[foo.'a.b'.bar]). This also seems to make a little more sense (it was joining parts into a string only to immediately callspliton it). Changed various APIs to take aConfigKeyobject instead of a string to avoid that splitting behavior.ValueDeserializernow pre-computes theDefinitionso that it can provide a better error message when a value fails to deserialize.Overall, there shouldn't be significant user-visible changes. Some error messages have changed and warnings have been added for some ignored keys.
-Zadvanced-envnow works for source and target tables, though I'm not really happy with that feature.