fix install shim target resolution#6684
Conversation
|
Ready to review this PR? Stage has broken it down into 3 individual chapters for you:
Chapters generated by Stage for commit 68f8bd0 on Jul 6, 2026 10:11pm UTC. |
Summary by CodeRabbit
WalkthroughThe PR adds a ChangesShim Target Resolution
Sequence Diagram(s)sequenceDiagram
participant install_app
participant create_shims
participant resolve_shim_target
participant GetCommand as Get-Command
install_app->>create_shims: call with original_dir
create_shims->>resolve_shim_target: resolve_shim_target(dir, target, original_dir)
resolve_shim_target->>resolve_shim_target: check rooted/relative candidates in dir and original_dir
alt candidate found
resolve_shim_target-->>create_shims: return matched leaf path
else no candidate found
resolve_shim_target->>GetCommand: Get-Command target -CommandType Application
GetCommand-->>resolve_shim_target: Source path
resolve_shim_target-->>create_shims: return Source
end
Estimated code review effort: 2 (out of 5) Related issues: None found. Related PRs: None found. Suggested labels: enhancement Suggested reviewers: None identified. 🐰 🚥 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lib/install.ps1 (1)
180-205: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: consider testing/covering priority ordering (current wins over original).
The resolver correctly checks
$dirbefore$original_dir, but there's no test asserting this precedence when a target exists in both locations. Not blocking, but would strengthen regression coverage for the exact bug this PR fixes.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0d1e380f-ec64-4065-8d02-ad7b3bc04e84
📒 Files selected for processing (2)
lib/install.ps1test/Scoop-Install.Tests.ps1
Summary
This fixes shim creation during install when the newly linked
currentdirectory does not expose the target binary at the momentcreate_shimsresolves it.Root cause
install_applinks the version directory tocurrentand then callscreate_shimswith only thecurrentpath. IfTest-Path "$dir\$target"does not see the binary, the fallback callsGet-Command $target, which emits a command-not-found error and can leave the app in a failed install state even though the downloaded binary exists in the version directory.Changes
create_shims.current, then the original version directory, then existing absolute/relative/PATH targets.Get-Command -ErrorAction Ignorefor the PATH fallback so missing app-local binaries do not leak a raw command-not-found error.Validation
Tests Passed: 10, Failed: 0.scoop-searchsuccessfully withscoop uninstall scoop-search; scoop install scoop-search --no-update-scoop.Get-Command scoop-searchresolves to the shim andscoop-search gitreturns results.git diff --check -- lib/install.ps1 test/Scoop-Install.Tests.ps1.