Skip to content
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 --showconfig/--help-ini with -v emits invalid ini content #2622

Closed
cboylan opened this issue Dec 7, 2022 · 4 comments · Fixed by #2652
Closed

Tox 4 --showconfig/--help-ini with -v emits invalid ini content #2622

cboylan opened this issue Dec 7, 2022 · 4 comments · Fixed by #2652
Labels
area:documentation help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted.

Comments

@cboylan
Copy link

cboylan commented Dec 7, 2022

Issue

Tox 4 --showconfig/--help-ini with -v emits invalid ini content. Under tox 3.x this worked if you ignored the first couple of lines of output, but under v4 the extra debugging info is interleaved with the ini content making this much more difficult.

I think there are a few possible options to handle this:

  • Don't emit extra debug info when running --showconfig.
  • Write extra debug info to stderr instead of stdout so the output streams can be separated.
  • Prefix the extra debug info with ini comments causing the ini parser to ignore them.

This problem gets worse as you add -v flags too.

Environment

Provide at least:

  • OS: OpenSUSE Tumbleweed
  • pip list of the host Python where tox is installed:
Package       Version
------------- -------
cachetools    5.2.0
chardet       5.1.0
colorama      0.4.6
distlib       0.3.6
filelock      3.8.2
packaging     21.3
pip           22.2.2
platformdirs  2.6.0
pluggy        1.0.0
pyparsing     3.0.9
pyproject_api 1.2.1
setuptools    63.2.0
tomli         2.0.1
tox           4.0.0
virtualenv    20.17.1

[notice] A new release of pip available: 22.2.2 -> 22.3.1
[notice] To update, run: python3 -m pip install --upgrade pip

Minimal example

If possible, provide a minimal reproducer for the issue:

cat tox.ini

[tox]
minversion = 1.6
envlist = foo

[testenv]
basepython = python3
allowlist_externals = bash
commands = bash -c 'echo foo'

[testenv:foo]
deps = bindep

tox -c tox.ini -v --showconfig

[testenv:foo]
type = VirtualEnvRunner
set_env = PIP_DISABLE_PIP_VERSION_CHECK=1
base = testenv
runner = virtualenv
description = 
depends = 
env_name = foo
labels = 
env_dir = /home/clark/tmp/.tox/foo
env_tmp_dir = /home/clark/tmp/.tox/foo/tmp
env_log_dir = /home/clark/tmp/.tox/foo/log
suicide_timeout = 0.0
interrupt_timeout = 0.3
terminate_timeout = 0.2
platform = 
pass_env =
  CURL_CA_BUNDLE
  LANG
  LANGUAGE
  LD_LIBRARY_PATH
  PIP_*
  REQUESTS_CA_BUNDLE
  SSL_CERT_FILE
  TERM
  TMPDIR
  VIRTUALENV_*
  http_proxy
  https_proxy
  no_proxy
parallel_show_output = False
recreate = False
allowlist_externals = bash
pip_pre = False
ROOT: find interpreter for spec PythonSpec(major=3)
ROOT: proposed PythonInfo(spec=CPython3.10.8.final.0-64, system=/usr/bin/python3.10, exe=/home/clark/tmp/toxenv/bin/python3, platform=linux, version='3.10.8 (main, Oct 19 2022, 07:46:20) [GCC]', encoding_fs_io=utf-8-utf-8)
install_command = python -I -m pip install '{packages}'
list_dependencies_command = python -m pip freeze --all
commands_pre = 
commands = bash -c 'echo foo'
commands_post = 
change_dir = /home/clark/tmp
args_are_paths = True
ignore_errors = False
ignore_outcome = False
base_python = python3
env_site_packages_dir = /home/clark/tmp/.tox/foo/lib/python3.10/site-packages
env_bin_dir = /home/clark/tmp/.tox/foo/bin
env_python = /home/clark/tmp/.tox/foo/bin/python
deps = bindep
system_site_packages = False
always_copy = False
download = False
skip_install = False
use_develop = False
package = skip

[tox]
config_file_path = tox.ini
tox_root = /home/clark/tmp
work_dir = /home/clark/tmp/.tox
temp_dir = /home/clark/tmp/.tmp
env_list = foo
base = 
min_version = 1.6
provision_tox_env = .tox
requires = tox>=1.6
labels = 
ignore_base_python_conflict = False
skip_missing_interpreters = True
no_package = False

Note the ROOT: lines that originate from -v. Since there are multiple entries with the same prefix under [testenv:foo] configparser complains that there are duplicates and raises an exception when parsing this content:

import configparser
c = configparser.RawConfigParser()
c.read('shownconfig.ini')

Produces:

Traceback (most recent call last):
  File "/home/clark/tmp/bar.py", line 3, in <module>
    c.read('shownconfig.ini')
  File "/usr/lib64/python3.10/configparser.py", line 698, in read
    self._read(fp, filename)
  File "/usr/lib64/python3.10/configparser.py", line 1097, in _read
    raise DuplicateOptionError(sectname, optname,
configparser.DuplicateOptionError: While reading from 'shownconfig.ini' [line 35]: option 'root' in section 'testenv:foo' already exists
@gaborbernat gaborbernat added this to the 4.0.x milestone Dec 7, 2022
@gaborbernat
Copy link
Member

gaborbernat commented Dec 7, 2022

Have you considered not adding the -v argument instead?

Under tox 3.x this worked if you ignored the first couple of lines of output,

Perhaps, but was never supported...

@cboylan
Copy link
Author

cboylan commented Dec 7, 2022

Yes, that is what we have done to work around this. It just seems odd that the command to emit ini content doesn't do that. I think there are a number of good ways to address that as I've mentioned above.

@gaborbernat
Copy link
Member

gaborbernat commented Dec 7, 2022

Don't emit extra debug info when running --showconfig.

This is not a good idea because what if we fail during the run, we still need logs to find out what went wrong.

Write extra debug info to stderr instead of stdout so the output streams can be separated.

This would be just as much a breaking change, and many tools consider stderr as failure, so would fail other CIs.

Prefix the extra debug info with ini comments causing the ini parser to ignore them.

For the reporting engine, we don't have a config mode and a non-config mode, so all reports would be prefixed with a comment, which again would be a breaking change. So you see, neither of your proposals is tenable. The easiest fix is:

Have you considered not adding the -v argument instead?

You've been using an unsupported behavior, and now it broke. You can't complain about it, that now you need to make changes to be in supported land. Especially considering the major release versioning behind what it was done.

@gaborbernat gaborbernat added area:documentation help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted. and removed needs:more-info labels Dec 8, 2022
jugmac00 added a commit to jugmac00/tox that referenced this issue Dec 8, 2022
So if you want to generate a valid ini file, you must not use verbose
mode.

This fixes tox-dev#2622.
jugmac00 added a commit to jugmac00/tox that referenced this issue Dec 8, 2022
So if you want to generate a valid ini file, you must not use verbose
mode.

This fixes tox-dev#2622.
jugmac00 added a commit to jugmac00/tox that referenced this issue Dec 9, 2022
So if you want to generate a valid ini file, you must not use verbose
mode.

This fixes tox-dev#2622.
@ssbarnea
Copy link
Member

ssbarnea commented Mar 5, 2023

To be honest, I @cboylan assumption that the option that outputs an INI file is perfectly normal. The bug is that tox 4 still abuses stdout for logging purposes instead of using stderr for it.

This behavior also broke one of my small mk utilities as the output is no longer valid ini, so I cannot load it. Lucky in my case adding -qq worked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:documentation help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants