Skip to content
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/changelog/2435.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a regression issue related to inability to use ``file|`` substitution option in nested ``set_env`` sections of ``ini`` configurations since tox4 update.
31 changes: 23 additions & 8 deletions src/tox/config/set_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ def __init__( # noqa: C901, PLR0912
return
for line in raw.splitlines(): # noqa: PLR1702
if line.strip():
if line.startswith("file|"): # environment files to be handled later
self._env_files.append(line[len("file|") :])
if self._is_file_line(line):
self._env_files.append(self._parse_file_line(line))
else:
try:
key, value = self._extract_key_value(line)
Expand All @@ -52,12 +52,20 @@ def __init__( # noqa: C901, PLR0912
else:
self._raw[key] = value

@staticmethod
def _is_file_line(line: str) -> bool:
return line.startswith("file|")

@staticmethod
def _parse_file_line(line: str) -> str:
return line[len("file|") :]

def use_replacer(self, value: Replacer, args: ConfigLoadArgs) -> None:
self._replacer = value
for filename in self._env_files:
self._read_env_file(filename, args)
self._raw.update(self._stream_env_file(filename, args))

def _read_env_file(self, filename: str, args: ConfigLoadArgs) -> None:
def _stream_env_file(self, filename: str, args: ConfigLoadArgs) -> Iterator[tuple[str, str]]:
# Our rules in the documentation, some upstream environment file rules (we follow mostly the docker one):
# - https://www.npmjs.com/package/dotenv#rules
# - https://docs.docker.com/compose/env-file/
Expand All @@ -70,8 +78,7 @@ def _read_env_file(self, filename: str, args: ConfigLoadArgs) -> None:
env_line = env_line.strip() # noqa: PLW2901
if not env_line or env_line.startswith("#"):
continue
key, value = self._extract_key_value(env_line)
self._raw[key] = value
yield self._extract_key_value(env_line)

@staticmethod
def _extract_key_value(line: str) -> tuple[str, str]:
Expand Down Expand Up @@ -100,10 +107,18 @@ def __iter__(self) -> Iterator[str]:
# start with the materialized ones, maybe we don't need to materialize the raw ones
yield from self._materialized.keys()
yield from list(self._raw.keys()) # iterating over this may trigger materialization and change the dict
args = ConfigLoadArgs([], self._name, self._env_name)
while self._needs_replacement:
line = self._needs_replacement.pop(0)
expanded_line = self._replacer(line, ConfigLoadArgs([], self._name, self._env_name))
sub_raw = dict(self._extract_key_value(sub_line) for sub_line in expanded_line.splitlines() if sub_line)
expanded_line = self._replacer(line, args)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain the change here and why you done it this way?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • [+] wrote descriptive pull request text

This part you didn't really address. Writing a descriptive text involves the approach you've taken and why.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's ConfigLoadArgs variable is being used twice now since the call to _stream_env_file(_read_env_file) requires it. Seems like no point in instantiating it twice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the description is pointing out to the long standing regression issue that wasn't fixed yet, this is an attempt to do it, since it was stated there "PR welcome to fix it.", what else could be said. 🤷

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's ConfigLoadArgs variable is being used twice now since the call to _stream_env_file(_read_env_file) requires it

Was looking for an explanation for this, as in why we need to call it twice now?

sub_raw: dict[str, str] = {}
for sub_line in filter(None, expanded_line.splitlines()):
if not self._is_file_line(sub_line):
sub_raw.__setitem__(*self._extract_key_value(sub_line))
else:
for key, value in self._stream_env_file(self._parse_file_line(sub_line), args):
if key not in self._raw:
sub_raw[key] = value # noqa: PERF403
self._raw.update(sub_raw)
self.changed = True # loading while iterating can cause these values to be missed
yield from sub_raw.keys()
Expand Down
48 changes: 48 additions & 0 deletions tests/config/test_set_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -240,3 +240,51 @@ def test_set_env_environment_file_missing(tox_project: ToxProjectCreator) -> Non
result = project.run("r")
result.assert_failed()
assert f"py: failed with {project.path / 'magic.txt'} does not exist for set_env" in result.out


# https://github.com/tox-dev/tox/issues/2435
def test_set_env_environment_with_file_and_expanded_substitution(
tox_project: ToxProjectCreator, monkeypatch: MonkeyPatch
) -> None:
conf = {
"tox.ini": """
[tox]
envlist =
check

[testenv]
setenv =
file|.env
PRECENDENCE_TEST_1=1_expanded_precedence

[testenv:check]
setenv =
{[testenv]setenv}
PRECENDENCE_TEST_1=1_self_precedence
PRECENDENCE_TEST_2=2_self_precedence
""",
".env": """
PRECENDENCE_TEST_1=1_file_precedence
PRECENDENCE_TEST_2=2_file_precedence
PRECENDENCE_TEST_3=3_file_precedence
""",
}
monkeypatch.setenv("env_file", ".env")
project = tox_project(conf)

result = project.run("c", "-k", "set_env", "-e", "check")
result.assert_success()
set_env = result.env_conf("check")["set_env"]
content = {k: set_env.load(k) for k in set_env}
assert content == {
"PIP_DISABLE_PIP_VERSION_CHECK": "1",
"PYTHONHASHSEED": ANY,
"PYTHONIOENCODING": "utf-8",
"PRECENDENCE_TEST_1": "1_expanded_precedence",
"PRECENDENCE_TEST_2": "2_self_precedence",
"PRECENDENCE_TEST_3": "3_file_precedence",
}

result = project.run("r", "-e", "check")
result.assert_success()
assert "check: OK" in result.out