Better handling of build backend without editable support and add --exit-and-dump-after flag (#2597)

Resolves #2567
Resolves #2595

Author: gaborbernat

docs/changelog/2595.bugfix.rst

@@ -0,0 +1,2 @@
+Fix leaking backend processes when the build backend does not support editable wheels and fix failure when multiple
+environments exist that have a build backend that does not support editable wheels - by :user:`gaborbernat`.

docs/changelog/2595.feature.rst

@@ -0,0 +1,2 @@
+Add ``--exit-and-dump-after`` flag that allows automatically killing tox if does not finish within the passed seconds,
+and dump the thread stacks (useful to debug tox when it seemingly hangs) - by :user:`gaborbernat`.

pyproject.toml

@@ -29,8 +29,8 @@ dependencies = [
   "pluggy>=1",
   "pyproject-api>=1.2.1",
   'tomli>=2.0.1; python_version < "3.11"',
-  "virtualenv>=20.17",
-  "filelock>=3.8.1",
+  "virtualenv>=20.17.1",
+  "filelock>=3.8.2",
   'importlib-metadata>=5.1; python_version < "3.8"',
   'typing-extensions>=4.4; python_version < "3.8"',
 ]

src/tox/config/cli/parser.py

@@ -132,6 +132,8 @@ def is_colored(self) -> bool:
         """:return: flag indicating if the output is colored or not"""
         return cast(bool, self.colored == "yes")
 
+    exit_and_dump_after: int
+
 
 ArgumentArgs = Tuple[Tuple[str, ...], Optional[Type[Any]], Dict[str, Any]]
 
@@ -305,9 +307,21 @@ def add_color_flags(parser: ArgumentParser) -> None:
     )
 
 
+def add_exit_and_dump_after(parser: ArgumentParser) -> None:
+    parser.add_argument(
+        "--exit-and-dump-after",
+        dest="exit_and_dump_after",
+        metavar="seconds",
+        default=0,
+        type=int,
+        help="dump tox threads after n seconds and exit the app - useful to debug when tox hangs, 0 means disabled",
+    )
+
+
 def add_core_arguments(parser: ArgumentParser) -> None:
     add_color_flags(parser)
     add_verbosity_flags(parser)
+    add_exit_and_dump_after(parser)
     parser.add_argument(
         "-c",
         "--conf",

src/tox/execute/pep517_backend.py

@@ -90,6 +90,7 @@ def close(self) -> None:
                         execute.process.wait(timeout=0.1)  # pragma: no cover
                     except TimeoutExpired:  # pragma: no cover
                         execute.process.terminate()  # pragma: no cover  # if does not stop on its own kill it
+        self.is_alive = False
 
 
 class LocalSubProcessPep517ExecuteInstance(ExecuteInstance):

src/tox/run.py

@@ -1,6 +1,7 @@
 """Main entry point for tox."""
 from __future__ import annotations
 
+import faulthandler
 import logging
 import os
 import sys
@@ -51,6 +52,8 @@ def setup_state(args: Sequence[str]) -> State:
     # parse CLI arguments
     options = get_options(*args)
     options.parsed.start = start
+    if options.parsed.exit_and_dump_after:
+        faulthandler.dump_traceback_later(timeout=options.parsed.exit_and_dump_after, exit=True)  # pragma: no cover
     # build tox environment config objects
     state = State(options, args)
     return state

src/tox/tox_env/package.py

@@ -33,16 +33,21 @@ def __str__(self) -> str:
         return str(self.path)
 
 
+locked = False
+
+
 def _lock_method(thread_lock: RLock, file_lock: FileLock | None, meth: Callable[..., Any]) -> Callable[..., Any]:
     def _func(*args: Any, **kwargs: Any) -> Any:
         with thread_lock:
+            file_locks = False
             if file_lock is not None and file_lock.is_locked is False:  # file_lock is to lock from other tox processes
                 file_lock.acquire()
+                file_locks = True
             try:
                 return meth(*args, **kwargs)
             finally:
-                if file_lock is not None:
-                    file_lock.release()
+                if file_locks:
+                    cast(FileLock, file_lock).release()
 
     return _func
 
@@ -94,8 +99,7 @@ def mark_active_run_env(self, run_env: RunToxEnv) -> None:
 
     def teardown_env(self, conf: EnvConfigSet) -> None:
         self._envs.remove(conf.name)
-        has_envs = bool(self._envs)
-        if not has_envs:
+        if len(self._envs) == 0:
             self._teardown()
 
     @abstractmethod

src/tox/tox_env/python/api.py

@@ -108,9 +108,9 @@ def extract_base_python(env_name: str) -> str | None:
         candidates: list[str] = []
         for factor in env_name.split("-"):
             spec = PythonSpec.from_string_spec(factor)
-            if spec.implementation is not None:
-                if spec.implementation.lower() in INTERPRETER_SHORT_NAMES and env_name is not None:
-                    candidates.append(factor)
+            impl = spec.implementation or "python"
+            if impl.lower() in INTERPRETER_SHORT_NAMES and env_name is not None and spec.path is None:
+                candidates.append(factor)
         if candidates:
             if len(candidates) > 1:
                 raise ValueError(f"conflicting factors {', '.join(candidates)} in {env_name}")
@@ -123,18 +123,19 @@ def _validate_base_python(env_name: str, base_pythons: list[str], ignore_base_py
         elements.update(env_name.split("-"))  # and also any factor
         for candidate in elements:
             spec_name = PythonSpec.from_string_spec(candidate)
-            if spec_name.implementation is not None and spec_name.implementation.lower() in ("pypy", "cpython"):
-                for base_python in base_pythons:
-                    spec_base = PythonSpec.from_string_spec(base_python)
-                    if any(
-                        getattr(spec_base, key) != getattr(spec_name, key)
-                        for key in ("implementation", "major", "minor", "micro", "architecture")
-                        if getattr(spec_base, key) is not None and getattr(spec_name, key) is not None
-                    ):
-                        msg = f"env name {env_name} conflicting with base python {base_python}"
-                        if ignore_base_python_conflict:
-                            return [env_name]  # ignore the base python settings
-                        raise Fail(msg)
+            if spec_name.implementation and spec_name.implementation.lower() not in INTERPRETER_SHORT_NAMES:
+                continue
+            for base_python in base_pythons:
+                spec_base = PythonSpec.from_string_spec(base_python)
+                if any(
+                    getattr(spec_base, key) != getattr(spec_name, key)
+                    for key in ("implementation", "major", "minor", "micro", "architecture")
+                    if getattr(spec_base, key) is not None and getattr(spec_name, key) is not None
+                ):
+                    msg = f"env name {env_name} conflicting with base python {base_python}"
+                    if ignore_base_python_conflict:
+                        return [env_name]  # ignore the base python settings
+                    raise Fail(msg)
         return base_pythons
 
     @abstractmethod

src/tox/tox_env/python/package.py

@@ -107,3 +107,10 @@ def child_pkg_envs(self, run_conf: EnvConfigSet) -> Iterator[PackageToxEnv]:
             env = self._wheel_build_envs.get(run_conf["wheel_build_env"])
             if env is not None and env.name != self.name:
                 yield env
+
+    def _teardown(self) -> None:
+        for env in self._wheel_build_envs.values():
+            if env is not self:
+                with env.display_context(self._has_display_suspended):
+                    env.teardown()
+        super()._teardown()

src/tox/tox_env/python/virtual_env/package/pyproject.py

@@ -3,6 +3,7 @@
 import logging
 import os
 import sys
+from collections import defaultdict
 from contextlib import contextmanager
 from pathlib import Path
 from threading import RLock
@@ -92,7 +93,7 @@ class Pep517VirtualEnvPackager(PythonPackageToxEnv, VirtualEnv):
     def __init__(self, create_args: ToxEnvCreateArgs) -> None:
         super().__init__(create_args)
         self._frontend_: Pep517VirtualEnvFrontend | None = None
-        self.builds: set[str] = set()
+        self.builds: defaultdict[str, list[EnvConfigSet]] = defaultdict(list)
         self._distribution_meta: PathDistribution | None = None
         self._package_dependencies: list[Requirement] | None = None
         self._package_name: str | None = None
@@ -136,7 +137,8 @@ def meta_folder(self) -> Path:
 
     def register_run_env(self, run_env: RunToxEnv) -> Generator[tuple[str, str], PackageToxEnv, None]:
         yield from super().register_run_env(run_env)
-        self.builds.add(run_env.conf["package"])
+        build_type = run_env.conf["package"]
+        self.builds[build_type].append(run_env.conf)
 
     def _setup_env(self) -> None:
         super()._setup_env()
@@ -169,13 +171,15 @@ def perform_packaging(self, for_env: EnvConfigSet) -> list[Package]:
         try:
             deps = self._load_deps(for_env)
         except BuildEditableNotSupported:
+            targets = [e for e in self.builds.pop("editable") if e["package"] == "editable"]
+            names = ", ".join(sorted({t.env_name for t in targets if t.env_name}))
             logging.error(
-                f"package config for {for_env.env_name} is editable, however the build backend {self._frontend.backend}"
+                f"package config for {names} is editable, however the build backend {self._frontend.backend}"
                 f" does not support PEP-660, falling back to editable-legacy - change your configuration to it",
             )
-            self.builds.remove("editable")
-            self.builds.add("editable-legacy")
-            for_env._defined["package"].value = "editable-legacy"  # type: ignore
+            for env in targets:
+                env._defined["package"].value = "editable-legacy"  # type: ignore
+                self.builds["editable-legacy"].append(env)
             deps = self._load_deps(for_env)
         of_type: str = for_env["package"]
         if of_type == "editable-legacy":