diff --git a/hermes_cli/_update_takeover.py b/hermes_cli/_update_takeover.py index dcb1d49489..491bb2c278 100644 --- a/hermes_cli/_update_takeover.py +++ b/hermes_cli/_update_takeover.py @@ -20,7 +20,7 @@ def prepare(request: dict) -> tuple[Path, dict[str, str]]: from pm import paths, receipt from pm.client import ensure, sync_venv, venv_is_current from pm.lock import Lockfile - from pm.registry import source_install_packages + from pm.registry import tool_roots from pm.environments import activation_environment, install_state_dir, runtime_facts_path from hermes_cli._launchers import resolve_store_python from hermes_cli.venv_sync import publish_launchers @@ -30,8 +30,13 @@ def prepare(request: dict) -> tuple[Path, dict[str, str]]: lock = Lockfile(paths.lockfile_path()) # A pre-PM installation has no required-tool facts. A current Python # generation alone does not prove its Node/Git/tool closure is ready. - for name in source_install_packages(lock.names()): + for name in tool_roots(lock.names()): ensure(name, explicit=True) + from pm.install import activate + + problems = [problem for problem in activate(allow_incomplete=True) if not problem.startswith("venv:")] + if problems: + raise RuntimeError(f"tools not on PATH before venv sync: {'; '.join(problems)}") extras = ["all"] if not runtime_facts_path(root).is_file() else None repair_marker = install_state_dir(root) / ".repair-incomplete" # Repair preserves the old stamp. Changed source inputs instead need diff --git a/pm/cli.py b/pm/cli.py index 4011ff8c2d..62ac205028 100644 --- a/pm/cli.py +++ b/pm/cli.py @@ -15,7 +15,7 @@ from pm.install import _facts, _lockfile, _store, ensure, stage_only from pm.operations import lock_project from pm.package import InstallError from pm.paths import repo_root -from pm.registry import get_package, source_install_packages +from pm.registry import get_package, source_install_packages, tool_roots from pm.store import ALL_TARGETS, current_target, hash_url from pm.update import Resolved, resolve_package, reuse_index_responses @@ -165,11 +165,31 @@ def cmd_install(args) -> int: # Source-install launchers require the store interpreter, even though # Python remains optional when provisioning individual tools. extras = list(dict.fromkeys(getattr(args, "extra", None) or ())) + tools_only = bool(getattr(args, "tools_only", 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 extras and cross_target: print("✗ --extra syncs this install's venv and cannot combine with --target") return 1 names = args.names if args.names or extras else source_install_packages(_lockfile().names()) - failed = _install_names(names, target=cross_target) + # Tools before the venv. A bare `pm install` used to install tools and + # sync the venv in one breath, so a native build (Windows ARM64 source + # 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) + if failed: + return 1 + if not cross_target and (not args.names or tools_only): + from pm.install import activate + + problems = activate(allow_incomplete=True) + if problems: + print(f"✗ tools not on PATH before venv sync: {'; '.join(problems)}", flush=True) + return 1 + if tools_only: + return 0 if extras or not args.names: from pm.install import sync_venv @@ -549,6 +569,8 @@ def main(argv=None) -> int: p.add_argument("names", nargs="*") p.add_argument("--extra", action="append", default=[], metavar="NAME", 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( "--target", help="stage for a cross target (e.g. linux-arm64-bionic on a glibc " diff --git a/pm/install.py b/pm/install.py index 1b4f26307b..ae527a5dab 100644 --- a/pm/install.py +++ b/pm/install.py @@ -741,7 +741,7 @@ def _store_path_dirs() -> list[str]: return dirs -def activate() -> list[str]: +def activate(*, allow_incomplete: bool = False) -> list[str]: """Make the installed store usable: prepend its tool dirs to os.environ['PATH'] so reactive `shutil.which('git'|'bash'|'ffmpeg'|...)` resolves the bundled binaries. The gate is `check()` — if the store is @@ -749,6 +749,10 @@ def activate() -> list[str]: Return the check's problems, or an empty list on success, so startup callers can report the verdict without checking the store twice. + ``allow_incomplete`` is the install-time exception: tools are published + before the venv sync, so a missing venv must not hide the tools the sync + is about to build against. A missing tool still refuses. + This is the ONE sanctioned global PATH write: PATH is the discovery contract every `which` reads, not a tool-specific env leak. Store-first unconditionally — pinned bundled versions win on dev machines too. @@ -756,6 +760,8 @@ def activate() -> list[str]: import os problems = check() + if allow_incomplete: + problems = [problem for problem in problems if not problem.startswith("venv:")] if problems: return problems dirs = _store_path_dirs() diff --git a/pm/registry.py b/pm/registry.py index ff1384bd6c..11df8101e1 100644 --- a/pm/registry.py +++ b/pm/registry.py @@ -9,7 +9,7 @@ import sys from types import ModuleType from typing import Any -from pm.package import InstallError, Package +from pm.package import InstallError, Package, StatePackage _packages: dict[str, Package] = {} @@ -56,6 +56,11 @@ def source_install_packages(names: list[str]) -> list[str]: if not get_package(name).internal and (name == "python" or not get_package(name).optional)] +def tool_roots(names: list[str]) -> list[str]: + """Packages to publish before a venv sync. The venv is not one of them.""" + return [name for name in source_install_packages(names) if not isinstance(get_package(name), StatePackage)] + + def package_definitions(names: list[str] | None = None) -> list[dict[str, Any]]: """Declarations for a fresh worker; built-ins already load with pm. diff --git a/scripts/windows-build-deps.ps1 b/scripts/windows-build-deps.ps1 index 51d3385347..a06f6c9a4f 100644 --- a/scripts/windows-build-deps.ps1 +++ b/scripts/windows-build-deps.ps1 @@ -1,7 +1,14 @@ # Native build dependencies are separate from PM's application environment. function Invoke-HermesBuildCommand { param([string]$Command, [string[]]$Arguments) - $executable = (Get-Command $Command -CommandType Application -ErrorAction Stop).Source + # Windows PowerShell 5.1 returns every match from Get-Command even without + # -All. .Source on that array is every path joined by a space, and the call + # operator then treats the joined string as one program name. Git for + # Windows puts git.exe in both cmd\ and bin\, so a bare lookup is that bug. + $executable = @(Get-Command $Command -CommandType Application -ErrorAction Stop | Select-Object -First 1)[0].Source + if ($executable -isnot [string] -or -not (Test-Path -LiteralPath $executable -PathType Leaf)) { + throw "Could not resolve a single executable for $Command (got: $executable)" + } $previousPreference = $ErrorActionPreference try { $ErrorActionPreference = 'Continue' diff --git a/setup-hermes.ps1 b/setup-hermes.ps1 index 57f544f642..2f2da3618d 100644 --- a/setup-hermes.ps1 +++ b/setup-hermes.ps1 @@ -70,17 +70,14 @@ if (Test-Path $uv) { } # --------------------------------------------------------------------------- -# ARM64 source wheels need the native compiler and OpenSSL development libraries. -# Activation runs setup in a child, so these build variables do not leak into its caller. -if ($arch -eq 'arm64') { - . (Join-Path $repo 'scripts\windows-build-deps.ps1') - Initialize-HermesArm64BuildTools -StateRoot (Split-Path $store -Parent) -} - -# Delegate to pm: python + venv + tool store + hash-verified venv sync +# Tools first, then the compiler environment, then the venv sync. +# ARM64 source wheels need the native compiler and OpenSSL development +# libraries, and that setup shells out to git. The git it finds must be the +# one pm just installed, not a host install whose PATH has two git.exe files +# (cmd\ and bin\). Activation runs setup in a child, so these build variables +# do not leak into its caller. # --------------------------------------------------------------------------- -Write-Host 'Installing python + tools + dependencies via pm (hash-verified via uv.lock)...' -ForegroundColor Cyan -Write-Host '(first run on a fresh checkout can take 1-5 minutes)' +Write-Host 'Installing python + tools via pm...' -ForegroundColor Cyan Push-Location $repo try { # PM can replace its uv entry only after the bootstrap uv has exited. @@ -88,6 +85,28 @@ try { if ($LASTEXITCODE -ne 0) { throw 'bootstrap Python installation failed' } $bootPy = (& $uv python find --managed-python $pyVersion) -join "`n" if ($LASTEXITCODE -ne 0 -or -not $bootPy) { throw 'bootstrap Python lookup failed' } + # 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 + if ($LASTEXITCODE -ne 0) { throw 'pm tool install failed - see output above.' } +} finally { + Pop-Location +} +Write-Host 'Tools installed' -ForegroundColor Green + +if ($arch -eq 'arm64') { + . (Join-Path $repo 'scripts\windows-build-deps.ps1') + Initialize-HermesArm64BuildTools -StateRoot (Split-Path $store -Parent) +} + +# The venv sync. Tools are already on PATH inside that process (pm install +# publishes them before syncing); the compiler env set above is inherited. +# --------------------------------------------------------------------------- +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 if ($LASTEXITCODE -ne 0) { throw 'pm install failed - see output above.' } } finally { diff --git a/tests/pm/test_install_default_closure.py b/tests/pm/test_install_default_closure.py index b008b10c70..77f5466e16 100644 --- a/tests/pm/test_install_default_closure.py +++ b/tests/pm/test_install_default_closure.py @@ -23,7 +23,7 @@ import pm.cli @pytest.fixture() def install_spy(monkeypatch): - calls = {"names": None, "sync_extras": None} + calls = {"names": None, "sync_extras": None, "activated": []} def fake_install_names(names, target=None): calls["names"] = list(names) @@ -33,21 +33,46 @@ def install_spy(monkeypatch): calls["sync_extras"] = list(extras or []) return None + def fake_activate(**kwargs): + calls["activated"].append(kwargs) + return [] + monkeypatch.setattr(pm.cli, "_install_names", fake_install_names) monkeypatch.setattr(importlib.import_module("pm.install"), "sync_venv", fake_sync_venv) + monkeypatch.setattr(importlib.import_module("pm.install"), "activate", fake_activate) return calls def test_default_closure_includes_the_boot_interpreter(install_spy): - assert pm.cli.cmd_install(argparse.Namespace(names=None)) == 0 + assert pm.cli.cmd_install(argparse.Namespace(names=None, tools_only=False)) == 0 assert "python" in install_spy["names"] + assert "venv" not in install_spy["names"] assert all(not pm.cli.get_package(name).internal for name in install_spy["names"]) assert install_spy["sync_extras"] == ["all"] - + assert install_spy["activated"] == [{"allow_incomplete": True}] @pytest.mark.parametrize("names", [["npm", "ripgrep"], ["dmgbuild"]]) def test_explicit_names_pass_through_untouched(install_spy, names) -> None: - assert pm.cli.cmd_install(argparse.Namespace(names=names)) == 0 + assert pm.cli.cmd_install(argparse.Namespace(names=names, tools_only=False)) == 0 assert install_spy["names"] == names assert install_spy["sync_extras"] is None + assert install_spy["activated"] == [] + + +def test_tools_only_publishes_tools_and_stops_before_the_venv(install_spy): + assert pm.cli.cmd_install(argparse.Namespace(names=None, extra=[], target=None, tools_only=True)) == 0 + assert "python" in install_spy["names"] + assert "venv" not in install_spy["names"] + assert install_spy["sync_extras"] is None + assert install_spy["activated"] == [{"allow_incomplete": True}] + + +def test_a_missing_tool_blocks_the_venv_sync(install_spy, monkeypatch, capsys): + monkeypatch.setattr( + importlib.import_module("pm.install"), "activate", + lambda **kwargs: ["git: not installed or outdated"], + ) + assert pm.cli.cmd_install(argparse.Namespace(names=None, tools_only=False)) == 1 + assert install_spy["sync_extras"] is None + assert "git: not installed or outdated" in capsys.readouterr().out diff --git a/tests/pm/test_install_extra.py b/tests/pm/test_install_extra.py index 980ff50232..4725fd652a 100644 --- a/tests/pm/test_install_extra.py +++ b/tests/pm/test_install_extra.py @@ -15,6 +15,7 @@ def test_hint_names_a_command_the_cli_accepts(monkeypatch): 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}")) + monkeypatch.setattr(install_mod, "activate", lambda **kwargs: []) argv = install_hint("anthropic").split()[2:] assert cli.main(argv) == 0 @@ -27,8 +28,9 @@ 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) - assert cli.cmd_install(SimpleNamespace(names=[], extra=["otlp", "mcp", "otlp"], target=None)) == 0 + assert cli.cmd_install(SimpleNamespace(names=[], extra=["otlp", "mcp", "otlp"], target=None, tools_only=False)) == 0 assert synced == [["otlp", "mcp"]] diff --git a/tests/pm/test_windows_build_deps.py b/tests/pm/test_windows_build_deps.py index 1f31dabbf1..4b35784ca6 100644 --- a/tests/pm/test_windows_build_deps.py +++ b/tests/pm/test_windows_build_deps.py @@ -300,6 +300,17 @@ Invoke-HermesBuildCommand $command @() $failed = $false try { Invoke-HermesBuildCommand (Join-Path $Root 'absent.exe') @() } catch { $failed = $true } if (-not $failed) { throw 'missing executable was accepted after a successful command' } +$a = Join-Path $Root 'cmd' +$b = Join-Path $Root 'bin' +New-Item -ItemType Directory -Force $a, $b | Out-Null +Set-Content -Path (Join-Path $a 'git.exe') -Value 'first' -Encoding ascii +Set-Content -Path (Join-Path $b 'git.exe') -Value 'second' -Encoding ascii +$env:PATH = "$a;$b;$env:PATH" +$resolved = @(Get-Command git -CommandType Application -ErrorAction Stop | Select-Object -First 1)[0].Source +if ($resolved -ne (Join-Path $a 'git.exe')) { throw "resolved every git.exe: $resolved" } +# The call operator must receive that one path, not both paths joined by a space. +$executable = $resolved +if ($executable -isnot [string] -or $executable.Contains(' ')) { throw "joined path leaked: $executable" } Write-Output 'PASS' ''', encoding="utf-8") env = dict(os.environ) diff --git a/website/docs/reference/package-management.md b/website/docs/reference/package-management.md index 0f3e08db10..e911be4926 100644 --- a/website/docs/reference/package-management.md +++ b/website/docs/reference/package-management.md @@ -492,7 +492,8 @@ hermes pm install chromium | Command | Effect | |---|---| -| `pm install [names...]` | Install named packages. With no names, provision required tools plus Python and sync the `all` extra. | +| `pm install [names...]` | Install named packages. With no names, provision required tools plus Python, put those tools on PATH, and then sync the `all` extra. | +| `pm install --tools-only` | Install that tool closure and put it on PATH, then stop. The venv sync does not run. | | `pm env [names...]` | Print the composed environment of installed packages as JSON. It does not install missing packages. | | `pm doctor` | Check installed tool identities, files, and digests against the lock. | | `pm repair` | Rebuild the recorded Python dependency set in a new generation, validate it, then select it. Does not update pins, features, or plugin configuration. |