fix: muxterm binary discovery — find local build before PATH search
_find_muxterm_binary() now resolves in this order:
1. Explicit binary_path argument (unchanged)
2. MUXTERM_BINARY env var (new)
3. <repo_root>/muxterm/muxterm — the go build output for dev installs (new)
4. shutil.which('muxterm') — PATH fallback (unchanged)
Before this fix the service logged 'muxterm binary not found; terminal
feature disabled' on every restart because the binary is not on PATH
and no explicit path was passed from main.py.
Also fix stop_muxterm() to catch ProcessLookupError from terminate()
when the process has already exited (was only caught in the wait_for
block, not the terminate() call itself), which caused a noisy
'Application shutdown failed' traceback in the service journal.
This commit is contained in:
+33
-4
@@ -16,6 +16,8 @@ Internal:
|
|||||||
|
|
||||||
import asyncio
|
import asyncio
|
||||||
import logging
|
import logging
|
||||||
|
import os
|
||||||
|
import pathlib
|
||||||
import shutil
|
import shutil
|
||||||
|
|
||||||
logger = logging.getLogger(__name__)
|
logger = logging.getLogger(__name__)
|
||||||
@@ -35,16 +37,37 @@ _restart_task: asyncio.Task | None = None # type: ignore[type-arg]
|
|||||||
def _find_muxterm_binary(binary_path: str | None = None) -> str:
|
def _find_muxterm_binary(binary_path: str | None = None) -> str:
|
||||||
"""Locate the muxterm binary.
|
"""Locate the muxterm binary.
|
||||||
|
|
||||||
Checks explicit *binary_path* first, then falls back to
|
Resolution order:
|
||||||
``shutil.which('muxterm')``. Raises :exc:`FileNotFoundError` if
|
1. Explicit *binary_path* argument.
|
||||||
the binary cannot be found.
|
2. ``MUXTERM_BINARY`` environment variable.
|
||||||
|
3. ``<repo_root>/muxterm/muxterm`` — the build output for dev/local installs
|
||||||
|
(package lives at ``<repo_root>/muxplex/``, binary at
|
||||||
|
``<repo_root>/muxterm/muxterm``).
|
||||||
|
4. ``shutil.which('muxterm')`` — installed on PATH.
|
||||||
|
|
||||||
|
Raises :exc:`FileNotFoundError` if none of the above resolves.
|
||||||
"""
|
"""
|
||||||
if binary_path is not None:
|
if binary_path is not None:
|
||||||
return binary_path
|
return binary_path
|
||||||
|
|
||||||
|
env_path = os.environ.get("MUXTERM_BINARY")
|
||||||
|
if env_path:
|
||||||
|
candidate = pathlib.Path(env_path)
|
||||||
|
if candidate.is_file() and os.access(candidate, os.X_OK):
|
||||||
|
return str(candidate)
|
||||||
|
|
||||||
|
# Dev/local-install: binary is built next to the Python package directory.
|
||||||
|
# muxterm.py lives in <repo_root>/muxplex/
|
||||||
|
# muxterm binary is at <repo_root>/muxterm/muxterm
|
||||||
|
local_candidate = pathlib.Path(__file__).parent.parent / "muxterm" / "muxterm"
|
||||||
|
if local_candidate.is_file() and os.access(local_candidate, os.X_OK):
|
||||||
|
return str(local_candidate)
|
||||||
|
|
||||||
found = shutil.which("muxterm")
|
found = shutil.which("muxterm")
|
||||||
if found is None:
|
if found is None:
|
||||||
raise FileNotFoundError(
|
raise FileNotFoundError(
|
||||||
"muxterm binary not found on PATH; pass binary_path explicitly"
|
"muxterm binary not found; tried MUXTERM_BINARY env var, "
|
||||||
|
f"{local_candidate}, and PATH. Build it with `cd muxterm && go build .`"
|
||||||
)
|
)
|
||||||
return found
|
return found
|
||||||
|
|
||||||
@@ -157,9 +180,15 @@ async def stop_muxterm() -> None:
|
|||||||
_restart_task = None
|
_restart_task = None
|
||||||
|
|
||||||
if _muxterm_process is not None:
|
if _muxterm_process is not None:
|
||||||
|
try:
|
||||||
_muxterm_process.terminate()
|
_muxterm_process.terminate()
|
||||||
|
except ProcessLookupError:
|
||||||
|
pass # already exited
|
||||||
try:
|
try:
|
||||||
await asyncio.wait_for(_muxterm_process.wait(), timeout=5.0)
|
await asyncio.wait_for(_muxterm_process.wait(), timeout=5.0)
|
||||||
except (asyncio.TimeoutError, ProcessLookupError):
|
except (asyncio.TimeoutError, ProcessLookupError):
|
||||||
|
try:
|
||||||
_muxterm_process.kill()
|
_muxterm_process.kill()
|
||||||
|
except ProcessLookupError:
|
||||||
|
pass
|
||||||
_muxterm_process = None
|
_muxterm_process = None
|
||||||
|
|||||||
@@ -46,16 +46,24 @@ def test_find_muxterm_binary_explicit_path(tmp_path):
|
|||||||
|
|
||||||
|
|
||||||
def test_find_muxterm_binary_uses_which():
|
def test_find_muxterm_binary_uses_which():
|
||||||
"""Falls back to shutil.which('muxterm') when no explicit path."""
|
"""Falls back to shutil.which('muxterm') when no env var and no local binary."""
|
||||||
with patch("shutil.which", return_value="/usr/bin/muxterm") as mock_which:
|
with (
|
||||||
|
patch.dict("os.environ", {"MUXTERM_BINARY": ""}), # suppress env override
|
||||||
|
patch("muxplex.muxterm.pathlib.Path.is_file", return_value=False), # no local build
|
||||||
|
patch("shutil.which", return_value="/usr/bin/muxterm") as mock_which,
|
||||||
|
):
|
||||||
result = _find_muxterm_binary()
|
result = _find_muxterm_binary()
|
||||||
mock_which.assert_called_once_with("muxterm")
|
mock_which.assert_called_once_with("muxterm")
|
||||||
assert result == "/usr/bin/muxterm"
|
assert result == "/usr/bin/muxterm"
|
||||||
|
|
||||||
|
|
||||||
def test_find_muxterm_binary_raises_when_not_found():
|
def test_find_muxterm_binary_raises_when_not_found():
|
||||||
"""Raises FileNotFoundError if muxterm is not found."""
|
"""Raises FileNotFoundError if muxterm is not found anywhere."""
|
||||||
with patch("shutil.which", return_value=None):
|
with (
|
||||||
|
patch.dict("os.environ", {"MUXTERM_BINARY": ""}), # suppress env override
|
||||||
|
patch("muxplex.muxterm.pathlib.Path.is_file", return_value=False), # no local build
|
||||||
|
patch("shutil.which", return_value=None),
|
||||||
|
):
|
||||||
with pytest.raises(FileNotFoundError):
|
with pytest.raises(FileNotFoundError):
|
||||||
_find_muxterm_binary()
|
_find_muxterm_binary()
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user