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

Plugins cannot use custom tox.ini config sections with a suffix name shared by a testenv #2926

Closed
dcrosta opened this issue Feb 14, 2023 · 2 comments

Comments

@dcrosta
Copy link
Member

dcrosta commented Feb 14, 2023

Issue

I noticed this with recent tox 4.x failing in CI for tox-docker plugin, which preivously worked.

The issue seems to be deep inside the config parsing code, if both a [testenv:foo] and [docker:foo] section exist with the same suffix ("foo"). Somewhere inside the parser code it seems to get confused and is looking for the docker plugin's keys within the testenv config section, and fails.

Here's a minimal(ish) script & tox.ini which reproduces it:

tox.ini

[testenv:foo]
custom = foo
command = python -c ""

[custom:foo]
bar = baz

tox_bug.py

from typing import List
from tox.config.loader.section import Section
from tox.config.sets import ConfigSet
from tox.config.sets import EnvConfigSet
from tox.plugin import impl
from tox.session.state import State
from tox.tox_env.api import ToxEnv


def bar_required(bar: str) -> str:
    assert bar
    return bar


class CustomConfigSet(ConfigSet):
    def register_config(self) -> None:
        self.add_config(
            keys=["bar"],
            of_type=str,
            default="",
            desc="description",
            post_process=bar_required,
        )

def parse_custom_config(custom_conf: CustomConfigSet) -> None:
    print(custom_conf["bar"])


@impl
def tox_before_run_commands(tox_env: ToxEnv) -> None:
    custom_configs = tox_env.conf.load("custom")
    # breakpoint()
    for conf in custom_configs:
        parse_custom_config(conf)


@impl
def tox_add_env_config(env_conf: EnvConfigSet, state: State) -> None:
    def build_config_set(suffix: object) -> CustomConfigSet:
        assert isinstance(suffix, str)
        custom_conf = state.conf.get_section_config(
            section=Section("custom", suffix),
            base=[],
            of_type=CustomConfigSet,
            for_env=None,
        )
        if not custom_conf.loaders:
            raise ValueError(f"Missing [custom:{suffix}] in tox.ini")
        return custom_conf

    env_conf.add_config(
        keys=["custom"],
        of_type=List[CustomConfigSet],
        default=[],
        desc="description",
        factory=build_config_set,
    )

setup.py

from setuptools import setup

setup(
    name="tox-bug",
    description="desc",
    maintainer="Dan Crosta",
    maintainer_email="dcrosta@example.com",
    py_modules=["tox_bug"],
    entry_points={"tox": ["bug = tox_bug"]},
)

You can reproduce with python setup.py develop && tox -e foo:

dcrosta@botfly.local:~/src/tox-docker-ini-bug $ tox -e foo
.pkg: _optional_hooks> python /Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: get_requires_for_build_sdist> python /Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: prepare_metadata_for_build_wheel> python /Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: build_sdist> python /Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
foo: install_package> python -I -m pip install --force-reinstall --no-deps /Users/dcrosta/src/tox-docker-ini-bug/.tox/.tmp/package/9/tox-bug-0.0.0.tar.gz
foo: internal error
Traceback (most recent call last):
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/tox/session/cmd/run/single.py", line 46, in _evaluate
    code, outcomes = run_commands(tox_env, no_test)
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/tox/session/cmd/run/single.py", line 77, in run_commands
    MANAGER.tox_before_run_commands(tox_env)
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/tox/plugin/manager.py", line 75, in tox_before_run_commands
    self.manager.hook.tox_before_run_commands(tox_env=tox_env)
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pluggy/_hooks.py", line 265, in __call__
    return self._hookexec(self.name, self.get_hookimpls(), kwargs, firstresult)
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pluggy/_manager.py", line 80, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pluggy/_callers.py", line 60, in _multicall
    return outcome.get_result()
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pluggy/_result.py", line 60, in get_result
    raise ex[1].with_traceback(ex[2])
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pluggy/_callers.py", line 39, in _multicall
    res = hook_impl.function(*args)
  File "/Users/dcrosta/src/tox-docker-ini-bug/tox_bug.py", line 34, in tox_before_run_commands
    parse_custom_config(conf)
  File "/Users/dcrosta/src/tox-docker-ini-bug/tox_bug.py", line 26, in parse_custom_config
    print(custom_conf["bar"])
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/tox/config/sets.py", line 114, in __getitem__
    return self.load(item)
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/tox/config/sets.py", line 125, in load
    return config_definition.__call__(self._conf, self.loaders, ConfigLoadArgs(chain, self.name, self.env_name))
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/tox/config/of_type.py", line 112, in __call__
    value = self.post_process(value)
  File "/Users/dcrosta/src/tox-docker-ini-bug/tox_bug.py", line 11, in bar_required
    assert bar
AssertionError
.pkg: _exit> python /Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
  foo: FAIL code 2 (2.81 seconds)
  evaluation failed :( (2.84 seconds)

That should pass with, as the plugin is trying to read the bar = baz line of config, then assert that it gets a truthy value.

Environment

It seems this issue was introduced in 4.0.0rc1:

dcrosta@botfly.local:~/src/tox-docker-ini-bug $ pip install --pre tox==4.0.0rc1
# ...
Successfully installed tox-4.0.0rc1
dcrosta@botfly.local:~/src/tox-docker-ini-bug $ tox -e foo
.pkg: _optional_hooks> python /Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: get_requires_for_build_sdist> python /Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: prepare_metadata_for_build_wheel> python /Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: build_sdist> python /Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
foo: install_package> python -I -m pip install --force-reinstall --no-deps /Users/dcrosta/src/tox-docker-ini-bug/.tox/.pkg/dist/tox-bug-0.0.0.tar.gz
foo: internal error
Traceback (most recent call last):
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/tox/session/cmd/run/single.py", line 46, in _evaluate
    code, outcomes = run_commands(tox_env, no_test)
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/tox/session/cmd/run/single.py", line 75, in run_commands
    MANAGER.tox_before_run_commands(tox_env)
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/tox/plugin/manager.py", line 74, in tox_before_run_commands
    self.manager.hook.tox_before_run_commands(tox_env=tox_env)
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pluggy/_hooks.py", line 265, in __call__
    return self._hookexec(self.name, self.get_hookimpls(), kwargs, firstresult)
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pluggy/_manager.py", line 80, in _hookexec
    return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pluggy/_callers.py", line 60, in _multicall
    return outcome.get_result()
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pluggy/_result.py", line 60, in get_result
    raise ex[1].with_traceback(ex[2])
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pluggy/_callers.py", line 39, in _multicall
    res = hook_impl.function(*args)
  File "/Users/dcrosta/src/tox-docker-ini-bug/tox_bug.py", line 34, in tox_before_run_commands
    parse_custom_config(conf)
  File "/Users/dcrosta/src/tox-docker-ini-bug/tox_bug.py", line 26, in parse_custom_config
    print(custom_conf["bar"])
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/tox/config/sets.py", line 113, in __getitem__
    return self.load(item)
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/tox/config/sets.py", line 124, in load
    return config_definition.__call__(self._conf, self.loaders, ConfigLoadArgs(chain, self.name, self.env_name))
  File "/Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/tox/config/of_type.py", line 112, in __call__
    value = self.post_process(value)
  File "/Users/dcrosta/src/tox-docker-ini-bug/tox_bug.py", line 11, in bar_required
    assert bar
AssertionError
.pkg: _exit> python /Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
  foo: FAIL code 2 (2.76 seconds)
  evaluation failed :( (2.78 seconds)

vs

dcrosta@botfly.local:~/src/tox-docker-ini-bug $ pip install --pre tox==4.0.0b3
# ...
Successfully installed tox-4.0.0b3
dcrosta@botfly.local:~/src/tox-docker-ini-bug $ tox -e foo
.pkg: _optional_hooks> python /Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: get_requires_for_build_sdist> python /Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: prepare_metadata_for_build_wheel> python /Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
.pkg: build_sdist> python /Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
foo: install_package> python -I -m pip install --force-reinstall --no-deps /Users/dcrosta/src/tox-docker-ini-bug/.tox/.pkg/dist/tox-bug-0.0.0.tar.gz
baz
.pkg: _exit> python /Users/dcrosta/.pyenv/versions/3.10.1/lib/python3.10/site-packages/pyproject_api/_backend.py True setuptools.build_meta __legacy__
  foo: OK (2.78 seconds)
  congratulations :) (2.80 seconds)
dcrosta@botfly.local:~/src/tox-docker-ini-bug $

Provide at least:

  • OS: macOS 12.6.3
  • Py 3.10.1
  • clean virtualenv with just tox & the plugin shown above
@masenf
Copy link
Collaborator

masenf commented Feb 19, 2023

Thanks for the report.

Now that I'm back from vacation, I'll look into this one.

@masenf
Copy link
Collaborator

masenf commented Feb 21, 2023

The bug potentially exists in these two lines

sections = self._section_mapping.get(section.name)
key = sections[0] if sections else section.key

Regardless of whether the section passed in has a prefix or not, it prefers to look up the section name in the _section_mapping, which only contains tox-recognized testenv sections. So if your custom section has the same name as a testenv section, then tox prefers the testenv section, even when explicitly qualified.

The fix that I have in mind changes the logic here to look up the _section_mapping for the given section's name only when the prefix is None.

I need to further test compatibility concerns, but I think this approach is ideal, beacuse it also allows tox plugins to leverage tox config parsers to read top-level, non-testenv sections that might have names overlapping with testenv:* sections. However, there might be parts of the code that are assuming an empty string prefix would be equivalent to testenv (hopefully not).

PR coming soon.

masenf added a commit to masenf/tox that referenced this issue Feb 21, 2023
masenf added a commit to masenf/tox that referenced this issue Feb 21, 2023
ensure that loaders returned from .ini source are bound
to the correct section prefix, if specified.

add comment explaining why the code must look up the name
in the _section_mapping

fix tox-dev#2926
masenf added a commit to masenf/tox that referenced this issue Feb 21, 2023
ensure that loaders returned from .ini source are bound
to the correct section prefix, if specified.

add comment explaining why the code must look up the name
in the _section_mapping

fix tox-dev#2926
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants