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

Fix regression with multiple env substitutions for the same key #2873

Merged
merged 3 commits into from
Jan 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/2869.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Fix regression introduced in 4.3.0 which occured when a substitution expression
for an environment variable that had previously been substituted appears in the
ini file after a substitution expression for a different environment variable.
This situation erroneously resulted in an exception about "circular chain
between set" of those variables - by :user:`masenf`.
11 changes: 7 additions & 4 deletions src/tox/config/loader/ini/replace.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,29 +178,32 @@ def join(self, value: MatchArg) -> str:
return "".join(self(value))

def _replace_match(self, value: MatchExpression) -> str:
# use a copy of conf_args so any changes from this replacement do NOT
# affect adjacent substitutions (#2869)
conf_args = self.conf_args.copy()
of_type, *args = flattened_args = [self.join(arg) for arg in value.expr]
if of_type == "/":
replace_value: str | None = os.sep
elif of_type == "" and args == [""]:
replace_value = os.pathsep
elif of_type == "env":
replace_value = replace_env(self.conf, args, self.conf_args)
replace_value = replace_env(self.conf, args, conf_args)
elif of_type == "tty":
replace_value = replace_tty(args)
elif of_type == "posargs":
replace_value = replace_pos_args(self.conf, args, self.conf_args)
replace_value = replace_pos_args(self.conf, args, conf_args)
else:
replace_value = replace_reference(
self.conf,
self.loader,
ARG_DELIMITER.join(flattened_args),
self.conf_args,
conf_args,
)
if replace_value is not None:
needs_expansion = any(isinstance(m, MatchExpression) for m in find_replace_expr(replace_value))
if needs_expansion:
try:
return replace(self.conf, self.loader, replace_value, self.conf_args, self.depth + 1)
return replace(self.conf, self.loader, replace_value, conf_args, self.depth + 1)
except MatchRecursionError as err:
LOGGER.warning(str(err))
return replace_value
Expand Down
40 changes: 38 additions & 2 deletions tests/config/loader/ini/replace/test_replace_env_var.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import pytest

from tests.config.loader.ini.replace.conftest import ReplaceOne
from tox.pytest import MonkeyPatch
from tox.pytest import LogCaptureFixture, MonkeyPatch


def test_replace_env_set(replace_one: ReplaceOne, monkeypatch: MonkeyPatch) -> None:
Expand Down Expand Up @@ -75,6 +75,20 @@ def test_replace_env_missing_default_from_env(replace_one: ReplaceOne, monkeypat
assert result == "yes"


def test_replace_env_var_multiple(replace_one: ReplaceOne, monkeypatch: MonkeyPatch) -> None:
"""Multiple env substitutions on a single line."""
monkeypatch.setenv("MAGIC", "MAGIC")
monkeypatch.setenv("TRAGIC", "TRAGIC")
result = replace_one("{env:MAGIC} {env:TRAGIC} {env:MAGIC}")
assert result == "MAGIC TRAGIC MAGIC"


def test_replace_env_var_multiple_default(replace_one: ReplaceOne) -> None:
"""Multiple env substitutions on a single line with default values."""
result = replace_one("{env:MAGIC:foo} {env:TRAGIC:bar} {env:MAGIC:baz}")
assert result == "foo bar baz"


def test_replace_env_var_circular(replace_one: ReplaceOne, monkeypatch: MonkeyPatch) -> None:
"""Replacement values will not infinitely loop"""
monkeypatch.setenv("MAGIC", "{env:MAGIC}")
Expand All @@ -97,12 +111,34 @@ def avoid_infinite_loop() -> None: # pragma: no cover


@pytest.mark.usefixtures("reset_env_var_after_delay")
def test_replace_env_var_circular_flip_flop(replace_one: ReplaceOne, monkeypatch: MonkeyPatch) -> None:
def test_replace_env_var_circular_flip_flop(
replace_one: ReplaceOne,
monkeypatch: MonkeyPatch,
caplog: LogCaptureFixture,
) -> None:
"""Replacement values will not infinitely loop back and forth"""
monkeypatch.setenv("TRAGIC", "{env:MAGIC}")
monkeypatch.setenv("MAGIC", "{env:TRAGIC}")
result = replace_one("{env:MAGIC}")
assert result == "{env:MAGIC}"
assert "circular chain between set env MAGIC, TRAGIC" in caplog.messages


@pytest.mark.usefixtures("reset_env_var_after_delay")
def test_replace_env_var_circular_flip_flop_5(
replace_one: ReplaceOne,
monkeypatch: MonkeyPatch,
caplog: LogCaptureFixture,
) -> None:
"""Replacement values will not infinitely loop back and forth (longer chain)"""
monkeypatch.setenv("MAGIC", "{env:TRAGIC}")
monkeypatch.setenv("TRAGIC", "{env:RABBIT}")
monkeypatch.setenv("RABBIT", "{env:HAT}")
monkeypatch.setenv("HAT", "{env:TRICK}")
monkeypatch.setenv("TRICK", "{env:MAGIC}")
result = replace_one("{env:MAGIC}")
assert result == "{env:MAGIC}"
assert "circular chain between set env MAGIC, TRAGIC, RABBIT, HAT, TRICK" in caplog.messages


@pytest.mark.parametrize("fallback", [True, False])
Expand Down