Repository navigation
Look a bare executable name up on the overlay PATH [patch] - #137
Conversation
With UseShellExecute off, Process.Start searches the calling process's PATH, not the PATH in ProcessStartInfo.Environment. An overlay that set PATH to pin a toolchain either failed to start a tool only it held, or silently started a same-named tool from the caller's PATH. When the overlay sets PATH and the name has no directory part, the name is now resolved against the overlay's PATH: first executable match wins, relative entries are skipped, and PATHEXT is applied on Windows. With no match, or no PATH in the overlay, the old lookup is kept. Fixes #122 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW5RAKCxdwrsVmygQDP
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW5RAKCxdwrsVmygQDP
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| foreach (KeyValuePair<string, string?> variable in environmentVariables) | ||
| { | ||
| if (string.Equals(variable.Key, "PATH", comparison)) | ||
| { | ||
| path = variable.Value; | ||
| } | ||
| } |
There was a problem hiding this comment.
This stays as a loop. The suggestion only changes style, and the loop already gives the "last match wins" result the alert describes. That result matters on Windows, where the overlay dictionary can hold both Path and PATH. System.Linq is currently imported only for netstandard2.0, so switching to LINQ would mean adding a using for every target, with no change in behaviour.
Generated by Claude Code
| return null; | ||
| } | ||
|
|
||
| string candidate = Path.Combine(directory, fileName); |
There was a problem hiding this comment.
I traced this, and it can't happen through the current caller. ResolveOnSearchPath returns early for any name that contains /, or on Windows \ or :, and a PATHEXT entry only appends an extension. So fileName never reaches TryExecutable rooted. The guard costs one condition, so 030eae5 adds it anyway: a rooted name is now skipped instead of being combined.
Generated by Claude Code
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW5RAKCxdwrsVmygQDP
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW5RAKCxdwrsVmygQDP
|
Fixes #122
What changed
With
UseShellExecute = false,Process.Startsearches the calling process'sPATHfor a bareFileName, not thePATHinProcessStartInfo.Environment. An overlay that setPATHto pin a toolchain either threwWin32Exceptionfor a tool that only the overlay held, or silently started a same-named tool from the caller'sPATH.CreateStartInfonow looks a bare name up on the overlay'sPATHonce the overlay has been applied:PATHand the name has no directory part (/, plus\or:on Windows).PATHentries are skipped, so the result doesn't depend on the caller's current directory.PATHEXTentry, taken from the command's environment, with a default of.COM;.EXE;.BAT;.CMD.PATH, or it removesPATHwithnull, the lookup falls back to the current behaviour.The README's Environment Variables section now describes this lookup.
Tests
The three new tests run on Unix and are inconclusive on Windows, following the existing pattern:
ExecuteAsyncShouldStartAToolThatOnlyTheOverlayPathHolds: without the fix this threwWin32Exception.ExecuteAsyncShouldPreferTheOverlayPathOverTheCallersPath: anechoscript on the overlayPATHruns instead of/usr/bin/echo.ExecuteAsyncShouldSkipANonExecutableFileOnTheOverlayPath: a file without an execute bit is passed over for the next directory.All three fail with the fix reverted and pass with it. The full suite on net10.0 (Linux) gives 67 passed, 2 skipped (the elevation tests) and 0 failed. The library builds clean on every target framework.
Coordination with #131
#131 resolves relative paths such as
./build.shagainstWorkingDirectory. This PR resolves bare names on the overlayPATH. The two cases don't overlap. I placed the helpers belowCreateStartInfoso that this branch and #131 merge without a textual conflict (checked withgit merge-tree).Generated by Claude Code