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_CONFIG_FILE overrides local config #2890

Closed
kpfleming opened this issue Jan 24, 2023 · 6 comments · Fixed by #2896
Closed

TOX_CONFIG_FILE overrides local config #2890

kpfleming opened this issue Jan 24, 2023 · 6 comments · Fixed by #2896

Comments

@kpfleming
Copy link

Issue

TOX_CONFIG_FILE is documented to be a way to specify a user-level configuration file, to set defaults to be used. There is a default for the configuration file as well, but the behavior differs when the file is automatically discovered vs. being explicitly specified.

In the examples below, the A and B outputs are correct, but the C output is not, even though TOX_CONFIG_FILE is set to point to the same file that is automatically used when it is not set.

Environment

Provide at least:

  • OS: Debian Bookworm
  • tox version: 4.3.5 installed using pipx

Minimal example

$ mkdir foo
$ cd foo
$ cat > tox.ini << EOF
[tox]

[testenv:foo]

[testenv:bar]

[testenv:baz]
EOF

$ tox list (A)
default environments:
py  -> [no description]

additional environments:
foo -> [no description]
bar -> [no description]
baz -> [no description]

$ mkdir $HOME/.config/tox
$ cat > $HOME/.config/tox/config.ini << EOF
[tox]
workdir=/tmp
EOF

$ tox list (B)
default environments:
py  -> [no description]

additional environments:
foo -> [no description]
bar -> [no description]
baz -> [no description]

$ export TOX_CONFIG_FILE=$HOME/.config/tox/config.ini

$ tox list (C)
default environments:
py  -> [no description]
@kpfleming
Copy link
Author

Note: 'work_dir' is misspelled in the user-level config file in the example but the behavior does not change if it is spelled correctly.

@masenf
Copy link
Collaborator

masenf commented Jan 24, 2023

confirmed, reproducible on tox 4.3.5

@masenf
Copy link
Collaborator

masenf commented Jan 25, 2023

The workaround here is to specify

TOX_CONFIG_FILE=path/to/override/config.ini tox -c ./tox.ini

I believe the crux of the problem is that tox is overloading the name config_file in the parser:

dest="config_file",

☝️ here config_file refers to the main tox.ini

self.config_file = Path(config_file if config_file is not None else DEFAULT_CONFIG_FILE)

☝️ here config_file refers to the override config

still unteasing the true root cause.

@masenf
Copy link
Collaborator

masenf commented Jan 25, 2023

There's a fundamental incompatibility here, where tox implicitly looks for TOX_{key_upper}, and finds TOX_CONFIG_FILE as the main config file, but also explicitly looks for TOX_CONFIG_FILE and sets that as the "user config".

for environ_key in (f"TOX_{key_upper}", f"TOX{key_upper}"):

I propose that we rename the user config envvar to TOX_USER_CONFIG_FILE and save TOX_CONFIG_FILE to refer to the main tox.ini.

diff --git a/src/tox/config/cli/ini.py b/src/tox/config/cli/ini.py
index 8839f430..c1823a1d 100644
--- a/src/tox/config/cli/ini.py
+++ b/src/tox/config/cli/ini.py
@@ -19,7 +19,7 @@ DEFAULT_CONFIG_FILE = Path(user_config_dir("tox")) / "config.ini"
 
 
 class IniConfig:
-    TOX_CONFIG_FILE_ENV_VAR = "TOX_CONFIG_FILE"
+    TOX_CONFIG_FILE_ENV_VAR = "TOX_USER_CONFIG_FILE"
     STATE = {None: "failed to parse", True: "active", False: "missing"}
 
     def __init__(self) -> None:

With this change, passing TOX_USER_CONFIG_FILE=path/to/override/config.ini tox works just fine.

So the fix here could be as simple as renaming this conflicting variable and updating the configuration. Should I proceed @gaborbernat?

masenf added a commit to masenf/tox that referenced this issue Jan 25, 2023
Allow overriding the user config file without overriding the main config file.

Fix tox-dev#2890
@gaborbernat
Copy link
Member

Yeah please.

@kpfleming
Copy link
Author

@masenf Thank you for tracking this down. The root cause seems so obvious in hindsight :-)

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

Successfully merging a pull request may close this issue.

3 participants