tox 3.27.1 to 4.0.9, environment var substitution failing #2732
Comments
I'm failing to replicate this. Can you provide a complete repository that manifests this? Please also try with tox 4.0.11. |
gaborbernat
added
the
needs:reproducer
|
I created a minimal repo here: https://github.com/techpriest002/tox_substitution_issue.git I also tried with tox 4.0.11, but I am seeing the same behavior. Below are the commands I used to clone the repo above and prepare the environment: C:\WINDOWS\system32>cd \Users\nab1234\source\repos
C:\Users\nab1234\source\repos>git clone https://github.com/techpriest002/tox_substitution_issue.git
C:\Users\nab1234\source\repos\tox_substitution_issue>"C:\Users\nab1234\AppData\Local\Programs\Python\Python39\python.exe" -m venv .venv
C:\Users\nab1234\source\repos\tox_substitution_issue>.venv\Scripts\activate
(.venv) C:\Users\nab1234\source\repos\tox_substitution_issue>pip install -r requirements.txt
(.venv) C:\Users\nab1234\source\repos\tox_substitution_issue>tox -e docs
docs: commands[0]> python -c "print(r\"html -- C:\Users\nab1234\source\repos\tox_substitution_issue\docs\_build{env:BUILD}\")"
html -- C:\Users\nab1234\source\repos\tox_substitution_issue\docs\_build{env:BUILD}
docs: OK (4.67=setup[4.17]+cmd[0.50] seconds)
congratulations :) (5.27 seconds)
(.venv) C:\Users\nab1234\source\repos\tox_substitution_issue>tox --version
4.0.11 from C:\Users\nab1234\source\repos\tox_substitution_issue\.venv\lib\site-packages\tox\__init__.pyFor whatever reason, the |
gaborbernat
added
bug:minor
|
It looks like there are two issues here:
I have an idea for a fix. |
explicit test case for issue tox-dev#2732 which affects {/} os.sep replacement on windows platform.
1. Split the value to be replaced on double backslash. 2. If there is more than 1 split, recursively perform replace on each sub value. 3. Rejoin replaced sub values on single backslash. This allows "\\" to be an escape for "\" while preserving recursive expansion semantics, and not having already-escaped backslashes affect processing replacements in other parts of the string. Fixes tox-dev#2732
if the replaced value is a single backslash, then process replace on the left and right parts of the value separately so that the replacement cannot act as an escape in other parts of the value. Fixes tox-dev#2732
|
The simple solution that I had proposed over the weekend does not work when the double backslash escape appears nested inside another expression, like The simple solution was splitting the string on double backslash and doing the replace on the inbetween sections of string, but this ultimately breaks the expression since it would lose its trailing A proper solution is needed which recursively replaces sub-parts of the expression, while respecting the backslash escape. I plan to work on this, but am currently focused on a different project. |
explicit test case for issue tox-dev#2732 which affects {/} os.sep replacement on windows platform.
explicit test case for issue tox-dev#2732 which affects {/} os.sep replacement on windows platform. test_replace_os_sep_subexp_regression WiP
|
I finally got around to rewriting the replace parser, so progress is being made on resolving this issue, and cleaning up other edge cases. Some related context in issue #1708, which tells me replace expression semantics may have been unclear for a while. By that, I'm hesitant to push for such a rewrite without clearly documenting the new behavior and ensuring good regression testing. I already have at least 5 test cases which pass using the new parser and fail using the old parser. I think the "new" way is more correct, but since this is a behavior change, users may be affected by it. The biggest change is that parsing is now iterative through the string: it will never recursively combine and replace anything that originated from a replacement. So if For example, it's enough in current tox 4 to get into an infinite loop via With my new parser, this just prints I think this is the less astonishing approach, because you can imagine silliness like: With the new parser, that prints With the current parser, we see Using the new parser design, we can selectively allow recursive replacement via a different syntax, such as |
|
@gaborbernat in the v4 docs, I'm not finding the config substitutions section that describes the behavior relevant to this issue. Did that end up somewhere else in the docs? If not, I can migrate the relevant v3 content, remove the explicit examples (which are already enumerated on the v4 config page), and describe the new semantics as part of this upcoming PR. |
|
Go ahead with that 👍 we didn't migrate. Just make sure to explain this in a Ini configuration parser section and not generic configuration section. |
explicit test case for issue tox-dev#2732 which affects {/} os.sep replacement on windows platform. test_replace_os_sep_subexp_regression WiP
|
There were some additional issues with the reproducer posted here that are addressed in PS C:\Users\masen\code\repro-2732> tox -e docs
docs: recreate env because constraint options changed: old=None new={'constrain_package_deps': True, 'use_frozen_constraints': False}
docs: remove tox env folder C:\Users\masen\code\repro-2732\.tox\docs
docs: commands[0]> python -c "print(\"html -- C:\\Users\\masen\\code\\repro-2732\\docs\\_build\\html\")"
html -- C:\Users\masen\code\repro-2732\docs\_build\html
docs: OK (0.58=setup[0.42]+cmd[0.16] seconds)
congratulations :) (0.67 seconds)
PS C:\Users\masen\code\repro-2732> tox --version
4.4.0 from C:\Users\masen\AppData\Local\Programs\Python\Python311\Lib\site-packages\tox\__init__.py |
Issue
All of our build pipelines are failing due to what appears to be a bug with environment variable substitution in our tox.ini. The variable substitution works as expected with tox==3.27.1, but we start having issues with tox==4.0.9.
Environment
Provide at least:
pip listof the host Python wheretoxis installed:Output of running tox
Provide the output of
tox -rvv:Minimal example
This is a stripped down tox.ini file, "C:\tmp\tox_4.0.9_bug\tox.ini".
If I run "tox -e docs" with tox 4.0.9, this is the output. Notice that the substitution of
{env:BUILD}works in the first part of the line but not in the second part following the{/}:If I run "tox -e docs" with tox 3.27.1, this is the output:
The text was updated successfully, but these errors were encountered: