fix: doctor probes backend readiness; install.sh resolves user-scripts dir
tracker-policy-pr / check-pr (pull_request) Successful in 10s
test / integration-docker (pull_request) Successful in 17s
lint / lint (push) Successful in 57s
test / unit (pull_request) Successful in 2m10s
test / integration-firecracker (pull_request) Successful in 3m50s
test / coverage (pull_request) Successful in 16s
test / publish-infra (pull_request) Has been skipped
tracker-policy-pr / check-pr (pull_request) Successful in 10s
test / integration-docker (pull_request) Successful in 17s
lint / lint (push) Successful in 57s
test / unit (pull_request) Successful in 2m10s
test / integration-firecracker (pull_request) Successful in 3m50s
test / coverage (pull_request) Successful in 16s
test / publish-infra (pull_request) Has been skipped
Addresses the third review round on PR #481. - `bot-bottle doctor` now checks `is_backend_ready()` (a full backend status() probe: daemon reachable, network pool present, KVM usable) instead of the cheap PATH-only `is_backend_available()`. A host with a stopped Docker daemon or half-configured Firecracker no longer reports `ok: backend` / exit 0 when `start` can't actually work; each not-ready backend prints its own diagnostics, and doctor passes only if at least one backend is ready. - `install.sh` resolves the pip `--user` scripts directory from the interpreter (`sysconfig.get_path("scripts", get_preferred_scheme("user"))`) instead of hardcoding `~/.local/bin`, which is wrong on a python.org macOS interpreter (`~/Library/Python/<X.Y>/bin`). The PATH guidance now prints the actual directory. Tests: doctor tests mock `is_backend_ready` (the readiness contract) and cover the not-ready → fail path; a new install-script test drives the macOS `osx_framework_user` scheme and asserts it resolves a non-~/.local/bin directory. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -2,7 +2,8 @@
|
||||
bot-bottle and report what's ready.
|
||||
|
||||
Fails (non-zero exit) only on the two hard requirements: a new-enough
|
||||
Python and at least one usable backend. The config directory is a soft
|
||||
Python and at least one backend that is *ready* (passes its full status
|
||||
checks, so `start` can actually work). The config directory is a soft
|
||||
check — `install.sh` creates it, but a missing one only warrants a note,
|
||||
not a failure, since `start` provisions what it needs on first run.
|
||||
"""
|
||||
@@ -13,7 +14,7 @@ import argparse
|
||||
import sys
|
||||
from pathlib import Path
|
||||
|
||||
from ...backend import is_backend_available, known_backend_names
|
||||
from ...backend import is_backend_ready, known_backend_names
|
||||
from ..constants import PROG
|
||||
|
||||
MIN_PYTHON = (3, 11)
|
||||
@@ -43,19 +44,25 @@ def _check_python() -> bool:
|
||||
|
||||
|
||||
def _check_backends() -> bool:
|
||||
"""At least one backend must be available on this host. Report each
|
||||
known backend so the operator sees why a missing one is missing."""
|
||||
available = []
|
||||
"""At least one backend must be *ready* to run a bottle — i.e. pass its
|
||||
full status() checks (daemon reachable, network pool present, KVM usable),
|
||||
not merely have a binary on PATH. A binary-only check would report `ok`
|
||||
on a host with a stopped Docker daemon or a half-configured Firecracker,
|
||||
where `start` still can't work. Each not-ready backend prints its own
|
||||
diagnostics (quiet=False) so the operator sees exactly what's missing."""
|
||||
ready = []
|
||||
for name in known_backend_names():
|
||||
if is_backend_available(name):
|
||||
available.append(name)
|
||||
if available:
|
||||
_ok("backend", f"available: {', '.join(available)}")
|
||||
if is_backend_ready(name, quiet=False):
|
||||
_ok("backend", f"{name}: ready")
|
||||
ready.append(name)
|
||||
else:
|
||||
_warn("backend", f"{name}: not ready (see diagnostics above)")
|
||||
if ready:
|
||||
return True
|
||||
_fail(
|
||||
"backend",
|
||||
"no backend available; install Apple Container (macOS), "
|
||||
"Firecracker (Linux), or Docker",
|
||||
"no backend is ready to run a bottle; start Docker, or finish "
|
||||
"Apple Container (macOS) / Firecracker (Linux) setup",
|
||||
)
|
||||
return False
|
||||
|
||||
|
||||
@@ -33,7 +33,7 @@ user whether their host is actually ready to run a bottle.
|
||||
`bot-bottle doctor`. It never installs Docker or a VM backend silently
|
||||
and never uses `sudo`.
|
||||
- `install.sh` is idempotent — safe to re-run.
|
||||
- `bot-bottle doctor` reports Python version, backend availability, and
|
||||
- `bot-bottle doctor` reports Python version, backend *readiness*, and
|
||||
config-dir presence, exiting non-zero when a hard prerequisite is unmet.
|
||||
- The package keeps **zero runtime pip dependencies** (stdlib-only,
|
||||
matching the existing constraint in `AGENTS.md`).
|
||||
@@ -129,8 +129,11 @@ A POSIX `sh` bootstrapper that:
|
||||
4. Installs via `pipx` if available, else `python3 -m pip install --user`.
|
||||
The spec defaults to the git URL and is overridable via
|
||||
`BOT_BOTTLE_INSTALL_SPEC` (used by tests / local installs).
|
||||
4. Locates the `bot-bottle` entry point (PATH or `~/.local/bin`).
|
||||
5. Runs `bot-bottle doctor` and reports the result.
|
||||
5. Locates the `bot-bottle` entry point: PATH first, else the
|
||||
interpreter's own user-scheme scripts dir resolved via `sysconfig`
|
||||
(`~/.local/bin` on Linux, `~/Library/Python/<X.Y>/bin` on a python.org
|
||||
macOS interpreter — not hardcoded).
|
||||
6. Runs `bot-bottle doctor` and reports the result.
|
||||
|
||||
It is idempotent and never calls `sudo`.
|
||||
|
||||
@@ -140,10 +143,12 @@ A new store-free subcommand (no DB migration required) that checks and
|
||||
reports:
|
||||
|
||||
- **python** — interpreter version (hard requirement: ≥ 3.11).
|
||||
- **backend** — at least one backend available on this host
|
||||
(macos-container / firecracker / docker), reusing
|
||||
`is_backend_available()` rather than hardcoding Docker, since the
|
||||
default backend is now a VM backend. Hard requirement.
|
||||
- **backend** — at least one backend *ready* on this host
|
||||
(macos-container / firecracker / docker), via `is_backend_ready()` — a
|
||||
full backend `status()` probe (daemon reachable, network pool present,
|
||||
KVM usable), not a PATH-only check: a stopped daemon or half-configured
|
||||
backend must not report `ok` when `start` can't work. Each not-ready
|
||||
backend prints its own diagnostics. Hard requirement.
|
||||
- **config** — whether `~/.bot-bottle/` exists (advisory only; `start`
|
||||
provisions on first run).
|
||||
|
||||
@@ -152,7 +157,8 @@ Exits 0 when both hard requirements pass, non-zero otherwise.
|
||||
## Testing strategy
|
||||
|
||||
- Unit test `bot-bottle doctor` success/failure paths with backend
|
||||
availability and Python version mocked.
|
||||
readiness (`is_backend_ready`) and Python version mocked, including the
|
||||
available-but-not-ready → fail case.
|
||||
- Unit test that `pyproject.toml` parses, declares the entry point and an
|
||||
empty `dependencies` list, and that every `package-data` glob resolves
|
||||
to a file that exists on disk (guards against drift).
|
||||
|
||||
+13
-4
@@ -94,13 +94,22 @@ fi
|
||||
|
||||
# --- locate the entry point --------------------------------------------------
|
||||
|
||||
# The pip --user scripts directory is platform-specific: ~/.local/bin on Linux,
|
||||
# but ~/Library/Python/<X.Y>/bin on a python.org macOS interpreter. Ask the
|
||||
# interpreter for its own user-scheme scripts dir instead of hardcoding.
|
||||
USER_SCRIPTS="$(python3 - <<'PY'
|
||||
import sysconfig
|
||||
print(sysconfig.get_path("scripts", sysconfig.get_preferred_scheme("user")))
|
||||
PY
|
||||
)"
|
||||
|
||||
if command -v bot-bottle >/dev/null 2>&1; then
|
||||
BOT_BOTTLE_BIN="bot-bottle"
|
||||
elif [ -x "${HOME}/.local/bin/bot-bottle" ]; then
|
||||
BOT_BOTTLE_BIN="${HOME}/.local/bin/bot-bottle"
|
||||
say "note: add ${HOME}/.local/bin to your PATH to run 'bot-bottle' directly"
|
||||
elif [ -n "${USER_SCRIPTS}" ] && [ -x "${USER_SCRIPTS}/bot-bottle" ]; then
|
||||
BOT_BOTTLE_BIN="${USER_SCRIPTS}/bot-bottle"
|
||||
say "note: add ${USER_SCRIPTS} to your PATH to run 'bot-bottle' directly"
|
||||
else
|
||||
die "bot-bottle was installed but is not on PATH; add ~/.local/bin to PATH and re-run"
|
||||
die "bot-bottle was installed but is not on PATH; add ${USER_SCRIPTS:-your user scripts dir} to PATH and re-run"
|
||||
fi
|
||||
|
||||
# --- verify ------------------------------------------------------------------
|
||||
|
||||
@@ -2,8 +2,12 @@
|
||||
|
||||
`doctor` is a store-free diagnostic — it must run on a fresh install
|
||||
before any DB migration, and its exit code gates only the two hard
|
||||
prerequisites (Python and an available backend). The config-dir check is
|
||||
advisory and never affects the exit code.
|
||||
prerequisites (Python and at least one *ready* backend). The config-dir
|
||||
check is advisory and never affects the exit code.
|
||||
|
||||
Backend readiness is probed with `is_backend_ready()` (a full status()
|
||||
check), not the cheap PATH-only `is_backend_available()` — a host with a
|
||||
stopped daemon or half-configured backend must not report `ok`.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -26,26 +30,43 @@ def _run(argv: list[str] | None = None) -> tuple[int, str]:
|
||||
|
||||
|
||||
class TestDoctor(unittest.TestCase):
|
||||
def test_passes_when_python_and_backend_ok(self):
|
||||
def test_passes_when_python_and_backend_ready(self):
|
||||
with patch.object(doctor, "known_backend_names", return_value=("docker",)), \
|
||||
patch.object(doctor, "is_backend_available", return_value=True):
|
||||
patch.object(doctor, "is_backend_ready", return_value=True):
|
||||
code, out = _run()
|
||||
self.assertEqual(0, code)
|
||||
self.assertIn("ok: python", out)
|
||||
self.assertIn("ok: backend: available: docker", out)
|
||||
self.assertIn("ok: backend: docker: ready", out)
|
||||
|
||||
def test_fails_when_no_backend_available(self):
|
||||
def test_fails_when_no_backend_ready(self):
|
||||
# The regression the reviewer flagged: a backend whose binary is on PATH
|
||||
# but whose daemon/pool isn't ready must NOT pass. is_backend_ready is
|
||||
# the full status() check, so returning False here means "not ready".
|
||||
with patch.object(doctor, "known_backend_names", return_value=("docker", "firecracker")), \
|
||||
patch.object(doctor, "is_backend_available", return_value=False):
|
||||
patch.object(doctor, "is_backend_ready", return_value=False):
|
||||
code, out = _run()
|
||||
self.assertEqual(1, code)
|
||||
self.assertIn("fail: backend", out)
|
||||
self.assertIn("warn: backend: docker: not ready", out)
|
||||
|
||||
def test_passes_when_at_least_one_backend_ready(self):
|
||||
# docker not ready, firecracker ready → overall pass, mixed report.
|
||||
def ready(name: str, *, quiet: bool = False) -> bool:
|
||||
del quiet
|
||||
return name == "firecracker"
|
||||
|
||||
with patch.object(doctor, "known_backend_names", return_value=("docker", "firecracker")), \
|
||||
patch.object(doctor, "is_backend_ready", side_effect=ready):
|
||||
code, out = _run()
|
||||
self.assertEqual(0, code)
|
||||
self.assertIn("warn: backend: docker: not ready", out)
|
||||
self.assertIn("ok: backend: firecracker: ready", out)
|
||||
|
||||
def test_fails_when_python_too_old(self):
|
||||
# Force the version gate to fail without touching the interpreter.
|
||||
with patch.object(doctor, "MIN_PYTHON", (99, 0)), \
|
||||
patch.object(doctor, "known_backend_names", return_value=("docker",)), \
|
||||
patch.object(doctor, "is_backend_available", return_value=True):
|
||||
patch.object(doctor, "is_backend_ready", return_value=True):
|
||||
code, out = _run()
|
||||
self.assertEqual(1, code)
|
||||
self.assertIn("fail: python", out)
|
||||
@@ -57,7 +78,7 @@ class TestDoctor(unittest.TestCase):
|
||||
with tempfile.TemporaryDirectory() as tmp, \
|
||||
patch.object(doctor.Path, "home", return_value=Path(tmp)), \
|
||||
patch.object(doctor, "known_backend_names", return_value=("docker",)), \
|
||||
patch.object(doctor, "is_backend_available", return_value=True):
|
||||
patch.object(doctor, "is_backend_ready", return_value=True):
|
||||
code, out = _run()
|
||||
self.assertEqual(0, code)
|
||||
self.assertIn("warn: config", out)
|
||||
@@ -66,7 +87,7 @@ class TestDoctor(unittest.TestCase):
|
||||
with tempfile.TemporaryDirectory() as tmp, \
|
||||
patch.object(doctor.Path, "home", return_value=Path(tmp)), \
|
||||
patch.object(doctor, "known_backend_names", return_value=("docker",)), \
|
||||
patch.object(doctor, "is_backend_available", return_value=True):
|
||||
patch.object(doctor, "is_backend_ready", return_value=True):
|
||||
(Path(tmp) / ".bot-bottle").mkdir()
|
||||
code, out = _run()
|
||||
self.assertEqual(0, code)
|
||||
|
||||
@@ -9,6 +9,7 @@ create the config tree, install the package, and verify with `doctor`.
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import sysconfig
|
||||
import unittest
|
||||
from pathlib import Path
|
||||
|
||||
@@ -68,6 +69,33 @@ class TestInstallScript(unittest.TestCase):
|
||||
self.assertIn("EXTERNALLY-MANAGED", self.text)
|
||||
self.assertIn("pipx", self.text)
|
||||
|
||||
def test_resolves_user_scripts_dir_not_hardcoded(self):
|
||||
# The pip --user scripts dir differs by platform; the script must ask
|
||||
# the interpreter (sysconfig + the preferred *user* scheme) rather than
|
||||
# hardcoding Linux's ~/.local/bin (which is wrong on macOS python.org).
|
||||
self.assertIn("get_preferred_scheme", self.text)
|
||||
self.assertIn("sysconfig", self.text)
|
||||
# No hardcoded Linux path in executable lines (a comment may mention it).
|
||||
code = "\n".join(
|
||||
ln for ln in self.text.splitlines()
|
||||
if ln.strip() and not ln.lstrip().startswith("#")
|
||||
)
|
||||
self.assertNotIn(".local/bin", code)
|
||||
|
||||
def test_macos_user_scheme_is_not_dot_local_bin(self):
|
||||
# The case the fix exists for: a python.org macOS interpreter uses the
|
||||
# osx_framework_user scheme, whose scripts land under
|
||||
# ~/Library/Python/<X.Y>/bin — NOT ~/.local/bin. Drive the same
|
||||
# sysconfig lookup install.sh uses, with a mac-like userbase, to prove
|
||||
# it resolves a non-~/.local/bin directory.
|
||||
self.assertIn("osx_framework_user", sysconfig.get_scheme_names())
|
||||
scripts = sysconfig.get_path(
|
||||
"scripts", "osx_framework_user",
|
||||
vars={"userbase": "/Users/dev/Library/Python/3.11"},
|
||||
)
|
||||
self.assertEqual("/Users/dev/Library/Python/3.11/bin", scripts)
|
||||
self.assertNotIn("/.local/bin", scripts)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
unittest.main()
|
||||
|
||||
@@ -109,6 +109,15 @@ class TestWheelMode(unittest.TestCase):
|
||||
(root2 / "bot_bottle" / "cli" / "__init__.py").read_text(),
|
||||
)
|
||||
|
||||
def test_failed_stage_cleans_up_temp_dir(self):
|
||||
# A failure mid-stage must not leave a half-written temp dir behind.
|
||||
with self._wheel():
|
||||
base = resources.bot_bottle_root() / "build-root"
|
||||
with patch.object(resources.shutil, "copytree", side_effect=OSError("boom")):
|
||||
with self.assertRaises(OSError):
|
||||
resources.build_root()
|
||||
self.assertEqual([], list(base.glob(".staging-*")))
|
||||
|
||||
def test_rebuilds_when_stage_incomplete(self):
|
||||
# A crash mid-stage can leave a dir without its `.complete` marker; the
|
||||
# next call must rebuild it rather than trust the partial tree.
|
||||
|
||||
Reference in New Issue
Block a user