Skip to content

Commit

Permalink
Better handling of build backend without editable support and add --e…
Browse files Browse the repository at this point in the history
…xit-and-dump-after flag

Signed-off-by: Bernát Gábor <bgabor8@bloomberg.net>
  • Loading branch information
gaborbernat committed Dec 6, 2022
1 parent 4c77457 commit 3be37e8
Show file tree
Hide file tree
Showing 15 changed files with 75 additions and 32 deletions.
2 changes: 2 additions & 0 deletions 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`.
2 changes: 2 additions & 0 deletions 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`.
4 changes: 2 additions & 2 deletions pyproject.toml
Expand Up @@ -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"',
]
Expand Down
14 changes: 14 additions & 0 deletions src/tox/config/cli/parser.py
Expand Up @@ -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]]

Expand Down Expand Up @@ -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",
Expand Down
1 change: 1 addition & 0 deletions src/tox/execute/pep517_backend.py
Expand Up @@ -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):
Expand Down
3 changes: 3 additions & 0 deletions 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
Expand Down Expand Up @@ -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)
# build tox environment config objects
state = State(options, args)
return state
12 changes: 8 additions & 4 deletions src/tox/tox_env/package.py
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down
31 changes: 16 additions & 15 deletions src/tox/tox_env/python/api.py
Expand Up @@ -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}")
Expand All @@ -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
Expand Down
7 changes: 7 additions & 0 deletions src/tox/tox_env/python/package.py
Expand Up @@ -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()
16 changes: 10 additions & 6 deletions src/tox/tox_env/python/virtual_env/package/pyproject.py
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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":
Expand Down
2 changes: 2 additions & 0 deletions tests/config/cli/test_cli_env_var.py
Expand Up @@ -44,6 +44,7 @@ def test_verbose_no_test() -> None:
"list_envs": False,
"devenv_path": None,
"env": CliEnv(),
"exit_and_dump_after": 0,
"skip_missing_interpreters": "config",
"skip_pkg_install": False,
"recreate": False,
Expand Down Expand Up @@ -118,6 +119,7 @@ def test_env_var_exhaustive_parallel_values(
"config_file": None,
"factors": [],
"labels": [],
"exit_and_dump_after": 0,
}
assert options.parsed.verbosity == 4
assert options.cmd_handlers == core_handlers
Expand Down
2 changes: 2 additions & 0 deletions tests/config/cli/test_cli_ini.py
Expand Up @@ -98,6 +98,7 @@ def default_options(tmp_path: Path) -> dict[str, Any]:
"config_file": (tmp_path / "tox.ini").absolute(),
"factors": [],
"labels": [],
"exit_and_dump_after": 0,
}


Expand Down Expand Up @@ -132,6 +133,7 @@ def test_ini_exhaustive_parallel_values(exhaustive_ini: Path, core_handlers: dic
"config_file": exhaustive_ini,
"factors": [],
"labels": [],
"exit_and_dump_after": 0,
}
assert options.parsed.verbosity == 4
assert options.cmd_handlers == core_handlers
Expand Down
2 changes: 0 additions & 2 deletions tests/tox_env/python/test_python_api.py
Expand Up @@ -77,7 +77,6 @@ def test_diff_msg_no_diff() -> None:
[
("magic", ["pypy"]),
("magic", ["py39"]),
(".pkg", ["py"]),
],
ids=lambda a: "|".join(a) if isinstance(a, list) else str(a),
)
Expand All @@ -90,7 +89,6 @@ def test_base_python_env_no_conflict(env: str, base_python: list[str], ignore_co
@pytest.mark.parametrize(
("env", "base_python", "conflict"),
[
("py", ["pypy"], ["pypy"]),
("cpython", ["pypy"], ["pypy"]),
("pypy", ["cpython"], ["cpython"]),
("pypy2", ["pypy3"], ["pypy3"]),
Expand Down
Expand Up @@ -175,10 +175,10 @@ def test_pyproject_deps_static_with_dynamic(
def test_pyproject_no_build_editable_fallback(tox_project: ToxProjectCreator, demo_pkg_inline: Path) -> None:
proj = tox_project({"tox.ini": ""}, base=demo_pkg_inline)
execute_calls = proj.patch_execute(lambda r: 0 if "install" in r.run_id else None)
result = proj.run("r", "--notest", "--develop")
result = proj.run("r", "-e", "a,b", "--notest", "--develop")
result.assert_success()
warning = (
".pkg: package config for py is editable, however the build backend build does not support PEP-660, "
".pkg: package config for a, b is editable, however the build backend build does not support PEP-660, "
"falling back to editable-legacy - change your configuration to it"
)
assert warning in result.out.splitlines()
Expand All @@ -187,7 +187,9 @@ def test_pyproject_no_build_editable_fallback(tox_project: ToxProjectCreator, de
(".pkg", "_optional_hooks"),
(".pkg", "build_wheel"),
(".pkg", "get_requires_for_build_sdist"),
("py", "install_package"),
("a", "install_package"),
(".pkg", "get_requires_for_build_sdist"),
("b", "install_package"),
(".pkg", "_exit"),
]
found_calls = [(i[0][0].conf.name, i[0][3].run_id) for i in execute_calls.call_args_list]
Expand Down
1 change: 1 addition & 0 deletions whitelist.txt
Expand Up @@ -63,6 +63,7 @@ exe
executables
extlinks
extractall
faulthandler
favicon
filelock
fixup
Expand Down

0 comments on commit 3be37e8

Please sign in to comment.