Skip to content

fix(backend): apply install_env to install commands#9929

Merged
jdx merged 6 commits into
jdx:mainfrom
risu729:fix/install-env-backends
May 18, 2026
Merged

fix(backend): apply install_env to install commands#9929
jdx merged 6 commits into
jdx:mainfrom
risu729:fix/install-env-backends

Conversation

@risu729

@risu729 risu729 commented May 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • applies per-tool install_env to command-backed install paths across package-manager backends, vfox cmd.exec hooks, SPM build/probe commands, and core language install-time commands
  • adds install_env docs for affected backend and language pages, including SPM shorthand and vfox helper caveats
  • adds a vfox e2e covering install_env propagation into cmd.exec during install

Project item: https://github.com/users/risu729/projects/3/views/1?filterQuery=install_env&pane=issue&itemId=188373493

Tests

  • cargo fmt --all -- --check
  • git diff --check && git diff --cached --check
  • CARGO_TARGET_DIR="$(cargo metadata --format-version 1 --no-deps | jq -r .target_directory)" mise run test:e2e -- test_vfox_install_env

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the install_env tool option across all mise backends, allowing users to specify environment variables for tool installation and build processes. The update includes comprehensive documentation for each backend and a new E2E test. Feedback indicates that the precedence of environment variables needs adjustment in several plugins: critical mise-managed variables (e.g., RUSTUP_TOOLCHAIN, DOTNET_ROOT, PIP_REQUIRE_VIRTUALENV) should be protected from user overrides to prevent corrupted installations, whereas user-defined variables should be allowed to override non-critical defaults like MAKEFLAGS or global CFLAGS.

Comment thread src/plugins/core/rust.rs Outdated
Comment thread src/plugins/core/rust.rs Outdated
Comment thread src/plugins/core/dotnet.rs Outdated
Comment thread src/plugins/core/erlang.rs Outdated
Comment thread src/plugins/core/node.rs Outdated
Comment thread src/plugins/core/python.rs Outdated
@greptile-apps

greptile-apps Bot commented May 16, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR propagates the per-tool install_env option into every command-backed install path across all package-manager backends, vfox cmd.exec, SPM build/probe commands, and all core language install-time commands, along with matching documentation and an e2e test for vfox.

  • Core plumbing: A new ToolVersion::install_env() helper centralises access to the option; the SPM backend introduces a with_install_env wrapper (needed because duct::Expression has no .envs() method); the vfox backend merges install_env into cmd_env after dependency env, so it applies even when dependency resolution fails.
  • Node refactoring: fetch_tarball is restructured to separate the lock-platform read from the mutable entry update, which was required to allow tv to be passed to get_checksum/verify_with_gpg without holding two borrows on tv.lock_platforms simultaneously — the logic is equivalent to the original.
  • Env ordering: install_env is consistently inserted before mise-managed tool paths so those remain authoritative, matching the intentional pattern documented in previous review threads.

Confidence Score: 5/5

Safe to merge; the change is additive with no control-flow changes outside the deliberate node.rs borrow-checker refactor, which is logically equivalent to the original.

Every modified command site follows a consistent, reviewed ordering pattern. The structural refactor in node.rs preserves identical semantics. The vfox and pipx changes carry incidental correctness improvements. Previously discussed ordering trade-offs are acknowledged and intentional.

No files require special attention beyond what was already discussed in prior review threads.

Important Files Changed

Filename Overview
src/toolset/tool_version.rs Adds install_env() helper method on ToolVersion that clones the map from request.options() on every call; used uniformly across all call sites
src/backend/spm.rs Introduces with_install_env helper (necessary because duct Expression lacks an envs() method) and applies it to all three swift command sites
src/backend/vfox.rs Merges install_env into cmd_env after dependency env, correctly using unwrap_or_default() so install_env still applies when dependency_env errors
src/plugins/core/node.rs Refactors fetch_tarball to read lock_platforms before passing tv to get_checksum to avoid borrow conflicts; adds install_env to all six command sites
src/backend/pipx.rs Reorders env chain to add install_env and fixes existing bug where UV_TOOL_DIR and PIP_INDEX_URL could be overridden by ts.env_with_path_without_tools
src/plugins/core/erlang.rs Restructures kerl command so install_env can override MAKEFLAGS but KERL_BASE_DIR remains authoritative
src/plugins/core/rust.rs Adds install_env before exec_env on all three rustup/rustc command sites; exec_env still takes precedence for conflicting keys
src/plugins/core/dotnet.rs Adds install_env before exec_env on both commands; exec_env remains authoritative by design
e2e/backend/test_vfox_install_env New e2e test that creates an in-process vfox plugin and asserts install_env is visible inside PostInstall cmd.exec()
src/backend/npm.rs Adds install_env before BUN/PNPM/NPM manager-managed path env vars on all four install paths

Reviews (5): Last reviewed commit: "fix(backend): remove dead cargo feature ..." | Re-trigger Greptile

Comment thread src/plugins/core/erlang.rs Outdated
Comment thread src/install_context.rs Outdated
Comment thread src/plugins/core/rust.rs
Comment thread src/plugins/core/dotnet.rs
@risu729 risu729 force-pushed the fix/install-env-backends branch from 48938ed to 75037f2 Compare May 18, 2026 11:10
@risu729 risu729 marked this pull request as ready for review May 18, 2026 11:39
@risu729 risu729 force-pushed the fix/install-env-backends branch from 70dc752 to b9301f7 Compare May 18, 2026 18:46
@jdx jdx merged commit f9438e5 into jdx:main May 18, 2026
33 checks passed
@risu729 risu729 deleted the fix/install-env-backends branch May 18, 2026 19:06
mise-en-dev added a commit that referenced this pull request May 19, 2026
### 🚀 Features

- **(cli)** rename before flag to minimum release age by @risu729 in
[#9768](#9768)
- **(core)** deprecate default package files by @jdx in
[#9970](#9970)
- **(edit)** add --global flag for editing the global config file by
@fru1tworld in [#9953](#9953)

### 🐛 Bug Fixes

- **(aqua)** support cosign public-key bundles by @jdx in
[#9972](#9972)
- **(backend)** pass install_env to postinstall by @risu729 in
[#9930](#9930)
- **(backend)** apply install_env to install commands by @risu729 in
[#9929](#9929)
- **(cargo)** skip binstall for cargo install options by @risu729 in
[#9928](#9928)
- **(config)** restore MISE_ENV_FILE setting by @risu729 in
[#9903](#9903)

### 🚜 Refactor

- **(cli)** use tool wording in version env help by @risu729 in
[#9906](#9906)
- **(conda)** parse tool options locally by @risu729 in
[#9960](#9960)
- **(core)** parse plugin tool options locally by @risu729 in
[#9963](#9963)
- **(go)** parse tool options locally by @risu729 in
[#9961](#9961)
- **(http)** parse tool options locally by @risu729 in
[#9870](#9870)

### 📦️ Dependency Updates

- lock file maintenance by @renovate[bot] in
[#9954](#9954)
- lock file maintenance by @renovate[bot] in
[#9957](#9957)

### 📦 Registry

- use aqua backend for qsv by @risu729 in
[#9910](#9910)

### Ci

- build/publish snap package for arm64 by @jnsgruk in
[#9948](#9948)

### New Contributors

- @jnsgruk made their first contribution in
[#9948](#9948)

## 📦 Aqua Registry Updates

### New Packages (2)

- [`AOMediaCodec/libavif`](https://github.com/AOMediaCodec/libavif)
- [`julian7/redact`](https://github.com/julian7/redact)

### Updated Packages (1)

- [`apache/jena`](https://github.com/apache/jena)
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