diff options
Diffstat (limited to 'app')
| -rw-r--r-- | app/backends/common.py | 68 | ||||
| -rwxr-xr-x | app/backends/faster.py | 39 | ||||
| -rw-r--r-- | app/backends/probe.py | 15 | ||||
| -rw-r--r-- | app/backends/qwen.py | 10 | ||||
| -rw-r--r-- | app/backends/servers.py | 19 | ||||
| -rw-r--r-- | app/tests/test_backends.py | 25 | ||||
| -rw-r--r-- | app/tests/test_backends_faster.py | 39 | ||||
| -rw-r--r-- | app/tests/test_backends_servers.py | 17 |
8 files changed, 182 insertions, 50 deletions
diff --git a/app/backends/common.py b/app/backends/common.py index 10b4ccf..9b6232a 100644 --- a/app/backends/common.py +++ b/app/backends/common.py @@ -102,12 +102,19 @@ def resolve_wav_dir_arg(value: str) -> Path: def find_wav_files(input_dir: Path) -> List[Path]: - """Return the .wav files in INPUT_DIR, sorted alphabetically by name.""" - return sorted( - (path for path in input_dir.iterdir() - if path.is_file() and path.suffix.lower() == ".wav"), - key=lambda path: path.name.lower(), - ) + """Return the .wav files in INPUT_DIR, sorted alphabetically by name. + + A missing or unreadable directory yields [] so callers can treat it + like an empty directory (matching ``count_wavs``). + """ + try: + return sorted( + (path for path in input_dir.iterdir() + if path.is_file() and path.suffix.lower() == ".wav"), + key=lambda path: path.name.lower(), + ) + except OSError: + return [] def count_wavs(directory: Path) -> int: @@ -248,29 +255,38 @@ def server_running(url: str, timeout: float = 0.3) -> bool: return False -def update_config_value(key: str, value: str, +def update_config_value(key: str, value, config_path: Optional[Path] = None) -> bool: - """Rewrite a ``KEY = "value"`` line in app/converter/config.py. - - Only the quoted literal is replaced; surrounding lines and the trailing - comment are preserved. Returns True when the file was changed. Used by - the qwen and faster wizards to keep their API URL / voice / speaker - settings in sync with the converter. + """Set ``KEY`` to VALUE in app/converter/config.py and in memory. + + Only the value of the named assignment changes: indentation and any + trailing comment are preserved. Strings render double-quoted; other + literals (ints, booleans) render bare. After a successful write (or + when the file already holds VALUE) the new value is mirrored onto the + imported ``converter.config`` module, so a wizard's change takes + effect immediately instead of only after the next process start. + Returns True when the file now holds VALUE, False when it could not + be read or written (or KEY has no line in it). """ path = Path(config_path) if config_path is not None else CONFIG_PATH + rendered = f'"{value}"' if isinstance(value, str) else str(value) try: text = path.read_text(encoding="utf-8") + match = re.search( + rf'(?m)^(\s*{re.escape(key)}\s*=\s*)("[^"]*"|\S+)(\s*(?:#.*)?)$', + text) + if match is None: + return False + if match.group(2) != rendered: + text = text[:match.start(2)] + rendered + text[match.end(2):] + path.write_text(text, encoding="utf-8") except OSError: return False - match = re.search(r'(?m)^(\s*' + re.escape(key) + r'\s*=\s*")([^"]*)(")', - text) - if not match or match.group(2) == value: - return False - text = text[:match.start(2)] + value + text[match.end(2):] - try: - path.write_text(text, encoding="utf-8") - except OSError: - return False + # Local import: this module must stay importable before the venv + # exists (backends.envs bootstraps from it), and converter.config is + # stdlib-only constants, safe to load whenever a wizard runs. + from converter import config as _config + setattr(_config, key, value) return True @@ -458,16 +474,18 @@ def git_clone(url: str, target: Path, *, emit=None, cancel=None) -> int: emit=emit, cancel=cancel) -def pip_install(packages: List[str]) -> int: +def pip_install(packages: List[str], *, emit=None, cancel=None) -> int: """pip install PACKAGES into the managed venv (``envs/tts``). Returns exit code. Delegates to ``backends.envs.pip_install`` so backend TTS packages are installed alongside the app requirements in the tool-managed environment rather than into whatever interpreter happens to be running the wizard. - The import is local to avoid a circular import (envs imports this module). + With EMIT given (the in-TUI task view) pip runs with its output streamed + into EMIT; CANCEL aborts it. The import is local to avoid a circular + import (envs imports this module). """ from backends import envs - return envs.pip_install(packages) + return envs.pip_install(packages, emit=emit, cancel=cancel) def pip_uninstall(packages: List[str], *, emit=None) -> int: diff --git a/app/backends/faster.py b/app/backends/faster.py index 7ac4dce..50cf61f 100755 --- a/app/backends/faster.py +++ b/app/backends/faster.py @@ -289,12 +289,17 @@ def _wizard(stdscr, args: argparse.Namespace) -> Optional[dict]: return _after_whisper() def _after_whisper(): - # Default into the cloned checkout; fall back to the wav directory - # when the checkout is not present (so a flag-only run still works). + # Default into the cloned checkout — also when the clone is still + # pending in this run's steps (do_clone): detect() and the server + # launch only read voices.json from there, so a fresh install must + # not leave the file in the wav directory. The wav-directory + # fallback keeps flag-only runs working without any checkout. s["output_path"] = args.output if s["output_path"] is None: - s["output_path"] = (_checkout() / "voices.json") if _is_cloned() \ - else (s["wav_dir"] / "voices.json") + if _is_cloned() or s.get("do_clone"): + s["output_path"] = _checkout() / "voices.json" + else: + s["output_path"] = s["wav_dir"] / "voices.json" wav_files = find_wav_files(s["wav_dir"]) if wav_files and s["existing_voices"] and not args.force: return screen_transcription @@ -488,15 +493,23 @@ def _collect_from_flags(args: argparse.Namespace, language = normalize_language(args.language or config.LANGUAGE) except ValueError as exc: parser.error(str(exc)) - output_path = args.output if args.output is not None \ - else ((_checkout() / "voices.json") if _is_cloned() - else (wav_dir / "voices.json")) + do_install = (not _is_installed()) and not args.skip_install + do_clone = (not _is_cloned()) and not args.skip_clone + # The checkout's voices.json is the canonical location (detect() and + # the server launch read it there) — including when this run clones + # the checkout itself. Without a checkout, fall back to the wav dir. + output_path = args.output + if output_path is None: + if _is_cloned() or do_clone: + output_path = _checkout() / "voices.json" + else: + output_path = wav_dir / "voices.json" if output_path.exists() and not args.force: print("[INFO] Aborted; existing voices.json kept") return None return { - "do_install": (not _is_installed()) and not args.skip_install, - "do_clone": (not _is_cloned()) and not args.skip_clone, + "do_install": do_install, + "do_clone": do_clone, "wav_dir": wav_dir, "language": language, "whisper_model": args.whisper_model or "base", @@ -606,7 +619,11 @@ def uninstall(*, emit=None, cancel=None) -> int: killed mid-run. Returns the exit code (130 when cancelled before a remaining phase). """ - servers.stop("faster") + # Only stop when a pid file exists: without one this tool never + # started the server, so the "not started by this tool" notice would + # be uninstall-time noise. + if servers.pid_for("faster") is not None: + servers.stop("faster") if common.cancel_requested(cancel): return 130 rc = common.pip_uninstall(["faster-qwen3-tts"], emit=emit) @@ -622,7 +639,7 @@ def uninstall(*, emit=None, cancel=None) -> int: print(f"[INFO] Removing checkout {checkout}...") shutil.rmtree(checkout, ignore_errors=True) print("[OK] checkout removed.") - return 0 + return rc def main() -> int: diff --git a/app/backends/probe.py b/app/backends/probe.py index efa2963..ada143a 100644 --- a/app/backends/probe.py +++ b/app/backends/probe.py @@ -108,11 +108,20 @@ def _identify_gradio(base: str, timeout: float) -> Optional[str]: return None +def _canonical_host(host: str) -> str: + """Fold the loopback aliases so "localhost" and "127.0.0.1" compare equal.""" + return "127.0.0.1" if host in ("localhost", "::1", "[::1]") else host + + def same_endpoint(url_a: str, url_b: str) -> bool: """True when URL_A and URL_B address the same host and port. Scheme and path are ignored (127.0.0.1:8080 and http://127.0.0.1:8080/ - are the same server). Returns False when either URL is empty/unparsable. + are the same server), and the loopback names are folded together + ("localhost:8080" equals "127.0.0.1:8080") — the config's remote-URL + defaults point at the managed servers, so a user writing either form + must not get their own server double-counted as "[remote]". + Returns False when either URL is empty/unparsable. """ if not url_a or not url_b: return False @@ -121,8 +130,8 @@ def same_endpoint(url_a: str, url_b: str) -> bool: b = urllib.parse.urlsplit(url_b) except ValueError: return False - host_a = a.hostname or "127.0.0.1" - host_b = b.hostname or "127.0.0.1" + host_a = _canonical_host(a.hostname or "127.0.0.1") + host_b = _canonical_host(b.hostname or "127.0.0.1") port_a = a.port or (443 if (a.scheme or "http") == "https" else 80) port_b = b.port or (443 if (b.scheme or "http") == "https" else 80) return host_a == host_b and port_a == port_b diff --git a/app/backends/qwen.py b/app/backends/qwen.py index cc84f8e..33e7114 100644 --- a/app/backends/qwen.py +++ b/app/backends/qwen.py @@ -354,8 +354,12 @@ def uninstall(*, emit=None, cancel=None) -> int: so pip is never killed mid-run. Returns the exit code (130 when cancelled before pip ran). """ - servers.stop("qwen-custom") - servers.stop("qwen-clone") + for name in ("qwen-custom", "qwen-clone"): + # Only stop when a pid file exists: without one this tool never + # started the server, so the "not started by this tool" notice + # would be uninstall-time noise. + if servers.pid_for(name) is not None: + servers.stop(name) if common.cancel_requested(cancel): return 130 rc = common.pip_uninstall([QWEN_PIP_PKG], emit=emit) @@ -364,7 +368,7 @@ def uninstall(*, emit=None, cancel=None) -> int: f"{QWEN_PIP_PKG} from the managed venv manually") else: print(f"[OK] {QWEN_PIP_PKG} removed.") - return 0 + return rc def main() -> int: diff --git a/app/backends/servers.py b/app/backends/servers.py index 965d7df..986d82a 100644 --- a/app/backends/servers.py +++ b/app/backends/servers.py @@ -27,6 +27,7 @@ import signal import subprocess import sys import time +from datetime import datetime from pathlib import Path from typing import Callable, List, Optional @@ -253,6 +254,15 @@ def start(spec, progress: ProgressCallback = None, return True LOG_DIR.mkdir(parents=True, exist_ok=True) + # Refuse to double-start: a live pid file means a previous start is + # still booting (or its process is wedged). Spawning a second server + # on the same port would orphan the first with no pid record left. + if alive(spec.name): + report({"kind": "error", + "message": f"a {spec.name} server (pid " + f"{pid_for(spec.name)}) is already starting or " + "running; stop it first"}) + return False pid_file = _pid_path(spec.name) if pid_file.exists(): try: @@ -261,7 +271,10 @@ def start(spec, progress: ProgressCallback = None, pass cwd = getattr(spec, "cwd", None) - log_handle = _log_path(spec.name).open("w", encoding="utf-8") + # Append so an earlier boot's output survives (crash-loop debugging); + # the child inherits the handle and the parent's copy is closed right + # after the spawn, so nothing leaks here. + log_handle = _log_path(spec.name).open("a", encoding="utf-8") popen_kwargs = {"stdout": log_handle, "stderr": subprocess.STDOUT} if cwd is not None: popen_kwargs["cwd"] = str(cwd) @@ -277,6 +290,10 @@ def start(spec, progress: ProgressCallback = None, "message": f"could not start server: {exc}"}) log_handle.close() return False + log_handle.write(f"\n=== boot {datetime.now():%Y-%m-%d %H:%M:%S} " + f"(pid {proc.pid}) ===\n") + log_handle.flush() + log_handle.close() pid_file.write_text(str(proc.pid), encoding="utf-8") report({"kind": "starting", "name": spec.name, diff --git a/app/tests/test_backends.py b/app/tests/test_backends.py index 8ea6a3f..1f146e5 100644 --- a/app/tests/test_backends.py +++ b/app/tests/test_backends.py @@ -332,7 +332,9 @@ class QwenUninstallTests(unittest.TestCase): def test_stops_servers_and_pips(self): from backends import qwen - with patch.object(qwen.servers, "stop") as mk_stop, \ + # Pid files exist for both managed servers, so stop runs. + with patch.object(qwen.servers, "pid_for", return_value=1234), \ + patch.object(qwen.servers, "stop") as mk_stop, \ patch.object(qwen.common, "pip_uninstall", return_value=0) as mk_pip: rc = qwen.uninstall(emit="EMIT") @@ -342,23 +344,36 @@ class QwenUninstallTests(unittest.TestCase): # The task view's emit is forwarded so pip never touches the terminal. mk_pip.assert_called_once_with([qwen.QWEN_PIP_PKG], emit="EMIT") + def test_skips_stop_when_no_server_was_started(self): + # No pid files: stop() is not called (no "not started by this + # tool" noise during an uninstall). + from backends import qwen + with patch.object(qwen.servers, "pid_for", return_value=None), \ + patch.object(qwen.servers, "stop") as mk_stop, \ + patch.object(qwen.common, "pip_uninstall", return_value=0): + rc = qwen.uninstall() + self.assertEqual(rc, 0) + mk_stop.assert_not_called() + def test_cancel_before_pip_skips_uninstall(self): import threading from backends import qwen cancel = threading.Event() cancel.set() - with patch.object(qwen.servers, "stop") as mk_stop, \ + with patch.object(qwen.servers, "pid_for", return_value=1234), \ + patch.object(qwen.servers, "stop") as mk_stop, \ patch.object(qwen.common, "pip_uninstall") as mk_pip: rc = qwen.uninstall(cancel=cancel) self.assertEqual(rc, 130) self.assertEqual(mk_stop.call_count, 2) mk_pip.assert_not_called() - def test_pip_failure_warns_but_still_succeeds(self): + def test_pip_failure_propagates_the_exit_code(self): from backends import qwen - with patch.object(qwen.servers, "stop"), \ + with patch.object(qwen.servers, "pid_for", return_value=1234), \ + patch.object(qwen.servers, "stop"), \ patch.object(qwen.common, "pip_uninstall", return_value=1): rc = qwen.uninstall() - self.assertEqual(rc, 0) + self.assertEqual(rc, 1) diff --git a/app/tests/test_backends_faster.py b/app/tests/test_backends_faster.py index 7e27b47..55d617c 100644 --- a/app/tests/test_backends_faster.py +++ b/app/tests/test_backends_faster.py @@ -203,6 +203,31 @@ class MainTests(unittest.TestCase): self.assertTrue(custom.exists()) self.assertFalse(self.output.exists()) + def test_fresh_install_defaults_voices_json_into_the_checkout(self): + # Regression: on a fresh machine the clone runs as part of this + # same setup run, so voices.json must be written where detect() + # and the server launch read it (the checkout) — not the wav dir. + checkout = Path(self._tmp.name) / "faster-qwen3-tts" + + def fake_clone(url, target, emit=None, cancel=None): + checkout.mkdir(parents=True, exist_ok=True) # what git would do + return 0 + + with patch.object(make_voices, "_is_installed", return_value=False), \ + patch.object(make_voices, "_checkout", + return_value=checkout), \ + patch.object(make_voices.common, "pip_install", + return_value=0), \ + patch.object(make_voices.common, "git_clone", + side_effect=fake_clone) as mk_clone: + exit_code = self._run([str(self.folder)]) + self.assertEqual(exit_code, 0) + mk_clone.assert_called_once() + voices = json.loads( + (checkout / "voices.json").read_text(encoding="utf-8")) + self.assertEqual(list(voices), ["alpha", "narrator"]) + self.assertFalse((self.folder / "voices.json").exists()) + def test_invalid_language_errors_before_work(self): with patch.object(make_voices, "transcribe_reference_audio") as mock_transcribe: with self.assertRaises(SystemExit) as ctx: @@ -288,6 +313,8 @@ class UninstallTests(unittest.TestCase): checkout.mkdir() with patch.object(make_voices, "_checkout", return_value=checkout), \ + patch.object(make_voices.servers, "pid_for", + return_value=1234), \ patch.object(make_voices.servers, "stop") as mk_stop, \ patch.object(make_voices.common, "pip_uninstall", return_value=0) as mk_pip: @@ -301,17 +328,23 @@ class UninstallTests(unittest.TestCase): def test_no_checkout_still_uninstalls_the_package(self): with patch.object(make_voices, "_checkout", return_value=Path("/no/such/dir")), \ - patch.object(make_voices.servers, "stop"), \ + patch.object(make_voices.servers, "pid_for", + return_value=None), \ + patch.object(make_voices.servers, "stop") as mk_stop, \ patch.object(make_voices.common, "pip_uninstall", return_value=0) as mk_pip: rc = make_voices.uninstall() self.assertEqual(rc, 0) + # No pid file: no stop attempt (and no noise about it). + mk_stop.assert_not_called() mk_pip.assert_called_once_with(["faster-qwen3-tts"], emit=None) def test_cancel_before_pip_skips_everything_after_stopping(self): cancel = threading.Event() cancel.set() - with patch.object(make_voices.servers, "stop") as mk_stop, \ + with patch.object(make_voices.servers, "pid_for", + return_value=1234), \ + patch.object(make_voices.servers, "stop") as mk_stop, \ patch.object(make_voices.common, "pip_uninstall") as mk_pip: rc = make_voices.uninstall(cancel=cancel) self.assertEqual(rc, 130) @@ -328,6 +361,8 @@ class UninstallTests(unittest.TestCase): cancel.set() with patch.object(make_voices, "_checkout", return_value=checkout), \ + patch.object(make_voices.servers, "pid_for", + return_value=1234), \ patch.object(make_voices.servers, "stop"), \ patch.object(make_voices.common, "pip_uninstall", return_value=0): diff --git a/app/tests/test_backends_servers.py b/app/tests/test_backends_servers.py index 8eaf53f..ab05eed 100644 --- a/app/tests/test_backends_servers.py +++ b/app/tests/test_backends_servers.py @@ -35,6 +35,23 @@ class StartTests(unittest.TestCase): self.assertTrue(servers.start(self.spec)) mk.assert_not_called() + def test_refuses_to_double_start_while_previous_boot_is_alive(self): + """A live pid file blocks a second spawn of the same server. + + A previous ``start`` whose server is still booting must not be + orphaned by a duplicate process on the same port. + """ + pid_file = self.dir / "test-server.pid" + pid_file.write_text("4242", encoding="utf-8") + with patch.object(servers, "LOG_DIR", self.dir), \ + patch.object(servers, "_pid_alive", return_value=True), \ + patch("subprocess.Popen") as mk, \ + patch("backends.common.server_running", return_value=False): + ok = servers.start(self.spec) + self.assertFalse(ok) + mk.assert_not_called() + self.assertTrue(pid_file.exists()) + def test_happy_path_spawns_and_polls_until_ready(self): proc = MagicMock() proc.pid = 4242 |