Skip to content

refactor: share manager preference parsing and external tool lookup - #3143

Open
myuron wants to merge 2 commits into
voidzero-dev:mainfrom
myuron:feat/env-manager-runtime-override
Open

myuron wants to merge 2 commits into
voidzero-dev:mainfrom
myuron:feat/env-manager-runtime-override

Conversation

@myuron

@myuron myuron commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Preparatory refactor for #1899 (system-only management mode for NixOS). No behavior change: existing tests and snapshots are unchanged.

The follow-up PRs need two things that currently live inside the installer and the shim dispatcher. This PR moves them to shared places.

What's Changed

Manager preferences in EnvConfig

  • Add VP_NODE_MANAGER, VP_PM_MANAGER, VP_NPM_MANAGER, VP_PNPM_MANAGER, VP_YARN_MANAGER, and VP_BUN_MANAGER to vp_shared::env_vars.
  • Add ManagerPreference { Yes, No } and six Option<ManagerPreference> fields to EnvConfig. Only the exact values yes and no are recognized, as before.
  • self_setup now converts these fields with shim_mode instead of reading the variables in manager_mode. The precedence (family variable > VP_PM_MANAGER > default) is unchanged.

External tool lookup in vp_command

  • Move the body of find_external_tool_in from shim/dispatch.rs to vp_command::find_external_tool. The caller now passes PATH, the bypass directories, and the executable to exclude through ExternalToolQuery. In the NAPI binding, current_exe() is the Node.js executable, so a future local CLI caller can pass self_exe: None instead of excluding the system Node.js.
  • dispatch.rs keeps find_external_tool_in(tool, cwd) as a thin wrapper that reads the same process environment as before, so the existing test_find_external_tool_* tests run unchanged.
  • Move the managed-install check (target under <DATA>/js_runtime or <DATA>/package_manager) to vp_command::is_managed_install.

Follow-ups

Next, VP_*_MANAGER=system-only will make shims use only the system Node.js and package managers, without falling back to managed versions or downloading. Those changes and their tests come in separate PRs after this one merges.

Testing

Run on top of the current main:

  • just check
  • just lint
  • just test
  • cargo fmt --all -- --check
  • typos
  • just snapshot-test-global shim and just snapshot-test-global command_self_setup (CI)

Fails locally on NixOS before and after this change (same cases); relying on CI.

@fengmk2
fengmk2 requested a review from liangmiQwQ October 20, 2026 13:17
pub const VP_BUN_VERSION: &str = "VP_BUN_VERSION";

/// Node.js management preference (`yes` or `no`).
pub const VP_NODE_MANAGER: &str = "VP_NODE_MANAGER";

@liangmiQwQ liangmiQwQ Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

.env("VP_NODE_MANAGER", manager)

Cloud we reuse these variables in other modules as well?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants