-
-
Notifications
You must be signed in to change notification settings - Fork 514
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 v4 does not consistently require to escape the # #2893
Comments
If someone wants to open a PR this would likely be here https://github.com/tox-dev/tox/blob/main/src/tox/config/loader/ini/__init__.py |
I've had my eye on fixing #2365, which after digging in a bit, I've determined there's a problem when combining the new substitutions parser (4.3.x) with the shlex used in command arg splitting. Specifically, the double backslash gets handled during substitution parsing and thus shlex gets a string with a single backslash, even when the user expected that to be an escaped backslash. All that to say, I think at least one problem here is that for commands the substitution parser should leave backslashes alone, but for other strings, then it should follow the current escaping rules. I know one of the problems with my last PR for #2365 was related to handling the comment escapes. Definitely more test cases are needed in this area. This isn't to discourage anyone from taking a stab at a fix, but be wary, since we are definitely missing some test cases in this area, and currently the command parser behaves differently on windows vs Linux (and we don't have tests for those differences) |
@lmazuel do you have a minimal reproducer? I tried running with this [testenv]
deps =
pylint-guidelines-checker @ git+https://github.com/Azure/azure-sdk-for-python\#subdirectory=scripts/pylint_custom_plugin
commands = pip freeze On windows and linux with tox-4.3.5 and tox-4.4.2, I don't seem to reproduce the issue. |
@masenf sorry for the late answer, I paused my migration tox v4 for now. I can only tell you at this point this was a Azure DevOps job, likely on Linux that was showcasing this issue. I should have more time soon (worst this summer), to complete the tox v4 migration and retry it. |
Issue
Using this in
deps
:works like a charm, but I was assuming I should escape based on the tox v4 updating doc to:
But this one actually crashes tox. My CI unfortunately didn't kept the log, but the message is something like
cannot clone https://github.com/Azure/azure-sdk-for-python\
(note the\
at the end). If that matters, I can re-crash my CI on purpose to get the real error message.I expect that when the tox updating doc says "The hash sign (#) now always acts as comment within tox.ini", then it's the case.
Environment
Provide at least:
pip list
of the host Python wheretox
is installed:Jinja2-3.1.2 MarkupSafe-2.1.2 astroid-2.13.3 attrs-22.2.0 autorest-0.1.0 black-22.12.0 cachetools-5.3.0 chardet-5.1.0 click-8.1.3 colorama-0.4.6 dill-0.3.6 distlib-0.3.6 docutils-0.19 exceptiongroup-1.1.0 filelock-3.9.0 importlib-metadata-6.0.0 iniconfig-2.0.0 invoke-2.0.0 isort-5.11.4 json-rpc-1.14.0 lazy-object-proxy-1.9.0 m2r2-0.3.3 mccabe-0.7.0 mistune-0.8.4 mypy-0.991 mypy-extensions-0.4.3 nodeenv-1.7.0 packaging-23.0 pathspec-0.10.3 platformdirs-2.6.2 pluggy-1.0.0 ptvsd-4.3.2 pylint-2.15.10 pyproject-api-1.5.0 pyright-1.1.287 pytest-7.2.1 pyyaml-6.0 tomli-2.0.1 tomlkit-0.11.6 tox-4.3.5 typed-ast-1.5.4 types-PyYAML-6.0.12.3 typing-extensions-4.4.0 virtualenv-20.17.1 wrapt-1.14.1 zipp-3.11.0
I'm not blocked since I removed the backslah and it works.
The text was updated successfully, but these errors were encountered: