diff --git a/hermes_cli/venv_sync.py b/hermes_cli/venv_sync.py index e9bb6ccf35..08046dcac2 100644 --- a/hermes_cli/venv_sync.py +++ b/hermes_cli/venv_sync.py @@ -252,13 +252,22 @@ def prepare_launch(project_root: Path, argv: list[str]) -> Path | None: lock = UpdateLock() if not lock.acquire(): raise RuntimeError("an update is still running; wait for it to exit, then relaunch Hermes") - # The tail imports the application, whose entry point runs this very function: - # under the launching process's own claim (its pid is our ancestor) we ARE that - # tail and owe nothing — without this, a pending marker recurses forever. - if not lock.acquired and read_live_update() is not None: - return None try: - _finish_source_update(root, current=current, pending=pending) + # The tail imports the application, whose entry point runs this very function: + # under the launching process's own claim (its pid is our ancestor) we ARE that + # tail and owe nothing — without this, a pending marker recurses forever. + if not lock.acquired and read_live_update() is not None: + if current: + return None + # A process the update spawns before its dependencies are current (a restarted + # gateway) would boot on a tree built for another interpreter. Sync — never the + # tail, which is the updater's — then relaunch below into a current install. + _sync_source_dependencies(root, arm=False) + if not pm.venv_is_current(project_root=root): + # Relaunching would land back here and sync again, forever. + raise RuntimeError("dependency sync left this install out of date") + else: + _finish_source_update(root, current=current, pending=pending) finally: lock.release() python = resolve_store_python(root) @@ -273,9 +282,8 @@ def prepare_launch(project_root: Path, argv: list[str]) -> Path | None: def _finish_source_update(root: Path, *, current: bool, pending: Path) -> None: """Sync dependencies when they are stale, then run the tail the marker still owes.""" import sys - import pm from hermes_cli._early_recovery import _marker_owner_is_live - from pm.environments import activation_environment, runtime_facts_path + from pm.environments import activation_environment if not current: # Existing markers guard liveness, never create the completion obligation. @@ -284,24 +292,7 @@ def _finish_source_update(root: Path, *, current: bool, pending: Path) -> None: if any(_marker_owner_is_live(marker) for marker in legacy_markers): raise RuntimeError("an update is still running; wait for it to exit, then relaunch Hermes") print("hermes: completing source-update dependencies...", file=sys.stderr, flush=True) - # Owed from before the sync commits: a crash between the commit and the - # tail must leave the tail, not a "current" install with nothing built. - refuse_foreign_owned_venv(root) - arm_completion(root) - # Main-era installs have no PM ledger; carry what their venv held. - # Established PM installs retain their recorded extras and plugin union instead. - from pm.client import ensure_tools_for_sync - from pm.extras import legacy_selection - extras = legacy_selection(root) if not runtime_facts_path(root).is_file() else None - # Same order as `hermes update`: an interrupted update or a hand-run - # `git pull` leaves this tree's lockfile ahead of the installed tools. - ensure_tools_for_sync() - pm.sync_venv(extras, explicit=True, project_root=root) - collect_superseded_generations(root) - # These can predate the swap. Once PM commits the replacement they - # must not make early recovery immediately rebuild it a second time. - for name in (".update-incomplete", ".lazy-refresh-incomplete"): - (root / name).unlink(missing_ok=True) + _sync_source_dependencies(root, arm=True) else: print("hermes: finishing an interrupted source update...", file=sys.stderr, flush=True) # Sync commits the dependency generation, but a source update also owes @@ -330,6 +321,35 @@ def _finish_source_update(root: Path, *, current: bool, pending: Path) -> None: clear_completion(root) +def _sync_source_dependencies(root: Path, *, arm: bool) -> None: + """Commit the tree's dependency generation; *arm* also owes the tail afterwards.""" + import sys + import pm + from pm.client import ensure_tools_for_sync + from pm.environments import runtime_facts_path + from pm.extras import legacy_selection + + if not arm: + print("hermes: preparing dependencies for this update...", file=sys.stderr, flush=True) + refuse_foreign_owned_venv(root) + if arm: + # Owed from before the sync commits: a crash between the commit and the + # tail must leave the tail, not a "current" install with nothing built. + arm_completion(root) + # Main-era installs have no PM ledger; carry what their venv held. + # Established PM installs retain their recorded extras and plugin union instead. + extras = legacy_selection(root) if not runtime_facts_path(root).is_file() else None + # Same order as `hermes update`: an interrupted update or a hand-run + # `git pull` leaves this tree's lockfile ahead of the installed tools. + ensure_tools_for_sync() + pm.sync_venv(extras, explicit=True, project_root=root) + collect_superseded_generations(root) + # These can predate the swap. Once PM commits the replacement they + # must not make early recovery immediately rebuild it a second time. + for name in (".update-incomplete", ".lazy-refresh-incomplete"): + (root / name).unlink(missing_ok=True) + + def relaunch_command( python: Path, root: Path, argv: list[str], original: list[str], module: str | None, ) -> list[str]: diff --git a/pm/environments.py b/pm/environments.py index a7af2aab27..95e874f7c3 100644 --- a/pm/environments.py +++ b/pm/environments.py @@ -80,7 +80,8 @@ def record_activation_inputs(stamps: Path, mtimes: dict[str, int], project_root: os.utime(stamp, ns=(mtime, mtime)) -def base_venv(project_root: Path) -> Path: +def payload_venv(project_root: Path) -> Path | None: + """The environment a sealed payload ships beside its tree, or ``None``.""" root = Path(project_root).resolve() manifest_path = root.parent / "manifest.json" if manifest_path.is_file(): @@ -90,7 +91,11 @@ def base_venv(project_root: Path) -> Path: if not venv.is_relative_to(root.parent): raise RuntimeError("payload environment escapes its root") return venv - return project_venv_dir(root) or root / "venv" + return None + + +def base_venv(project_root: Path) -> Path: + return payload_venv(project_root) or project_venv_dir(Path(project_root).resolve()) or Path(project_root).resolve() / "venv" def store_root(project_root: Path) -> Path: @@ -152,11 +157,25 @@ def selected_venv(project_root: Path) -> Path: ``flush_before_selecting``, so the ``pyvenv.cfg`` probe below is a sanity check against a vanished tree, not the durability guarantee. """ + return _recorded_venv(project_root) or base_venv(project_root) + + +def committed_venv(project_root: Path) -> Path | None: + """The environment PM committed for this install (or a sealed payload's own), else ``None``. + + Unlike ``selected_venv`` this never answers with the in-tree ``venv``/``.venv``: that tree + predates PM and is built for whichever interpreter created it, so loading it from PM's store + Python mixes ABIs (compiled modules vanish) and PM deletes it once a generation is committed. + """ + return _recorded_venv(project_root) or payload_venv(project_root) + + +def _recorded_venv(project_root: Path) -> Path | None: path = runtime_facts_path(project_root) try: data = json.loads(path.read_text(encoding="utf-8-sig")) except FileNotFoundError: - return base_venv(project_root) + return None except (OSError, ValueError) as exc: raise RuntimeError(f"cannot read dependency environment: {path}") from exc try: @@ -165,7 +184,7 @@ def selected_venv(project_root: Path) -> Path: except AttributeError as exc: raise RuntimeError(f"invalid dependency environment record: {path}") from exc if value is None: - return base_venv(project_root) + return None if not isinstance(value, str): raise RuntimeError("invalid dependency environment path") environment = Path(value).resolve() @@ -258,6 +277,20 @@ def running_from_selected_environment(project_root: Path) -> bool: return any(Path(entry).resolve() == selected for entry in sys.path if entry) +def _require_own_dependencies(project_root: Path) -> None: + """With nothing committed, an interpreter keeps the packages it booted with. + + PM's store Python boots with none, so for it there is nothing to keep: refuse instead of + running on whatever PYTHONPATH it inherited (historically the pre-PM in-tree venv). + """ + import sys + + if sys.prefix != sys.base_prefix: + return # a venv interpreter (developer .venv, test env) carries its own packages + if Path(sys.base_prefix).resolve().is_relative_to(store_root(project_root).resolve()): + raise RuntimeError("no dependency environment is committed for this install") + + def activate_dependencies(project_root: Path) -> None: """Select the committed tree at process boot, before third-party imports. @@ -275,20 +308,24 @@ def activate_dependencies(project_root: Path) -> None: with runtime_lock(project_root) as held: if held: recover_publication(project_root) - environment = selected_venv(project_root) + environment = committed_venv(project_root) + if environment is None: + return _require_own_dependencies(project_root) release = lease_generation(environment) # Without the lock, an installer may commit a new generation between the # read and the lease, leaving the leased one unselected and collectable. - while not held and (current := selected_venv(project_root)) != environment: + while not held and (current := committed_venv(project_root)) not in (None, environment): release() environment, release = current, lease_generation(current) selected = site_packages(environment) if not selected.is_dir() and not runtime_facts_path(project_root).is_file(): return else: - # Older installs and sealed payloads still select once, before imports. + # Sealed payloads still select once, before imports. # Never consult VIRTUAL_ENV: it can describe the invoking shell's Python. - environment = base_venv(project_root) + environment = payload_venv(project_root) + if environment is None: + return _require_own_dependencies(project_root) selected = site_packages(environment) if not selected.is_dir(): return # External/Nix interpreter owns its original sys.path. @@ -316,10 +353,13 @@ def activation_environment(project_root: Path) -> dict[str, str]: from pm.registry import all_packages env = env_for(*all_packages()) - selected = site_packages(selected_venv(project_root)) + environment = committed_venv(project_root) env.pop("PYTHONHOME", None) env.pop("VIRTUAL_ENV", None) - env["PYTHONPATH"] = os.pathsep.join([str(project_root.resolve()), str(selected)]) + # Nothing committed: the child's own hermes_bootstrap decides (a bare store Python refuses), + # rather than inheriting the pre-PM in-tree venv from here. + env["PYTHONPATH"] = os.pathsep.join([str(project_root.resolve()), + *([str(site_packages(environment))] if environment else [])]) # The child-process sentinel. Its VALUE is the installed-state file this # environment was composed against, so a consumer learns that it inherited # an activated shell and which checkout/profile that shell came from. Its diff --git a/tests/pm/test_runtime_selection.py b/tests/pm/test_runtime_selection.py index 7edbcaaef4..ffc72b5de8 100644 --- a/tests/pm/test_runtime_selection.py +++ b/tests/pm/test_runtime_selection.py @@ -110,6 +110,49 @@ def test_manual_repair_bypasses_damaged_generation_activation(tmp_path, monkeypa assert "hermes pm repair" in result.stdout +@pytest.mark.parametrize("interpreter", ["store", "venv"]) +@pytest.mark.parametrize("with_state", [True, False]) +def test_boot_never_activates_the_pre_pm_venv(tmp_path, monkeypatch, interpreter, with_state): + """Nothing committed must not mean "load the in-tree venv": it was built for another + interpreter, so PM's store Python lost every compiled module from it after an update.""" + import os + import subprocess + import sys + from pm import environments as runtime_paths + + base_python = getattr(sys, "_base_executable", sys.executable) + base_prefix = Path(sys.base_prefix).resolve() + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "home")) + # The store interpreter is PM's: a non-venv Python living under the runtime dir. + monkeypatch.setenv("HERMES_RUNTIME_DIR", str(base_prefix.parent)) + root = tmp_path / "repo" + legacy = root / "venv" + (legacy / "pyvenv.cfg").parent.mkdir(parents=True) + (legacy / "pyvenv.cfg").write_text("home = test\n") + runtime_paths.site_packages(legacy).mkdir(parents=True) + (runtime_paths.site_packages(legacy) / "legacy_only.py").write_text("") + if with_state: + runtime_paths.install_state_dir(root).mkdir(parents=True) + python = base_python + if interpreter == "venv": + subprocess.run([base_python, "-m", "venv", "--without-pip", str(tmp_path / "dev")], check=True, timeout=60) + python = str(runtime_paths.venv_python(tmp_path / "dev")) + repo = Path(__file__).resolve().parents[2] + code = ( + "import sys, importlib.util; from pathlib import Path; sys.path.insert(0, sys.argv[1]); " + "from pm.environments import activate_dependencies\n" + "try:\n activate_dependencies(Path(sys.argv[2]))\n" + "except RuntimeError as exc:\n print('refused:', exc); raise SystemExit(0)\n" + "print('legacy importable:', importlib.util.find_spec('legacy_only') is not None)" + ) + result = subprocess.run([python, "-I", "-c", code, str(repo), str(root)], env=dict(os.environ), + capture_output=True, text=True, timeout=30) + assert result.returncode == 0, result.stderr + expected = ("refused: no dependency environment is committed" if interpreter == "store" + else "legacy importable: False") + assert result.stdout.strip().startswith(expected), result.stdout + + @pytest.mark.parametrize("data", [[], {"packages": []}, {"packages": {"venv": []}}]) def test_malformed_selection_has_actionable_error(tmp_path, monkeypatch, data): from pm import environments as runtime_paths diff --git a/tests/pm/test_source_update_launch.py b/tests/pm/test_source_update_launch.py index 66fc59e566..7f8ecb0386 100644 --- a/tests/pm/test_source_update_launch.py +++ b/tests/pm/test_source_update_launch.py @@ -268,6 +268,31 @@ def test_launch_without_marker_publishes_then_skips_and_rebuilds_on_lock_change( assert not (root / ".update-incomplete").exists() +@pytest.mark.platforms("posix") +def test_process_spawned_by_the_update_commits_dependencies_but_not_the_tail(source_launch, tmp_path): + """A process an update spawns before its dependencies are current (its restarted gateway) + must not boot on a tree built for another interpreter; it syncs, but leaves the tail alone.""" + import time + from hermes_cli.update_lock import update_marker_path + from pm.environments import committed_venv + + root, store_python, _ = source_launch + marker = update_marker_path() + marker.parent.mkdir(parents=True, exist_ok=True) + marker.write_text(f"{os.getppid()}\n{int(time.time())}\n", encoding="utf-8") # the updater is our ancestor + assert committed_venv(root) is None + + assert venv_sync.prepare_launch(root, []) == store_python + assert committed_venv(root) == Path(_fact(root)["environment"]) + assert not (tmp_path / "completion-calls").exists(), "the tail is the updater's, not its child's" + assert not venv_sync.completion_pending_path(root).exists() + + facts_bytes, receipts = runtime_facts_path(root).read_bytes(), _receipts(tmp_path) + venv_sync.prepare_launch(root, []) + assert runtime_facts_path(root).read_bytes() == facts_bytes + assert _receipts(tmp_path) == receipts, "a committed child synced again under the updater's claim" + + @pytest.mark.platforms("posix") def test_failed_real_sync_preserves_previous_selection_and_retries(source_launch, tmp_path): root, store_python, _ = source_launch