-
-
Notifications
You must be signed in to change notification settings - Fork 517
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
Tox 4.3.0 breaks substitution for values from other sections #2863
Comments
reproduced. i think the fix here would be to also run replace (once) on values originating from other ini lines (since those wouldn't be replaced at the point they are subbed in). |
Would that work for a reference of a reference too? E.g. depending on b, where b depends on c? I think this was the reason why we ran the replacement recursively until no replacement was possible. |
As i'm thinking of the solution, no, it would not. One of the benefits (IMO) of the new parser is that it doesn't do further replacements on the replacements, so you can't accidentally get into infinite loops if there is a cycle. I have thought of an alternate syntax like Am I on the wrong track with this? I could flip on recursive substitution in the current syntax, and maybe that would be less astonishing to users. In this case, expanding the value from another ini key makes sense for 1-level expansion, but i don't know how many users might be depending on multi-level expansion |
Thanks for being on it! Is tox missing tests for these As far as references of references go, I'd definitely expect that to work, but raise a noisy error if there was a cycle (or perhaps if it was over (say) 100 iterations). What did the previous version do? |
I think this is what we'd need to do 🤔 I'd like to avoid new syntax for recursive replacements. |
Yes.
I believe did this, checked for loops and raise error if detected it |
Expand substitution expressions that result from a previous subsitution expression replacement value (up to 100 times). Fix tox-dev#2863
* test_replace_tox_env: add missing chain cases When a replacement references a replacement in a non-testenv section it should also be expanded * Recursive ini-value substitution Expand substitution expressions that result from a previous subsitution expression replacement value (up to 100 times). Fix #2863 * cr: changelog: fix trailing period * test_replace_tox_env: tests for MAX_REPLACE_DEPTH Create a long chain of substitution values and assert that they stop being processed after some time.
@peterschutt 4.3.1 will be momentarily released containing a fix @benhoyt |
Thanks @gaborbernat! |
Originally this PR was running `pyupgrade --py38-plus` on the codebase, to pick up a few new features in Python 3.7 and 3.8 (apart from f-strings, which was done in #889). However, in particular we didn't like how it converted `TypedDict` to the class syntax, because then half of them were converted (the ones without `-` in the field names can't be converted). So avoid that so they're all consistent. This includes: * Use of set literals and comprehensions instead of set(...) * Use OSError instead of IOError as the latter is now an alias * Use b'' instead of bytes() * One use of f-string that apparently flynt didn't pick up This PR also: * Removes the tox 4.2.8 pinning now that tox-dev/tox#2863 is fixed. * Makes tox.ini source vars relative to fix autopep8 problem
Issue
Up till Tox 4.2.8 our tox config worked fine, however as of the just-released 4.3.0 it now breaks on the substitution of values from other sections using the
{[sectionname]valuename}
syntax (relevant Tox docs).I looked at the 4.3.0 changelog, and it is almost certainly caused by the recent merge of #2861, "Rewrite substitution parser". CC: @masenf @gaborbernat
A cut-down version of our
tox.ini
is as follows (full version here):With 4.2.8 and earlier,
tox -e foo
looked like this (note theECHOING
line):However, with 4.3.0,
tox -e foo
is not interpolating those variables but echoing them literally:Environment
Ubuntu 22.04 amd64
pip list
Output of running tox
Output of
tox -rvv
(when 4.3.0 is installed):tox -rvv
The text was updated successfully, but these errors were encountered: