🐛 fix(config): propagate overrides through config references#3951
Merged
Conversation
Documents the issue where TOX_OVERRIDE does not propagate through configuration references in both TOML and INI formats. When overriding a base environment value that is later referenced, the override is ignored and the original file value is used instead. Tests currently assert the buggy behavior. Expected behavior is documented in comments for when the issue is fixed. Root cause: Reference resolution loads raw config values without checking for overrides, bypassing the override system. Assisted-by: Claude Sonnet 4.5 <noreply@anthropic.com> Signed-off-by: Tales da Aparecida <tales.aparecida@redhat.com>
Contributor
Author
|
This is mostly a reproducer for #3950 |
TOX_OVERRIDE (and -x/--override) was ignored when overriding a base value
that is later referenced via {[section]key} (ini) or {replace = "ref",
of = [...]} (toml). Reference resolution reads raw values straight from the
parsed config, bypassing Loader.load() where overrides are applied.
Re-apply matching overrides onto the raw value at both reference-resolution
sites via a shared apply_overrides_to_raw helper, inferring the container
shape from the raw value. Flip the reproducer tests to assert the corrected
behavior and add a changelog entry plus docs.
Fixes tox-dev#3950
Contributor
Author
|
Thanks for working on this! |
This file contains hidden or 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
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.
TOX_OVERRIDE(and the-x/--overrideflag) silently lost effect whenever it targeted a base value that another environment pulled in by reference. Overridingenv_run_base.extrasand then reading it from an environment that referenced that base returned the original file value, which defeated the common pattern of overriding shared configuration once and having every dependent environment inherit it — handy in CI pipelines and local scripts. 🔧Overrides are normally applied inside
Loader.load, but reference resolution reads values straight from the parsed config and never reaches that path, so the override was dropped. The fix folds any matching override back onto the raw value at the two places references resolve — theofform of a TOML{replace = "ref", ...}and the{[section]key}form in ini — through a shared helper that reads from a newConfig.overridesview and infers the shape (list, dict, or scalar) from the value being overridden.Both replacement (⚠️ ini references are textual, so appending to a base value that sits inline with other tokens splices the appended item into that line — expected given how ini substitution works, and noted in the docs.
=) and append (+=) propagate now. The{replace = "ref", env = ..., key = ...}TOML form already worked because it resolves through a full environment load, so only the raw-read paths needed this.Fixes #3950.