cactbot: CI: Husky pre-commit hook fails when running in Github for Windows

When attempting to commit to cactbot while using Github for Windows, it fails with:

Commit failed - exit code 1 received, with output: 'husky > pre-commit (node v13.9.0)
Path/To/GitHub/cactbot/node_modules/.bin/lint-staged: line 5: cygpath: command not found
internal/modules/cjs/loader.js:983
  throw err;
  ^

Error: Cannot find module 'D:\lint-staged\bin\lint-staged.js'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:980:15)
    at Function.Module._load (internal/modules/cjs/loader.js:862:27)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
    at internal/main/run_main_module.js:17:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}
husky > pre-commit hook failed (add --no-verify to bypass)'

However, committing from the command line correctly invokes the hook and successfully commits.

From a little searching around, it looks like this is due to restrictions on PATH when invoked by the Github for Windows GUI.. It’s entirely possible that my Node installation is incorrect, but I’m not knowledgeable enough to troubleshoot that.

This doesn’t directly block me, since the command line is available, but I do like the Github for Windows client, and if it’s possible I would like to continue using it.

About this issue

Original URL
State: closed
Created 4 years ago
Comments: 22 (15 by maintainers)

Commits related to this issue

npm: Update Lint-Staged Command to Run Via npx Changes `lint-staged` to run via `npx` rather than being invoked directly. This change sets up the PATH for various programs like VSCode or GitHub Deskt... — committed to panicstevenson/cactbot by panicstevenson 4 years ago

Most upvoted comments

I was able to replicate this within a virtual machine of a fresh Windows 10 install:

Windows 10 Enterprise 64-bit Version 1903
Git for Windows v2.27.0
GitHub Desktop for Windows 10 v2.5.3
Node LTS Windows MSI v12.18.2

I used 100% default installations, cloned my repository via GitHub Desktop, and ran npm install via Git Bash. I assume this is a close enough mirror to the conditions both of you are in.

Commit failed - exit code 1 received, with output: 'husky > pre-commit (node v12.18.2)
/c/.../GitHub/cactbot/node_modules/.bin/lint-staged: line 5: cygpath: command not found
internal/modules/cjs/loader.js:969
  throw err;
  ^

Error: Cannot find module 'C:\lint-staged\bin\lint-staged.js'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:966:15)
    at Function.Module._load (internal/modules/cjs/loader.js:842:27)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:71:12)
    at internal/main/run_main_module.js:17:47 {
  code: 'MODULE_NOT_FOUND',
  requireStack: []
}
husky > pre-commit hook failed (add --no-verify to bypass)'

Playing around with things in this state, running npm install vs npm.cmd install I couldn’t get things exactly to a state where things were fixed; additionally, I didn’t want to edit PATH variables. I changed a few settings within GitHub Desktop, but those didn’t affect anything either. Essentially, I didn’t want any fix that was still going to impact the next user and they would have to figure it out or find this issue or give up. Ultimately the change would have to be that husky invoking lint-staged directly wouldn’t work and that it had to be hoisted up to npm where something would know about the environment necessary. npm run lint-staged doesn’t work, since that would be a user-defined script that we don’t have. However, npx lint-staged does work. At least, it does for me, on all of my (now three) setups.

Before opening a PR and calling this fixed for a workflow that I don’t use, I’d like to invite you both to changing the following code snippet within package.json:

"husky": {
  "hooks": {
    "pre-commit": "lint-staged"
  }
},

to:

"husky": {
  "hooks": {
    "pre-commit": "npx lint-staged"
  }
},

And see if that fixes your issues. The biggest issue I have seen with this is that it increases the time and cpu utilized of running pre-commit executions:

$ time lint-staged
ℹ No staged files found.
lint-staged  0.19s user 0.08s system 34% cpu 0.771 total

time npx lint-staged
npx: installed 97 in 3.824s
ℹ No staged files found.
npx lint-staged  3.73s user 2.18s system 109% cpu 5.409 total

There’s a relatively simple fix, which requires the developer to run:

npm install -g lint-staged

time npx lint-staged
ℹ No staged files found.
npx lint-staged  0.20s user 0.08s system 89% cpu 0.311 total

But unfortunately I don’t know of a way to disseminate that information in such a way that all users would know to install lint-staged globally.

Let me know how that fix works for you @JLGarber @VanSlanzar, and let me know if you find the trade-offs acceptable @quisquous. If not, we might be able to find another path forward.

panicstevenson on Jul 16, 2020

Yes, definitely. I think that would be good to spell out that using --no-verify if you’re stuck is an option.

quisquous on Jul 17, 2020

Ah, I misunderstood what problem the simple fix from a global installation was fixing! If nothing else, if quisquous approves this change, we can at least recommend a global install in the contribution guide.

Thanks so much for tracking this down.

JLGarber on Jul 17, 2020

Hmmm; well, it definitely works for me as I think we’d expect the path to go down. I’m assuming Windows 10? I only ask because some people actively refuse to ever upgrade, even the game that they authored a plugin for no longer officially supports it 👀

panicstevenson on Jul 16, 2020