fix(pm): put tools on PATH before the venv sync
Windows PowerShell 5.1 returns every match from Get-Command. .Source on that array joins the paths with a space, and the call operator then treats the joined string as one program name. Git for Windows ships git.exe in cmd\ and bin\, so setup died with CommandNotFoundException before pm install ran. setup-hermes.ps1 now installs the tool closure first (`pm install --tools-only`), then prepares the ARM64 compiler environment, then syncs the venv. The sync inherits that compiler environment. A bare `pm install` and the update takeover path publish tools and put them on PATH before uv sync. A missing tool stops the sync. A missing venv does not. Verified: scripts/run_tests.sh on test_install_default_closure.py, test_install_extra.py, and test_windows_build_deps.py — 13 passed.
This commit is contained in:
@@ -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
|
||||
|
||||
26
pm/cli.py
26
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 "
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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.
|
||||
|
||||
|
||||
@@ -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'
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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"]]
|
||||
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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. |
|
||||
|
||||
Reference in New Issue
Block a user