Repository navigation
fix(asdf): expose dependencies to install scripts - #11531
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughasdf installation now resolves dependency tool paths, places them before active tool paths, and removes duplicates before running plugin scripts. Backend helper naming and documentation were updated, with end-to-end coverage for PATH precedence and parallel installation. Changesasdf dependency PATH support
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Backend
participant install_version_
participant asdf_plugin_scripts
Backend->>install_version_: resolve dependency toolset
install_version_->>install_version_: combine and deduplicate dependency and active paths
install_version_->>asdf_plugin_scripts: run download and install scripts with ordered PATH
asdf_plugin_scripts-->>Backend: install dependent executable
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
e2e/backend/test_asdf_install_dependency_path (1)
8-14: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the ambient PATH fixture self-contained.
Create
$HOME/binand prepend it toPATHhere; otherwise a harness without that setup can skip the shadowing scenario or fail before the assertion.Proposed fix
# A same-named executable already on the ambient PATH must not outrank the # dependency declared by the tool. +mkdir -p "$HOME/bin" +export PATH="$HOME/bin:$PATH" cat >"$HOME/bin/dummy" <<'EOF'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@e2e/backend/test_asdf_install_dependency_path` around lines 8 - 14, Update the ambient PATH fixture around the dummy executable setup to create $HOME/bin before writing the file, then prepend $HOME/bin to PATH in the test environment. Keep the existing dummy executable contents and permissions unchanged so it reliably shadows any same-named executable during the dependency-path assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@e2e/backend/test_asdf_install_dependency_path`:
- Around line 8-14: Update the ambient PATH fixture around the dummy executable
setup to create $HOME/bin before writing the file, then prepend $HOME/bin to
PATH in the test environment. Keep the existing dummy executable contents and
permissions unchanged so it reliably shadows any same-named executable during
the dependency-path assertion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: 76f4968d-a6cb-4f98-88bc-5eb7dd57047f
📒 Files selected for processing (5)
docs/dev-tools/backends/asdf.mde2e/backend/test_asdf_install_dependency_pathsrc/backend/asdf.rssrc/backend/mod.rssrc/backend/vfox.rs
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Greptile SummaryThis PR exposes installed dependencies to asdf installation scripts.
Confidence Score: 5/5The PR appears safe to merge. The previously reported alias-resolution defect is fixed: dependency names are normalized to short and full backend forms before toolset filtering, and the regression test exercises the nodejs-to-node alias path. Important Files Changed
Reviews (3): Last reviewed commit: "fix(asdf): expose dependencies to instal..." | Re-trigger Greptile |
Summary
bin/downloadandbin/installscripts during the samemise installdependsentries offline, place their bin paths ahead of ambient active-tool paths, and remove exact duplicatesProblem
The install graph already waited for tools declared through
[tools].depends, but the asdf script environment was built from the raw, unresolved install toolset. A dependency could therefore finish installing successfully while its executable was still absent from the PATH used by the dependent plugin. This produced the first-install failure reported with Python and Poetry in #4384; a later invocation worked only because the shell environment had been refreshed.The registry now routes the Poetry shorthand through non-asdf backends, but the same generic defect remains for explicit, existing, and private asdf plugins.
Implementation
The dependency toolset helper now represents both backend-declared and per-instance dependencies and is shared by tools-enabled environment rendering and asdf installation. For asdf scripts, dependency paths are resolved offline after the install graph has completed those tools, prepended ahead of ambient configured paths, and deduplicated while preserving toolset order. Both
bin/downloadandbin/installreceive the resulting PATH. vfox hook PATH behavior is unchanged.Testing
mise run test:e2e e2e/backend/test_asdf_install_dependency_path e2e/cli/test_tool_depends e2e/backend/test_vfox_install_tools_env_depmise exec -- cargo test --all-features backend::asdf::testsmise exec -- cargo check --all-featuresmise exec -- cargo fmt --all -- --checkThe full
mise run lintwrapper cannot run in this Sapling working copy becausehkrequires Git repository discovery; all checks relevant to the changed files were run individually. Docker Desktop was unavailable locally, so the hermetic Unix E2E will receive its Linux coverage in CI.Addresses #4384
AI-assisted — Tool: Codex; model: OpenAI/GPT-5; version: unavailable.
Summary by CodeRabbit
New Features
Documentation
Tests