Skip to content

fix(lazy-deps): skip the install ladder on package-manager installs (salvage #48637) - #77500

Merged
kshitijk4poor merged 1 commit into
NousResearch:mainfrom
kshitijk4poor:salvage-48637-managed-guard
Aug 3, 2026
Merged

fix(lazy-deps): skip the install ladder on package-manager installs (salvage #48637)#77500
kshitijk4poor merged 1 commit into
NousResearch:mainfrom
kshitijk4poor:salvage-48637-managed-guard

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Salvages #48637 by @liuhao1024 — retargeted onto current main with authorship preserved (the PR's base predates today's ensure() by ~8,100 commits, so a cherry-pick was not viable; the change was reapplied at the correct site).

Fixes #48628.

Context — what this fixes, for whom

NixOS users (and anyone running a package-manager build of Hermes): when a lazy feature's deps are missing, ensure() walks a uv → pip → ensurepip ladder. On these installs the venv's site-packages lives in the read-only store, so the ladder spends ~15 seconds bootstrapping ensurepip and then fails against a target it could never write to. nix/packages.nix already acknowledges this ("lazy-install can't write to the read-only /nix/store"), and nix/nixosModules.nix sets HERMES_MANAGED=true for the native service — the information to fail fast was there, unused.

Now it fails immediately with a message that points at the system package manager.

Two corrections to the PR as authored (found in review)

  1. Durable-target regression (High). The Dockerfile sets HERMES_LAZY_INSTALL_TARGET=/opt/data/lazy-packages (a writable volume) and the NixOS container module runs with HERMES_MANAGED=true. The original guard sat before that check and would have unconditionally blocked installs the durable-target path legitimately satisfies — breaking the NixOS-container deployment. The guard is now gated on _lazy_install_target() is None.
  2. hermes update misreport (Medium). refresh_active_features classifies FeatureUnavailable by a "unsupported " prefix on the reason. The original wording ("cannot install on NixOS system; ...") was reported to users as a hard failure; the reason now starts with "unsupported " so it is reported as a skip. There is a test pinning that contract directly.

Also simplified: one get_managed_system() call replaces the original is_managed() + get_managed_system() pair, and the except FeatureUnavailable: raise re-raise dance is gone (it existed only because the original raised inside its own try block).

Placement

After _unsupported_feature_reason, before spec validation. A platform-specific reason is more actionable than "managed install" (e.g. Matrix/python-olm on Windows tells you to use WSL), and refresh_active_features pre-checks that same function before calling ensure() — a managed-first order would make the two paths disagree.

Verification

  • tests/tools/test_lazy_deps*.py (3 files): 75 passed, 1 skipped on current main
  • 6 guard tests incl. regression coverage for both corrections above (durable-target override, platform-precedence, the skipped-vs-failed wording contract) plus fail-open on unreadable config
  • Mutation check: revert tools/lazy_deps.py to main → 2 guard tests fail; restore → 6 pass
  • ruff clean
  • Note for @liuhao1024: the original PR's Homebrew case is now moot — _IGNORED_MANAGED_VALUES explicitly ignores brew/homebrew on current main, so that test was dropped.

Closes #48637 (superseded by this salvage — original author credited via commit authorship).

Salvage of NousResearch#48637 (Fixes NousResearch#48628). On a NixOS-style install the venv's
site-packages lives in the read-only store, so ensure()'s
uv -> pip -> ensurepip ladder spends ~15s bootstrapping ensurepip only
to fail against a target it can never write. Fail fast with an
actionable message pointing at the system package manager.

Retargeted onto current main (the PR's base predates the durable-target
subsystem by ~8.1K commits) with two corrections to the original:

- Gate on _lazy_install_target() is None. The container deployment sets
  HERMES_MANAGED=true AND HERMES_LAZY_INSTALL_TARGET (a writable
  volume); the original guard would have blocked installs that path
  legitimately satisfies, breaking the NixOS-container mode.
- Reason string starts with 'unsupported ' because
  refresh_active_features classifies FeatureUnavailable by that prefix;
  the original wording made 'hermes update' report a hard failure
  instead of a skip.

Placed after _unsupported_feature_reason so a platform-specific reason
(more actionable) wins, and so ensure() agrees with
refresh_active_features, which pre-checks that same function.
@kshitijk4poor
kshitijk4poor enabled auto-merge (rebase) August 3, 2026 08:34
@kshitijk4poor
kshitijk4poor merged commit f07f47f into NousResearch:main Aug 3, 2026
35 checks passed
@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/tools Tool registry, model_tools, toolsets area/nix Nix flake, NixOS module, container packaging labels Aug 3, 2026
@kshitijk4poor
kshitijk4poor deleted the salvage-48637-managed-guard branch August 5, 2026 07:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/nix Nix flake, NixOS module, container packaging comp/tools Tool registry, model_tools, toolsets P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf: lazy_deps runs the ensurepip→pip bootstrap on every launch for managed/read-only installs (NixOS) — ~20s CPU/start, never persists

3 participants