Don't error in the unusual case that a file is on the search path - #2660
Don't error in the unusual case that a file is on the search path#2660henrymercer merged 2 commits into
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
Copilot reviewed 23 out of 36 changed files in this pull request and generated no comments.
Files not reviewed (13)- package.json: Language not supported
- queries/binary-planting.ql: Language not supported
- src/init-action.ts: Evaluated as low risk
- src/codeql.test.ts: Evaluated as low risk
- src/tar.ts: Evaluated as low risk
- src/init.ts: Evaluated as low risk
- src/actions-util.ts: Evaluated as low risk
- src/git-utils.ts: Evaluated as low risk
- lib/util.js: Evaluated as low risk
- src/analyze.ts: Evaluated as low risk
- lib/analyze.js: Evaluated as low risk
- lib/codeql.test.js: Evaluated as low risk
- lib/init-action.js: Evaluated as low risk
src/util.ts:1197
- The function isBinaryAccessible should handle the case where the binary is found but is not executable. Consider adding a check to ensure the binary is executable after finding it.
await io.which(binary, true);
lib/tar.js:51
- Ensure that the behavior of io.which when the binary is not found is covered by tests. This is critical to confirm that the new implementation does not introduce unexpected behavior.
const tar = await io.which("tar", true);
lib/actions-util.js:225
- The error message should be more descriptive and user-friendly. Consider providing guidance on how to install the file program.
throw new FileCmdNotFoundError(`The `file` program is required, but does not appear to be installed. Please install it: ${e}`);
Tip: Copilot only keeps its highest confidence comments to reduce noise and keep you focused. Learn more
Sorry, something went wrong.
angelapwen
left a comment
There was a problem hiding this comment.
Looks good — also looks like your PR to safe-which is merged, so not sure if we want to just use that?
Sorry, something went wrong.
|
My thinking was that it makes slightly more sense to use @actions/io for consistency with other first-party Actions, but I don't think there's a huge difference. |
Sorry, something went wrong.
In the unusual case that a file is on the search path, log a warning but don't throw.
I've accomplished this by switching our which implementation to use the standard Actions one within @actions/io but have also submitted a PR to safe-which: chrisgavin/safe-which#14.
Merge / deployment checklist