Repository navigation
Skip a non-executable PATH match when looking up a helper [patch] - #35
Merged
Merged
Conversation
ExecutableLocator.Find returned the first file on PATH with the right name, so a stray non-executable systemd-inhibit earlier on PATH hid the real one: --status reported support and every Acquire failed with Permission denied. Skip a match with no execute bit (off Windows), as the shell and execvp do. Fixes #34 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW2EzJMFdK7ZQBB2keC
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW2EzJMFdK7ZQBB2keC
Collaborator
Author
|
The SonarCloud gate fails on 66.7% new-code coverage, and I'm leaving it as is. The new lines it reports as uncovered are the ones the Linux test run can't reach:
All the reachable paths are tested: an executable match, a non-executable match, a directory with the command's name, and no match at all. The gate is not a required check, so it doesn't block merging. Generated by Claude Code |
…table-path-entries
SonarCloud measures coverage from the Windows run, where the Unix-only branch never executes, so new-code coverage sat at 66.7%. The decision now takes its mode reader as a parameter and is tested directly, including the unreadable-mode cases, and a cross-platform test drives Find over a given path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW5jpiuVuBRXSgitSY1
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.



Fixes #34
What was wrong
ExecutableLocator.Findreturned the first file onPATHwhose name matched, using onlyFile.Exists. The shell,command -vandexecvpall skip a match that has no execute bit. So a stray, non-executablesystemd-inhibitearlier onPATHhid the real one.--statusthen reported "supported", and everyAcquirefailed with Permission denied.Change
Findnow skips a candidate unless it is a regular file and, off Windows, has at least one ofUserExecute,GroupExecuteorOtherExecuteset. A directory with the command's name was already skipped byFile.Exists, and the new test covers it.internal Find(fileName, path)overload that the publicFindcalls with thePATHvariable. The test passes its own search path, so it doesn't change the process environment.Tests
PlatformIntegrationTests.Find_SkipsANonExecutableMatchEarlierOnThePathsets up three directories in this order: one with a mode-644 file named like the command, one with a directory named like it, and one with an executable file. It checks thatFindreturns the executable, and returnsnullwhen only the first two are on the path. On Windows the test is inconclusive.With the
ExecutableLocatorchange reverted, the new test fails. With it, the full suite passes (30/30) on Linux.Generated by Claude Code