fix(pm): skip the tool re-hash during shell activation
source ./activate and . .\activate.ps1 always run setup, and setup always runs `python -m pm.cli install`. An explicit install re-hashes every published tool entry so it can repair a corrupted one. On this machine that hash reads about 550 MiB and takes 6.0 of the 6.6 seconds, even when nothing changed. Shell activation now passes --trust-recorded and trusts the digest the install recorded, the same check startup already uses. A missing tool is still installed and a stale venv is still rebuilt. `hermes pm install` and `hermes update` keep the byte check, and the flag refuses names, --extra, and --target so it cannot narrow an install someone asked for by name. Measured on this machine, already up to date: the activation path drops from 6.6 s to 0.9 s. A bare `python -m pm.cli install` stays at 7.2 s. The remaining 0.9 s is process startup and imports, not hashing.
This commit is contained in:
4
activate
4
activate
@@ -13,7 +13,9 @@
|
||||
# keeps its own command. A prompt prefix names the worktree.
|
||||
#
|
||||
# Sync through setup before selecting the environment. PM owns freshness;
|
||||
# activation does not maintain a second dependency stamp.
|
||||
# activation does not maintain a second dependency stamp. It trusts the
|
||||
# recorded tool digest instead of re-hashing every entry. `hermes pm
|
||||
# install` and `hermes update` keep that check.
|
||||
# ============================================================================
|
||||
_HERMES_REPO="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
|
||||
# Setup runs in a child: failures must not exit or partially activate the
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
# Source this file to sync and apply the PM environment; deactivate restores it.
|
||||
# Trusts the recorded tool digest. `hermes pm install` re-checks the bytes.
|
||||
$ErrorActionPreference = 'Stop'
|
||||
|
||||
$OutputEncoding = [System.Console]::OutputEncoding = [System.Console]::InputEncoding = [System.Text.Encoding]::UTF8
|
||||
|
||||
13
pm/cli.py
13
pm/cli.py
@@ -124,7 +124,7 @@ def _live_progress(name: str):
|
||||
return report
|
||||
|
||||
|
||||
def _install_names(names: list[str], target: str | None = None) -> int:
|
||||
def _install_names(names: list[str], target: str | None = None, *, verify: bool = True) -> int:
|
||||
from pm.install import _install_operation
|
||||
|
||||
failed = 0
|
||||
@@ -137,7 +137,7 @@ def _install_names(names: list[str], target: str | None = None) -> int:
|
||||
entry = stage_only(name, target)
|
||||
print(f"✓ {name} (staged for {target}: {entry.name})")
|
||||
else:
|
||||
ensure(name, explicit=True, progress=progress, _operation=operation)
|
||||
ensure(name, explicit=True, verify=verify, progress=progress, _operation=operation)
|
||||
if name == "python":
|
||||
from hermes_cli.venv_sync import publish_launchers
|
||||
|
||||
@@ -166,9 +166,13 @@ def cmd_install(args) -> int:
|
||||
# Python remains optional when provisioning individual tools.
|
||||
extras = list(dict.fromkeys(getattr(args, "extra", None) or ()))
|
||||
tools_only = bool(getattr(args, "tools_only", False))
|
||||
trust_recorded = bool(getattr(args, "trust_recorded", False))
|
||||
if tools_only and (extras or cross_target or args.names):
|
||||
print("✗ --tools-only installs the tool closure and then stops; it does not take names, --extra, or --target")
|
||||
return 1
|
||||
if trust_recorded and (extras or cross_target or args.names):
|
||||
print("✗ --trust-recorded installs the default closure and then stops; it does not take names, --extra, or --target")
|
||||
return 1
|
||||
if extras and cross_target:
|
||||
print("✗ --extra syncs this install's venv and cannot combine with --target")
|
||||
return 1
|
||||
@@ -178,7 +182,7 @@ def cmd_install(args) -> int:
|
||||
# wheels) resolved compilers and git from the host PATH. Publish every
|
||||
# tool first and put it on PATH; sync only after that.
|
||||
tool_names = names if args.names else tool_roots(names)
|
||||
failed = _install_names(tool_names, target=cross_target)
|
||||
failed = _install_names(tool_names, target=cross_target, verify=not trust_recorded)
|
||||
if failed:
|
||||
return 1
|
||||
if not cross_target and (not args.names or tools_only):
|
||||
@@ -571,6 +575,9 @@ def main(argv=None) -> int:
|
||||
help="enable a declared dependency extra in the venv (repeatable)")
|
||||
p.add_argument("--tools-only", action="store_true",
|
||||
help="install the tool closure, put it on PATH, and stop before the venv sync")
|
||||
p.add_argument("--trust-recorded", action="store_true",
|
||||
help="trust the recorded tool digest instead of re-hashing every entry. "
|
||||
"shell activation only. a deliberate install re-checks the bytes")
|
||||
p.add_argument(
|
||||
"--target",
|
||||
help="stage for a cross target (e.g. linux-arm64-bionic on a glibc "
|
||||
|
||||
@@ -422,6 +422,7 @@ def ensure(
|
||||
*,
|
||||
base_env: Optional[dict] = None,
|
||||
explicit: bool = False,
|
||||
verify: bool = True,
|
||||
progress=None,
|
||||
pause_event: threading.Event | None = None,
|
||||
download_progress: ProgressFn | None = None,
|
||||
@@ -431,6 +432,11 @@ def ensure(
|
||||
install`, `hermes pm bundle`) — those ARE the remedy the lazy-install
|
||||
policy names, so the policy does not apply to them.
|
||||
|
||||
``verify`` re-hashes an already-recorded entry and repairs it when the
|
||||
bytes moved. A deliberate install keeps that check. Shell activation
|
||||
passes ``False``. It trusts the recorded digest, the same check startup
|
||||
uses, because hashing every tool tree costs seconds per shell.
|
||||
|
||||
``progress(stage, done, total, label)`` reports the slow parts of an
|
||||
install to a UI, including ordered multi-archive labels.
|
||||
"""
|
||||
@@ -454,7 +460,7 @@ def ensure(
|
||||
json.dumps(_identity(lockfile, package.name, target), sort_keys=True))
|
||||
if identity in checked:
|
||||
continue
|
||||
if _installed_location(package, lockfile, target, verify=explicit) is None:
|
||||
if _installed_location(package, lockfile, target, verify=explicit and verify) is None:
|
||||
missing.append(package)
|
||||
else:
|
||||
checked.add(identity)
|
||||
|
||||
@@ -88,7 +88,7 @@ try {
|
||||
# The closure pm install would provision, minus the venv. A bare
|
||||
# `pm install` also syncs the venv, and that sync must not run until the
|
||||
# compiler environment below is on PATH.
|
||||
& $bootPy.Trim() -m pm.cli install --tools-only
|
||||
& $bootPy.Trim() -m pm.cli install --tools-only $(if ($RuntimeOnly) { '--trust-recorded' })
|
||||
if ($LASTEXITCODE -ne 0) { throw 'pm tool install failed - see output above.' }
|
||||
} finally {
|
||||
Pop-Location
|
||||
@@ -102,12 +102,13 @@ if ($arch -eq 'arm64') {
|
||||
|
||||
# The venv sync. Tools are already on PATH inside that process (pm install
|
||||
# publishes them before syncing); the compiler env set above is inherited.
|
||||
# Activation trusts the recorded tool digest. A direct setup re-checks it.
|
||||
# ---------------------------------------------------------------------------
|
||||
Write-Host 'Installing dependencies via pm (hash-verified via uv.lock)...' -ForegroundColor Cyan
|
||||
Write-Host '(first run on a fresh checkout can take 1-5 minutes)'
|
||||
Push-Location $repo
|
||||
try {
|
||||
& $bootPy.Trim() -m pm.cli install
|
||||
& $bootPy.Trim() -m pm.cli install $(if ($RuntimeOnly) { '--trust-recorded' })
|
||||
if ($LASTEXITCODE -ne 0) { throw 'pm install failed - see output above.' }
|
||||
} finally {
|
||||
Pop-Location
|
||||
|
||||
@@ -164,7 +164,7 @@ echo -e "${CYAN}→${NC} (first run on a fresh checkout can take 1-5 minutes)"
|
||||
"$uv" python install --no-bin "$py_version"
|
||||
boot_py="$("$uv" python find --managed-python "$py_version")"
|
||||
boot_py="${boot_py%$'\r'}"
|
||||
if ! "$boot_py" -m pm.cli install; then
|
||||
if ! "$boot_py" -m pm.cli install ${runtime_only:+--trust-recorded}; then
|
||||
echo -e "${RED}✗${NC} pm install failed — see output above."
|
||||
exit 1
|
||||
fi
|
||||
|
||||
@@ -32,7 +32,7 @@ def _sync_checkout(tmp_path: Path):
|
||||
with (root / "calls.jsonl").open("a", encoding="utf-8") as stream:
|
||||
stream.write(json.dumps(record) + "\\n")
|
||||
assert all(value is None for value in record["python_env"].values()), record
|
||||
assert sys.argv[1:] == ["runtime-only"], record
|
||||
assert sys.argv[1:] == ["runtime-only", "--trust-recorded"], record
|
||||
print("setup progress")
|
||||
if (root / "fail").exists():
|
||||
sys.exit(42)
|
||||
@@ -61,9 +61,9 @@ def _sync_checkout(tmp_path: Path):
|
||||
binary.write_text(f"#!/bin/sh\nexec {shlex.quote(str(python))} \"$@\"\n", encoding="utf-8")
|
||||
binary.chmod(0o755)
|
||||
(root / "setup-hermes.sh").write_text(
|
||||
'test "$#" = 1 && test "$1" = --runtime-only || exit 2\n'
|
||||
'test "$#" = 2 && test "$1" = --runtime-only && test "$2" = --trust-recorded || exit 2\n'
|
||||
f'cd {shlex.quote(str(root))} || exit 3\n'
|
||||
f'exec {shlex.quote(str(python))} sync.py runtime-only\n', encoding="utf-8",
|
||||
f'exec {shlex.quote(str(python))} sync.py runtime-only --trust-recorded\n', encoding="utf-8",
|
||||
)
|
||||
(root / "setup-hermes.ps1").write_text(
|
||||
"param([switch]$RuntimeOnly)\n"
|
||||
@@ -73,14 +73,14 @@ def _sync_checkout(tmp_path: Path):
|
||||
"argv=[Environment]::GetCommandLineArgs()}\n"
|
||||
"$record | ConvertTo-Json -Compress | Add-Content -LiteralPath \"$PSScriptRoot\\ps-calls.jsonl\"\n"
|
||||
"Set-Location -LiteralPath $PSScriptRoot\n"
|
||||
f"& '{python}' sync.py runtime-only\nexit $LASTEXITCODE\n", encoding="utf-8",
|
||||
f"& '{python}' sync.py runtime-only --trust-recorded\nexit $LASTEXITCODE\n", encoding="utf-8",
|
||||
)
|
||||
return root, env
|
||||
|
||||
|
||||
def _assert_syncs(root: Path):
|
||||
calls = [json.loads(line) for line in (root / "calls.jsonl").read_text(encoding="utf-8").splitlines()]
|
||||
assert calls == [{"argv": ["runtime-only"], "python_env": {
|
||||
assert calls == [{"argv": ["runtime-only", "--trust-recorded"], "python_env": {
|
||||
"PYTHONHOME": None, "PYTHONPATH": None, "VIRTUAL_ENV": None,
|
||||
}}] * 3
|
||||
assert (root / "builds").read_text(encoding="utf-8").splitlines() == ["first", "second"]
|
||||
|
||||
@@ -23,10 +23,11 @@ import pm.cli
|
||||
|
||||
@pytest.fixture()
|
||||
def install_spy(monkeypatch):
|
||||
calls = {"names": None, "sync_extras": None, "activated": []}
|
||||
calls = {"names": None, "verified": None, "sync_extras": None, "activated": []}
|
||||
|
||||
def fake_install_names(names, target=None):
|
||||
def fake_install_names(names, target=None, *, verify=True):
|
||||
calls["names"] = list(names)
|
||||
calls["verified"] = verify
|
||||
return 0
|
||||
|
||||
def fake_sync_venv(extras=None, **kwargs):
|
||||
@@ -68,6 +69,29 @@ def test_tools_only_publishes_tools_and_stops_before_the_venv(install_spy):
|
||||
assert install_spy["activated"] == [{"allow_incomplete": True}]
|
||||
|
||||
|
||||
def test_trust_recorded_skips_the_byte_check_and_still_syncs(install_spy):
|
||||
assert pm.cli.cmd_install(argparse.Namespace(
|
||||
names=None, extra=[], target=None, tools_only=False, trust_recorded=True)) == 0
|
||||
assert "python" in install_spy["names"]
|
||||
assert install_spy["verified"] is False
|
||||
assert install_spy["sync_extras"] == ["all"]
|
||||
|
||||
|
||||
@pytest.mark.parametrize("kwargs, message", [
|
||||
({"names": ["ripgrep"], "extra": [], "target": None, "tools_only": False, "trust_recorded": True},
|
||||
"--trust-recorded"),
|
||||
({"names": None, "extra": ["dev"], "target": None, "tools_only": False, "trust_recorded": True},
|
||||
"--trust-recorded"),
|
||||
({"names": None, "extra": [], "target": "linux-x64", "tools_only": False, "trust_recorded": True},
|
||||
"--target"),
|
||||
])
|
||||
def test_trust_recorded_refuses_a_narrowed_install(install_spy, capsys, kwargs, message):
|
||||
assert pm.cli.cmd_install(argparse.Namespace(**kwargs)) == 1
|
||||
assert message in capsys.readouterr().out
|
||||
assert install_spy["names"] is None
|
||||
assert install_spy["sync_extras"] is None
|
||||
|
||||
|
||||
def test_a_missing_tool_blocks_the_venv_sync(install_spy, monkeypatch, capsys):
|
||||
monkeypatch.setattr(
|
||||
importlib.import_module("pm.install"), "activate",
|
||||
|
||||
@@ -14,7 +14,7 @@ def test_hint_names_a_command_the_cli_accepts(monkeypatch):
|
||||
monkeypatch.setattr(runtime, "is_runtime", lambda: True)
|
||||
monkeypatch.setattr(install_mod, "sync_venv", lambda extras, **kwargs: synced.append((list(extras), kwargs)))
|
||||
monkeypatch.setattr(cli, "_install_names",
|
||||
lambda names, target=None: 0 if not names else pytest.fail(f"tools installed: {names}"))
|
||||
lambda names, target=None, **kwargs: 0 if not names else pytest.fail(f"tools installed: {names}"))
|
||||
monkeypatch.setattr(install_mod, "activate", lambda **kwargs: [])
|
||||
|
||||
argv = install_hint("anthropic").split()[2:]
|
||||
@@ -29,7 +29,7 @@ def test_extra_syncs_only_the_named_extras(monkeypatch):
|
||||
synced = []
|
||||
monkeypatch.setattr(install_mod, "sync_venv", lambda extras, **kwargs: synced.append(list(extras)))
|
||||
monkeypatch.setattr(install_mod, "activate", lambda **kwargs: [])
|
||||
monkeypatch.setattr(cli, "_install_names", lambda names, target=None: 0)
|
||||
monkeypatch.setattr(cli, "_install_names", lambda names, target=None, **kwargs: 0)
|
||||
assert cli.cmd_install(SimpleNamespace(names=[], extra=["otlp", "mcp", "otlp"], target=None, tools_only=False)) == 0
|
||||
assert synced == [["otlp", "mcp"]]
|
||||
|
||||
|
||||
@@ -185,6 +185,44 @@ def test_deps_compose_dependents_win(pm_env):
|
||||
assert path.index("toptool-1.0") < path.index("deptool-1.0")
|
||||
|
||||
|
||||
def test_activation_trusts_a_recorded_entry_a_deliberate_install_repairs(pm_env, monkeypatch):
|
||||
"""Shell activation skips the byte re-hash; a deliberate install keeps it.
|
||||
|
||||
Hashing every published tree costs seconds per shell, so activation trusts
|
||||
the digest the install recorded. That trust must not leak: the same
|
||||
corruption, installed without the flag, is still detected and rewritten.
|
||||
"""
|
||||
import importlib
|
||||
from pm.cli import _install_names
|
||||
|
||||
ensure = importlib.import_module("pm.install")
|
||||
lockfile_path, runtime, docroot, _ = pm_env
|
||||
_, digest = make_tar(docroot, "faketool-1.0.tar.gz", {"bin/faketool": "good"})
|
||||
_pin(lockfile_path, "faketool", "1.0", digest)
|
||||
assert _install_names(["faketool"]) == 0
|
||||
|
||||
fact = Facts(runtime / "facts.json").get("faketool")
|
||||
assert fact is not None
|
||||
binary = runtime / fact["entry"] / "bin/faketool"
|
||||
binary.write_text("corrupt", encoding="utf-8")
|
||||
|
||||
checked = []
|
||||
original = ensure._entry_verified
|
||||
|
||||
def verify(package, fact, store, target):
|
||||
checked.append(package.name)
|
||||
return original(package, fact, store, target)
|
||||
|
||||
monkeypatch.setattr(ensure, "_entry_verified", verify)
|
||||
assert _install_names(["faketool"], verify=False) == 0
|
||||
assert checked == []
|
||||
assert binary.read_text(encoding="utf-8") == "corrupt"
|
||||
|
||||
assert _install_names(["faketool"]) == 0
|
||||
assert "faketool" in checked
|
||||
assert binary.read_text(encoding="utf-8") == "good"
|
||||
|
||||
|
||||
def test_warm_install_verifies_shared_dependencies_once_under_lock(pm_env, monkeypatch):
|
||||
import importlib
|
||||
import os
|
||||
|
||||
@@ -44,7 +44,7 @@ def test_setup_reads_pins_independent_of_indentation(tmp_path, served, indent, b
|
||||
(core / "pm" / "__init__.py").touch()
|
||||
(core / "pm" / "cli.py").write_text(
|
||||
"import json, pathlib, sys\n"
|
||||
"assert sys.argv[1:] == ['install'], sys.argv\n"
|
||||
"assert sys.argv[1:] == ['install', '--trust-recorded'], sys.argv\n"
|
||||
f"pathlib.Path({str(receipt)!r}).write_text(json.dumps(sys.argv[1:]))\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
@@ -89,7 +89,7 @@ def test_setup_reads_pins_independent_of_indentation(tmp_path, served, indent, b
|
||||
)
|
||||
assert result.returncode == 0, result.stdout + result.stderr
|
||||
assert (runtime / f"uv-{uv_version}-{target}" / "uv").read_text() == uv_script
|
||||
assert json.loads(receipt.read_text()) == ["install"]
|
||||
assert json.loads(receipt.read_text()) == ["install", "--trust-recorded"]
|
||||
assert calls.read_text().splitlines() == [
|
||||
"--version", f"python install --no-bin {py_version}",
|
||||
f"python find --managed-python {py_version}",
|
||||
|
||||
@@ -319,9 +319,11 @@ The leading dot and space in PowerShell are required. Executing
|
||||
The POSIX script uses Bash syntax. Use Bash for this recipe rather than `sh`,
|
||||
fish, or assuming that a Zsh startup file has Bash semantics.
|
||||
|
||||
Each activation invokes PM's install/sync path. PM reuses current tools and
|
||||
dependency generations; missing or stale inputs can require downloads and a
|
||||
rebuild. A setup failure returns an error before changing the activated shell
|
||||
Each activation invokes PM's install/sync path and trusts the recorded tool
|
||||
digest instead of re-hashing every entry. PM still installs a missing tool and
|
||||
rebuilds a stale dependency generation; a deliberate install keeps the byte
|
||||
check. Run `python -m pm.cli install` or `hermes update` to re-check realized
|
||||
bytes. A setup failure returns an error before changing the activated shell
|
||||
environment, including when re-sourcing an already active environment.
|
||||
|
||||
After sync, activation prepends installed PM tools to `PATH` and sets
|
||||
|
||||
Reference in New Issue
Block a user