Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use isexe module #1404

Merged
merged 1 commit into from
Oct 20, 2020
Merged

Use isexe module #1404

merged 1 commit into from
Oct 20, 2020

Conversation

ehmicky
Copy link
Contributor

@ehmicky ehmicky commented Oct 15, 2020

Some of our code is a full copy/paste of the isexe dependency. I happened to remember it because I once looked at the code of this library, but our inlining was lacking attribution (this dependency license requires it).

By using the module, we also benefit from:

  • async instead of sync
  • Windows support

@ehmicky ehmicky added the type: chore work needed to keep the product and development running smoothly label Oct 15, 2020
@ehmicky ehmicky requested a review from erezrokah October 15, 2020 18:34
@ehmicky ehmicky requested a review from a team as a code owner October 15, 2020 18:34
@ehmicky ehmicky self-assigned this Oct 15, 2020
@@ -55,7 +34,7 @@ const isVersionOutdated = async ({ packageName, currentVersion }) => {
const shouldFetchLatestVersion = async ({ binPath, packageName, execName, execArgs, pattern, log }) => {
const execPath = path.join(binPath, getExecName({ execName }))

const exists = await execExist(execPath)
const exists = await isExe(execPath)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isexe contains the code of execExist() not of isExe().
See source

const stat = await fs.statAsync(binPath)
return stat.isFile() && isExe(stat.mode, stat.gid, stat.uid)
} catch (error) {
if (error.code === 'ENOENT') return false
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This ENOENT check is the reason we use ignoreErrors: true. However, this will ignore all errors, not only ENOENT.
Without that option, only EACCESS is ignored.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: chore work needed to keep the product and development running smoothly
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants