fix(pm): use the selected generation for venv freshness
A lock-only stamp reported current without a usable environment. Use PM's recorded inputs and boot-time selection for own-tree checks. Do not read or write the foreign-bootstrap stamp for that path. PM check and sync now share environment validation. Malformed records remain intact and produce a visible error instead of a healthy result. Foreign-root bootstrap and bare-import behavior remain unchanged. Verification: real temporary PM builds, deletion and replacement, plugin/Python/extra changes, malformed records and transaction neighbors. The final isolated gate passed 115 tests with no failures. Lint passed. No publication-lock rewrite or packaged adoption enforcement included.
This commit is contained in:
@@ -557,29 +557,12 @@ def run_steps(steps: Iterable) -> dict:
|
||||
|
||||
|
||||
def resync_and_reexec(args) -> int | None:
|
||||
"""Phase 1 of the update-phase: own the venv sync, then hand off.
|
||||
"""Sync dependencies before a fresh-process handoff.
|
||||
|
||||
Runs in whatever interpreter called us — usually the venv python the
|
||||
tree swap just invalidated — so it touches as little as possible:
|
||||
``venv_sync`` (stdlib-only at import by contract) decides from the
|
||||
lockfile digest whether the venv is stale, syncs it via pm when it
|
||||
is, and then this process REPLACES ITSELF with a fresh interpreter
|
||||
that has never mapped a pre-sync module.
|
||||
|
||||
Returns None to mean "you are already the fresh process — run phase
|
||||
2", or an exit code to propagate.
|
||||
|
||||
The boundary is double-guarded:
|
||||
|
||||
* the ``--resumed-after-sync`` argv flag is the loop-proofing — the
|
||||
exec'd child must not sync again even if another writer moves the
|
||||
stamp between exec and check, because a flag in argv cannot race;
|
||||
* the venv_sync stamp is the idempotence — a re-run of the whole
|
||||
update sees a fresh stamp and skips the sync entirely.
|
||||
|
||||
POSIX uses ``os.execv``: same pid, so an update-lock marker's owner
|
||||
stays literally correct. Windows has no true exec — spawn + wait +
|
||||
propagate, and the child passes any lock by process ancestry.
|
||||
``venv_sync`` uses PM's recorded inputs and selected environment for
|
||||
this checkout. The resumed flag prevents a second sync after handoff.
|
||||
Return None to continue in this process, or an exit code to propagate.
|
||||
POSIX replaces the process. Windows waits for a child and returns its code.
|
||||
"""
|
||||
if args.resumed_after_sync:
|
||||
return None
|
||||
|
||||
@@ -21,13 +21,12 @@ One of the two separately-invocable, stdlib-only-at-import halves of
|
||||
Stdlib-only at import is a hard contract: this runs on freshly-cloned
|
||||
trees where the venv does not exist yet, and after tree swaps where the
|
||||
venv is not trustworthy — exactly the moments a third-party import
|
||||
would explode. ``pm`` is itself stdlib-only first-party code, and it is
|
||||
imported lazily, only once a sync is actually due.
|
||||
would explode. ``pm`` is first-party code and is imported at sync time,
|
||||
not when this module is imported.
|
||||
|
||||
Why the sync is not just "run uv every time": ``uv sync`` on an
|
||||
already-current venv still costs ~1-2s of resolver work, and the boot
|
||||
path calls this through ``post_update``. The lockfile digest recorded in
|
||||
``<runtime dir>/cache/venv-sync.json`` makes currency a file read.
|
||||
PM decides whether this checkout needs a sync from its recorded inputs
|
||||
and selected environment. A foreign bootstrap root retains its separate
|
||||
lockfile-digest stamp until execution moves into that checkout.
|
||||
|
||||
Invocation:
|
||||
|
||||
@@ -87,13 +86,7 @@ def _stamp_path(project_root: Path) -> Path:
|
||||
|
||||
|
||||
def _lock_digest(project_root: Path) -> str | None:
|
||||
"""Content hash of what a sync would consume.
|
||||
|
||||
pyproject.toml is part of the key: an extras edit without a lock
|
||||
bump must re-sync (same reasoning as pm's venv expected_stamp keying
|
||||
on uv.lock — pyproject rides along here because this module cannot
|
||||
assume pm's ledger exists yet).
|
||||
"""
|
||||
"""Hash the manifest and lock inputs for a foreign-root bootstrap."""
|
||||
h = hashlib.sha256()
|
||||
found = False
|
||||
for name in ("uv.lock", "pyproject.toml"):
|
||||
@@ -179,31 +172,8 @@ def write_stamp(project_root: Path, digest: str) -> None:
|
||||
os.replace(tmp, path)
|
||||
|
||||
|
||||
def _pm_sync(root: Path) -> dict:
|
||||
"""Bring ``root``'s venv current. pm's ledger when root IS this tree,
|
||||
a direct pinned-uv drive when it is a foreign clone.
|
||||
|
||||
Returns ``{"ok": bool, "detail": str | None}``.
|
||||
"""
|
||||
try:
|
||||
from pm import paths as pm_paths
|
||||
|
||||
own_tree = Path(pm_paths.repo_root()).resolve() == root.resolve()
|
||||
except Exception:
|
||||
own_tree = False
|
||||
|
||||
if own_tree:
|
||||
# pm owns the extras ledger; explicit=True because reaching a
|
||||
# stale-venv verdict here IS the deliberate remedy (installer /
|
||||
# post-update path), the same trust `hermes update` carries.
|
||||
try:
|
||||
from pm import sync_venv
|
||||
|
||||
sync_venv(explicit=True)
|
||||
except Exception as exc:
|
||||
return {"ok": False, "detail": str(exc)}
|
||||
return {"ok": True, "detail": None}
|
||||
|
||||
def _sync_foreign(root: Path) -> dict:
|
||||
"""Bootstrap a foreign checkout with PM's pinned uv and environment."""
|
||||
uv, env = _managed_uv()
|
||||
if uv is None:
|
||||
return {
|
||||
@@ -225,18 +195,28 @@ def _pm_sync(root: Path) -> dict:
|
||||
|
||||
|
||||
def sync(project_root: Path | None = None, *, check: bool = False) -> dict:
|
||||
"""Bring the venv up to the tree. Returns a state dict, never raises.
|
||||
|
||||
States: ``sealed`` (nothing to sync, by design), ``current`` (stamp
|
||||
matches the lockfile digest), ``synced`` (the sync ran and the stamp
|
||||
moved), ``failed`` (the sync did not converge — detail says why),
|
||||
``would-sync`` (check mode found staleness and stopped).
|
||||
"""
|
||||
"""Report or sync dependencies. A malformed install stamp is a build error."""
|
||||
root = Path(project_root) if project_root else _project_root()
|
||||
|
||||
if _is_sealed(root):
|
||||
return {"state": "sealed", "ok": True}
|
||||
|
||||
try:
|
||||
from pm import paths as pm_paths
|
||||
|
||||
if Path(pm_paths.repo_root()).resolve() == root.resolve():
|
||||
from pm import sync_venv
|
||||
from pm.ensure import venv_is_current
|
||||
|
||||
if venv_is_current():
|
||||
return {"state": "current", "ok": True}
|
||||
if check:
|
||||
return {"state": "would-sync", "ok": True}
|
||||
sync_venv(explicit=True)
|
||||
return {"state": "synced", "ok": True}
|
||||
except Exception as exc:
|
||||
return {"state": "failed", "ok": False, "detail": str(exc)}
|
||||
|
||||
digest = _lock_digest(root)
|
||||
if digest is None:
|
||||
return {
|
||||
@@ -251,7 +231,7 @@ def sync(project_root: Path | None = None, *, check: bool = False) -> dict:
|
||||
if check:
|
||||
return {"state": "would-sync", "ok": True}
|
||||
|
||||
result = _pm_sync(root)
|
||||
result = _sync_foreign(root)
|
||||
if not result["ok"]:
|
||||
# No stamp write: the next run must try again, not skip.
|
||||
return {"state": "failed", "ok": False, "detail": result["detail"]}
|
||||
|
||||
42
pm/ensure.py
42
pm/ensure.py
@@ -464,13 +464,32 @@ def env_for(*names: str, base_env: Optional[dict] = None) -> dict[str, str]:
|
||||
|
||||
|
||||
def _runtime_state_matches(fact: dict, stamp: str) -> bool:
|
||||
if fact.get("stamp") != stamp:
|
||||
if not isinstance(fact, dict) or fact.get("stamp") != stamp:
|
||||
return False
|
||||
environment = fact.get("environment")
|
||||
if environment is None:
|
||||
return True # shipped/pre-generation state
|
||||
from pathlib import Path
|
||||
return isinstance(environment, str) and (Path(environment) / "pyvenv.cfg").is_file()
|
||||
from hermes_cli.runtime_paths import selected_venv
|
||||
|
||||
try:
|
||||
environment = selected_venv(paths.repo_root())
|
||||
except (OSError, RuntimeError, ValueError):
|
||||
return False
|
||||
recorded = fact.get("environment")
|
||||
if recorded is not None and (not isinstance(recorded, str) or Path(recorded).resolve() != environment):
|
||||
return False
|
||||
return (environment / "pyvenv.cfg").is_file()
|
||||
|
||||
|
||||
def venv_is_current() -> bool:
|
||||
"""Use the recorded PM inputs and the boot-time environment selection."""
|
||||
fact = Facts(paths.runtime_facts_path(), strict=True).get("venv")
|
||||
if fact is None:
|
||||
fact = Facts(paths.facts_path(), strict=True).get("venv")
|
||||
if fact is None:
|
||||
return False
|
||||
if (not isinstance(fact, dict) or not isinstance(fact.get("stamp"), str) or not fact["stamp"]
|
||||
or not isinstance(fact.get("extras"), list)
|
||||
or any(not isinstance(extra, str) for extra in fact["extras"])):
|
||||
raise ValueError("invalid recorded dependency state")
|
||||
return _runtime_state_matches(fact, get_package("venv").expected_stamp(fact["extras"]))
|
||||
|
||||
|
||||
def sync_venv(extras: Optional[list[str]] = None, *, explicit: bool = False, plugin_dirs=None, before_publish=None, repair: bool = False) -> None:
|
||||
@@ -669,11 +688,12 @@ def check() -> list[str]:
|
||||
venv = get_package("venv")
|
||||
except KeyError:
|
||||
venv = None
|
||||
fact = Facts(paths.runtime_facts_path()).get("venv") or facts.get("venv")
|
||||
if venv is not None and fact is not None:
|
||||
expected = venv.expected_stamp(fact.get("extras", []))
|
||||
if not _runtime_state_matches(fact, expected):
|
||||
problems.append("venv: out of sync with uv.lock")
|
||||
if venv is not None and (paths.runtime_facts_path().is_file() or facts.get("venv") is not None):
|
||||
try:
|
||||
if not venv_is_current():
|
||||
problems.append("venv: out of sync with uv.lock")
|
||||
except (OSError, RuntimeError, ValueError) as exc:
|
||||
problems.append(f"venv: {exc}")
|
||||
return problems
|
||||
|
||||
|
||||
|
||||
122
tests/hermes_cli/test_venv_sync_currency.py
Normal file
122
tests/hermes_cli/test_venv_sync_currency.py
Normal file
@@ -0,0 +1,122 @@
|
||||
"""Own-tree freshness follows the selected PM generation, not a second stamp."""
|
||||
import importlib
|
||||
import json
|
||||
import os
|
||||
from pathlib import Path
|
||||
import subprocess
|
||||
|
||||
import yaml
|
||||
|
||||
import pm
|
||||
from hermes_cli import venv_sync
|
||||
from hermes_cli.runtime_paths import install_state_dir, selected_venv
|
||||
from pm import paths
|
||||
from pm.lock import Lockfile
|
||||
from tests.pm.test_plugin_survival_contract import admission_env # noqa: F401
|
||||
|
||||
|
||||
def test_check_uses_real_pm_selection_and_keeps_invalid_evidence(admission_env, monkeypatch, capsys):
|
||||
root, home = admission_env
|
||||
core = root / 'core'
|
||||
ensure = importlib.import_module('pm.ensure')
|
||||
pin_path = root / 'pins.json'
|
||||
pins = Lockfile(pin_path)
|
||||
pins.set_pin('python', '1.0', {'any': {'url': 'https://example.invalid/python', 'sha256': 'a' * 64}})
|
||||
pins.save()
|
||||
monkeypatch.setattr(paths, 'lockfile_path', lambda: pin_path)
|
||||
venv_sync.write_stamp(core, venv_sync._lock_digest(core))
|
||||
cached = venv_sync._stamp_path(core).read_bytes()
|
||||
|
||||
def check(expected, code=0):
|
||||
capsys.readouterr()
|
||||
result = venv_sync.main(['--project-root', str(core), '--check', '--json'])
|
||||
output = json.loads(capsys.readouterr().out)
|
||||
assert result == code, output
|
||||
assert output['state'] == expected, output
|
||||
assert venv_sync._stamp_path(core).read_bytes() == cached
|
||||
return output
|
||||
|
||||
check('would-sync')
|
||||
pm.sync_venv(explicit=True)
|
||||
facts_path = paths.runtime_facts_path()
|
||||
pristine = facts_path.read_bytes()
|
||||
selected = selected_venv(core)
|
||||
assert selected.is_relative_to(install_state_dir(core) / 'environments')
|
||||
check('current')
|
||||
assert not any(problem.startswith('venv:') for problem in ensure.check())
|
||||
|
||||
config = home / 'config.yaml'
|
||||
old_config = config.read_bytes()
|
||||
member = home / 'plugins' / 'extra-member'
|
||||
member.mkdir(parents=True)
|
||||
(member / 'plugin.yaml').write_text('name: extra-member\npython_dependencies: ["example-dep==1"]\n', encoding='utf-8')
|
||||
config.write_text(yaml.safe_dump({'plugins': {'enabled': ['extra-member']}}), encoding='utf-8')
|
||||
check('would-sync')
|
||||
assert any(problem.startswith('venv:') for problem in ensure.check())
|
||||
config.write_bytes(old_config)
|
||||
check('current')
|
||||
|
||||
altered = json.loads(pristine)
|
||||
altered['packages']['venv']['extras'] = ['changed-extra']
|
||||
facts_path.write_text(json.dumps(altered), encoding='utf-8')
|
||||
check('would-sync')
|
||||
assert any(problem.startswith('venv:') for problem in ensure.check())
|
||||
facts_path.write_bytes(pristine)
|
||||
|
||||
pins.set_pin('python', '1.0', {'any': {'url': 'https://example.invalid/python', 'sha256': 'b' * 64}})
|
||||
pins.save()
|
||||
check('would-sync')
|
||||
assert any(problem.startswith('venv:') for problem in ensure.check())
|
||||
pins.set_pin('python', '1.0', {'any': {'url': 'https://example.invalid/python', 'sha256': 'a' * 64}})
|
||||
pins.save()
|
||||
|
||||
marker = selected / 'pyvenv.cfg'
|
||||
marker_bytes = marker.read_bytes()
|
||||
marker.unlink()
|
||||
check('would-sync')
|
||||
assert any(problem.startswith('venv:') for problem in ensure.check())
|
||||
marker.write_bytes(marker_bytes)
|
||||
check('current')
|
||||
|
||||
outside = root / 'foreign-environment'
|
||||
outside.mkdir()
|
||||
(outside / 'pyvenv.cfg').write_bytes(marker_bytes)
|
||||
altered = json.loads(pristine)
|
||||
altered['packages']['venv']['environment'] = str(outside)
|
||||
facts_path.write_text(json.dumps(altered), encoding='utf-8')
|
||||
check('would-sync')
|
||||
assert any(problem.startswith('venv:') for problem in ensure.check())
|
||||
|
||||
for invalid in (b'not JSON', b'{"schema":1,"packages":{"venv":[]}}'):
|
||||
facts_path.write_bytes(invalid)
|
||||
output = check('failed', 1)
|
||||
assert output.get('detail')
|
||||
assert any(problem.startswith('venv:') for problem in ensure.check())
|
||||
assert facts_path.read_bytes() == invalid
|
||||
assert not facts_path.with_suffix('.corrupt').exists()
|
||||
facts_path.unlink()
|
||||
check('would-sync')
|
||||
assert not facts_path.exists()
|
||||
|
||||
|
||||
def test_own_tree_sync_reuses_pm_without_writing_an_extra_stamp(admission_env):
|
||||
root, home = admission_env
|
||||
core = root / 'core'
|
||||
assert not venv_sync._stamp_path(core).exists()
|
||||
assert venv_sync.sync(core) == {'state': 'synced', 'ok': True}
|
||||
environment = selected_venv(core)
|
||||
saved = paths.runtime_facts_path().read_bytes()
|
||||
assert not venv_sync._stamp_path(core).exists()
|
||||
assert venv_sync.sync(core) == {'state': 'current', 'ok': True}
|
||||
assert paths.runtime_facts_path().read_bytes() == saved
|
||||
python = environment / ('Scripts/python.exe' if os.name == 'nt' else 'bin/python')
|
||||
result = subprocess.run([str(python), '-I', '-c', 'import sys; print(sys.prefix)'],
|
||||
cwd=home, capture_output=True, text=True, check=True, timeout=30)
|
||||
assert Path(result.stdout.strip()).resolve() == environment.resolve()
|
||||
(environment / 'pyvenv.cfg').unlink()
|
||||
assert venv_sync.sync(core) == {'state': 'synced', 'ok': True}
|
||||
replacement = selected_venv(core)
|
||||
assert replacement != environment
|
||||
assert (replacement / 'pyvenv.cfg').is_file()
|
||||
assert venv_sync.sync(core) == {'state': 'current', 'ok': True}
|
||||
assert not venv_sync._stamp_path(core).exists()
|
||||
@@ -604,10 +604,19 @@ class FakeVenv(StatePackage):
|
||||
|
||||
def apply(self, extras):
|
||||
self.applied.append(list(extras))
|
||||
from hermes_cli.runtime_paths import install_state_dir
|
||||
|
||||
environment = install_state_dir(paths.repo_root()) / "environments" / str(len(self.applied)) / "venv"
|
||||
environment.mkdir(parents=True)
|
||||
(environment / "pyvenv.cfg").write_text("home = test\n", encoding="utf-8")
|
||||
return {"environment": environment}
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def venv_env(pm_env):
|
||||
def venv_env(pm_env, tmp_path, monkeypatch):
|
||||
project = tmp_path / "venv-project"
|
||||
project.mkdir()
|
||||
monkeypatch.setattr(paths, "repo_root", lambda: project)
|
||||
fake = FakeVenv()
|
||||
registry._packages["venv"] = fake
|
||||
return pm_env, fake
|
||||
|
||||
Reference in New Issue
Block a user