fix(update): probe the venv interpreter, not the driving one
Self-review against the sibling probe `_venv_core_imports_healthy` surfaced this: that helper deliberately resolves the project venv's python rather than using `sys.executable`, because `hermes update` may be driven by a different interpreter than the install's own. The new import guard had the same requirement and missed it. Probing `sys.executable` would validate a tree the user never actually runs — and that divergence is most likely on Windows, the exact platform this guard was added for. Falls back to the running interpreter when there is no venv (normal in a dev checkout). Regression test asserts the venv python is chosen; it fails when the fix is reverted.
This commit is contained in:
parent
baecc840e5
commit
aa5d4fd6ee
|
|
@ -188,6 +188,11 @@ def _validate_critical_modules_import(root) -> tuple[bool, str | None, str | Non
|
|||
updater would pollute ``sys.modules`` and execute import-time side effects
|
||||
against the half-updated tree. Costs ~0.4s.
|
||||
|
||||
Uses the project venv's interpreter when there is one (matching
|
||||
``_venv_core_imports_healthy``): ``hermes update`` can be driven by a
|
||||
different Python than the install's own, and probing the wrong
|
||||
interpreter would test a tree the user never runs.
|
||||
|
||||
Returns ``(ok, failing_module, error_message)``.
|
||||
"""
|
||||
probe = (
|
||||
|
|
@ -203,8 +208,17 @@ def _validate_critical_modules_import(root) -> tuple[bool, str | None, str | Non
|
|||
"raise SystemExit(0)\n" % (_UPDATE_CRITICAL_MODULES,)
|
||||
)
|
||||
try:
|
||||
interpreter = sys.executable
|
||||
try:
|
||||
bin_dir = "Scripts" if _m()._is_windows() else "bin"
|
||||
python_name = "python.exe" if _m()._is_windows() else "python"
|
||||
venv_python = Path(root) / "venv" / bin_dir / python_name
|
||||
if venv_python.exists():
|
||||
interpreter = str(venv_python)
|
||||
except Exception:
|
||||
pass # fall back to the running interpreter
|
||||
result = subprocess.run(
|
||||
[sys.executable, "-c", probe],
|
||||
[interpreter, "-c", probe],
|
||||
cwd=str(root),
|
||||
capture_output=True,
|
||||
text=True,
|
||||
|
|
|
|||
|
|
@ -115,3 +115,35 @@ def test_hint_stays_silent_for_unrelated_failures(exc):
|
|||
if isinstance(exc, ImportError) and not isinstance(exc, ModuleNotFoundError):
|
||||
exc.name = "requests"
|
||||
assert partial_update_hint(exc) == []
|
||||
|
||||
|
||||
def test_import_guard_prefers_the_project_venv_interpreter(monkeypatch, tmp_path):
|
||||
"""``hermes update`` can run under a different Python than the install's.
|
||||
|
||||
Probing ``sys.executable`` would then validate a tree the user never
|
||||
actually runs -- the same reasoning behind ``_venv_core_imports_healthy``.
|
||||
On Windows (the platform this guard exists for) the driving interpreter
|
||||
and the venv interpreter routinely differ.
|
||||
"""
|
||||
bin_dir = "Scripts" if update_cmd._m()._is_windows() else "bin"
|
||||
name = "python.exe" if update_cmd._m()._is_windows() else "python"
|
||||
venv_python = tmp_path / "venv" / bin_dir / name
|
||||
venv_python.parent.mkdir(parents=True)
|
||||
venv_python.write_text("")
|
||||
|
||||
seen: dict = {}
|
||||
|
||||
def fake_run(cmd, **kwargs):
|
||||
seen["interpreter"] = cmd[0]
|
||||
|
||||
class R:
|
||||
returncode = 0
|
||||
stdout = ""
|
||||
stderr = ""
|
||||
|
||||
return R()
|
||||
|
||||
monkeypatch.setattr(update_cmd.subprocess, "run", fake_run)
|
||||
update_cmd._validate_critical_modules_import(tmp_path)
|
||||
|
||||
assert seen["interpreter"] == str(venv_python)
|
||||
|
|
|
|||
Loading…
Reference in New Issue