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

Regression: specifying workdir is treated like root #2713

Closed
jaraco opened this issue Dec 14, 2022 · 13 comments
Closed

Regression: specifying workdir is treated like root #2713

jaraco opened this issue Dec 14, 2022 · 13 comments
Labels
bug:minor does not affect many people or has no big impact help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted.

Comments

@jaraco
Copy link

jaraco commented Dec 14, 2022

In pypa/twine#684 (comment), we learned that since tox 4, invoking tox from a tox environment fails to create the environment. Here's a minimal reproducer:

 draft $ cat > tox.ini
[testenv:parent]
deps=tox
allowlist_externals=ls
commands=
	python -m tox -e child --notest
	ls .tox/child

[testenv:child]
 draft $ tox -e parent
parent: commands[0]> python -m tox -e child
  child: OK (0.02 seconds)
  congratulations :) (0.05 seconds)
parent: commands[1]> ls .tox/child
ls: .tox/child: No such file or directory
parent: exit 1 (0.01 seconds) /Users/jaraco/draft> ls .tox/child pid=69335
  parent: FAIL code 1 (0.18=setup[0.02]+cmd[0.15,0.01] seconds)
  evaluation failed :( (0.21 seconds)

I suspect one or more of the environment variables supplied in the parent env are affecting the invocation of tox and preventing the child env from being created.

@jaraco
Copy link
Author

jaraco commented Dec 14, 2022

This tox file runs successfully and illustrates how the issue is new with tox 4:

[testenv:parent]
deps=tox<4
allowlist_externals=ls
commands=
	python -m tox -e child --notest
	ls .tox/child

[testenv:child]
skip_install=true

@gaborbernat gaborbernat added this to the 4.0.x milestone Dec 14, 2022
@gaborbernat gaborbernat added bug:minor does not affect many people or has no big impact help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted. labels Dec 14, 2022
@gaborbernat
Copy link
Member

Feel free to submit a PR to address this 👍

@jabbera
Copy link

jabbera commented Dec 14, 2022

@jaraco the environment is getting created but in a different location: ls .tox/.tox/child

@jaraco
Copy link
Author

jaraco commented Dec 14, 2022

I suspect the following lines are related (where .tox is appended to work dir).

tox/src/tox/config/sets.py

Lines 191 to 192 in b8b0803

def work_dir_builder(conf: Config, env_name: str | None) -> Path: # noqa: U100
return (conf.work_dir if conf.work_dir is not None else cast(Path, self["tox_root"])) / ".tox"

My guess is the appending happens in the parent tox env, and then sets an env var, and then the child invocation adds it again.

Possibly related: #2654

@jaraco
Copy link
Author

jaraco commented Dec 14, 2022

Also possibly related: #2473

@jaraco
Copy link
Author

jaraco commented Dec 14, 2022

I figured out this fixes it:

 tox main $ git diff
diff --git a/src/tox/config/sets.py b/src/tox/config/sets.py
index e07f4bd4..cdacebfc 100644
--- a/src/tox/config/sets.py
+++ b/src/tox/config/sets.py
@@ -189,7 +189,7 @@ class CoreConfigSet(ConfigSet):
         )
 
         def work_dir_builder(conf: Config, env_name: str | None) -> Path:  # noqa: U100
-            return (conf.work_dir if conf.work_dir is not None else cast(Path, self["tox_root"])) / ".tox"
+            return conf.work_dir if conf.work_dir is not None else cast(Path, self["tox_root"]) / ".tox"
 
         self.add_config(
             keys=["work_dir", "toxworkdir"],

@jaraco
Copy link
Author

jaraco commented Dec 14, 2022

Unfortunately, I also get 4 new test failures when applying that patch, none of which are failing for reasons I can readily understand.

@jaraco
Copy link
Author

jaraco commented Dec 14, 2022

According to the docs, tox_root is where the configuration file is found and work_dir is {tox_root}/.tox.

image

But the help tells a different story:

$ tox --help | grep dir
usage: tox [-h] [--colored {yes,no}] [-v | -q] [--exit-and-dump-after seconds] [-c file] [--workdir dir] [--root dir] [--runner {virtualenv}] [--version] [--no-provision [REQ_JSON]] [--no-recreate-provision] [-r] [-x OVERRIDE]
  --workdir dir               tox working directory (if not specified will be the folder of the config file) (default: None)
  --root dir                  project root directory (if not specified will be the folder of the config file) (default: None)

@gaborbernat
Copy link
Member

The bug is in the help message.

@jaraco
Copy link
Author

jaraco commented Dec 15, 2022

It took me a while to trace, but I found where work_dir is getting set to the same thing as root if work_dir wasn't supplied.

root: Path = source.path.parent if parsed.root_dir is None else parsed.root_dir
work_dir: Path = source.path.parent if parsed.work_dir is None else parsed.work_dir

The only reason that didn't cause problems was because work_dir_builder was unconditionally adding .tox even when work dir was supplied:

tox/src/tox/config/sets.py

Lines 191 to 192 in 12f6268

def work_dir_builder(conf: Config, env_name: str | None) -> Path: # noqa: U100
return (conf.work_dir if conf.work_dir is not None else cast(Path, self["tox_root"])) / ".tox"

jaraco added a commit to jaraco/tox that referenced this issue Dec 15, 2022
@jaraco
Copy link
Author

jaraco commented Dec 15, 2022

Indeed, this issue is more serious than just with nested environments. Any environment that passes workdir fails by putting an extra .tox on every work dir:

 tox main $ git clean -fdx
 tox main $ tox --version
4.0.8 from /Users/jaraco/.local/pipx/venvs/tox/lib/python3.11/site-packages/tox/__init__.py
 tox main $ tox --workdir foo --notest -q
  python: OK (10.75 seconds)
  congratulations :) (10.79 seconds)
 tox main $ ls -la foo
total 0
drwxr-xr-x   3 jaraco  primarygroup   96 Dec 14 19:56 .
drwxr-xr-x  20 jaraco  primarygroup  640 Dec 14 19:56 ..
drwxr-xr-x   5 jaraco  primarygroup  160 Dec 14 19:56 .tox

@jaraco jaraco changed the title Regression: nested environments fail to create Regression: specifying workdir is treated like root Dec 15, 2022
@jaraco
Copy link
Author

jaraco commented Dec 15, 2022

I applied this diff and no new tests fail:

diff --git a/src/tox/config/sets.py b/src/tox/config/sets.py
index e07f4bd4..49625763 100644
--- a/src/tox/config/sets.py
+++ b/src/tox/config/sets.py
@@ -189,6 +189,7 @@ class CoreConfigSet(ConfigSet):
         )
 
         def work_dir_builder(conf: Config, env_name: str | None) -> Path:  # noqa: U100
+            assert conf.work_dir is not None
             return (conf.work_dir if conf.work_dir is not None else cast(Path, self["tox_root"])) / ".tox"
 
         self.add_config(

That means that the resolution of the work dir relative to the root is never used.

@jaraco
Copy link
Author

jaraco commented Dec 15, 2022

Indeed, this issue is just a dupe of #2654.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug:minor does not affect many people or has no big impact help:wanted Issues that have been acknowledged, a solution determined and a PR might likely be accepted.
Projects
None yet
Development

No branches or pull requests

3 participants