Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 45 additions & 6 deletions hermes_cli/managed_uv.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,54 @@ def resolve_uv() -> Optional[str]:
return None


def ensure_uv() -> Optional[str]:
class _UvResult(str):
"""``ensure_uv()`` return value that survives an update boundary.

``ensure_uv()``'s arity has flipped between a single path string and a
``(path, fresh_bootstrap)`` tuple across releases. ``hermes update`` runs
the call site from the *old*, already-imported ``hermes_cli.main`` against
this *freshly pulled* module, so the two can disagree on how many values
``ensure_uv()`` returns. An install parked on a 2-tuple release runs
``uv_bin, fresh_bootstrap = ensure_uv()`` against the single-value module
and crashes the first update: the returned path is a plain ``str``, which is
itself iterable, so the 2-target unpack walks its characters and raises
``ValueError: too many values to unpack (expected 2)`` (and on the failure
path the ``None`` return raises ``TypeError: cannot unpack non-iterable
NoneType``). This wrapper answers to both conventions:

uv_bin = ensure_uv() # behaves as the path str ("" when absent)
uv_bin, fresh = ensure_uv() # unpacks as (path|None, fresh_bootstrap)

Missing uv is the empty string (falsy) instead of ``None`` so legacy
2-target call sites can still unpack a failure without raising, while
``if not uv_bin`` keeps working for single-value callers.
"""

fresh_bootstrap: bool

def __new__(cls, path: Optional[str], fresh: bool = False) -> "_UvResult":
self = super().__new__(cls, path or "")
self.fresh_bootstrap = fresh
return self

def __iter__(self):
# Tuple-unpacking hook for legacy ``uv_bin, fresh = ensure_uv()`` sites.
# First element mirrors the historical contract: the path string, or
# ``None`` when uv is unavailable.
return iter(((str(self) or None), self.fresh_bootstrap))


def ensure_uv() -> "_UvResult":
"""Return the managed uv path, installing it first if necessary.

On failure returns ``None`` (never raises) so callers can fall
back to pip gracefully.
Returns a :class:`_UvResult` (a ``str`` subclass) that is both usable
directly as the path and unpackable as ``(path, fresh_bootstrap)`` for
older call sites. On failure the result is falsy (``""``) — never raises —
so callers can fall back to pip gracefully.
"""
existing = resolve_uv()
if existing:
return existing
return _UvResult(existing)

target = managed_uv_path()
target.parent.mkdir(parents=True, exist_ok=True)
Expand All @@ -71,7 +110,7 @@ def ensure_uv() -> Optional[str]:
except Exception as exc:
logger.warning("Managed uv install failed: %s", exc)
print(f" ✗ Failed to install managed uv: {exc}")
return None
return _UvResult(None)

# Verify
result = resolve_uv()
Expand All @@ -85,7 +124,7 @@ def ensure_uv() -> Optional[str]:
print(f" ✓ Managed uv installed ({version})")
else:
print(" ✗ Managed uv install appeared to succeed but binary not found")
return result
return _UvResult(result)


def update_managed_uv() -> Optional[str]:
Expand Down
47 changes: 45 additions & 2 deletions tests/hermes_cli/test_managed_uv.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,12 +92,55 @@ def fake_install(target):
assert path == str(tmp_path / "bin" / "uv")
mock_install.assert_called_once()

def test_install_failure_returns_none(self, tmp_path):
def test_install_failure_returns_falsy(self, tmp_path):
with patch("hermes_cli.managed_uv.get_hermes_home", return_value=tmp_path), \
patch("hermes_cli.managed_uv._install_uv", side_effect=RuntimeError("network down")):
from hermes_cli.managed_uv import ensure_uv
path = ensure_uv()
assert path is None
# Failure is a falsy sentinel (not None) so legacy 2-target call
# sites can still unpack it without raising — see
# TestEnsureUvUpdateBoundary for why.
assert not path


class TestEnsureUvUpdateBoundary:
"""``ensure_uv()`` must answer to both the single-value and the legacy
``(path, fresh_bootstrap)`` call conventions.

``hermes update`` runs the call site from the old, already-imported
``hermes_cli.main`` against the freshly pulled ``managed_uv``. A release
parked on a ``(path, fresh)`` tuple runs ``uv_bin, fresh = ensure_uv()``
against the single-value module; the path is an iterable ``str`` so the
2-target unpack walked its characters and raised
``ValueError: too many values to unpack (expected 2)`` (root cause behind
PR #39763), or ``TypeError`` on the ``None`` failure path. The result must
therefore be usable as a bare path *and* unpackable as a 2-tuple, in both
the success and failure cases.
"""

def test_success_usable_as_single_value(self, tmp_path):
_make_executable(tmp_path / "bin" / "uv")
with patch("hermes_cli.managed_uv.get_hermes_home", return_value=tmp_path):
from hermes_cli.managed_uv import ensure_uv
uv_bin = ensure_uv()
assert uv_bin == str(tmp_path / "bin" / "uv")
assert bool(uv_bin) is True

def test_success_unpacks_as_legacy_two_tuple(self, tmp_path):
_make_executable(tmp_path / "bin" / "uv")
with patch("hermes_cli.managed_uv.get_hermes_home", return_value=tmp_path):
from hermes_cli.managed_uv import ensure_uv
uv_bin, fresh = ensure_uv() # old: uv_bin, fresh_bootstrap = ensure_uv()
assert uv_bin == str(tmp_path / "bin" / "uv")
assert fresh is False

def test_failure_unpacks_without_raising(self, tmp_path):
with patch("hermes_cli.managed_uv.get_hermes_home", return_value=tmp_path), \
patch("hermes_cli.managed_uv._install_uv", side_effect=RuntimeError("network down")):
from hermes_cli.managed_uv import ensure_uv
uv_bin, fresh = ensure_uv()
assert uv_bin is None
assert fresh is False


# ---------------------------------------------------------------------------
Expand Down
Loading