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

Support cygpath for cygwin-based git executable #70

Closed
wants to merge 1 commit into from
Closed

Support cygpath for cygwin-based git executable #70

wants to merge 1 commit into from

Conversation

kwonoj
Copy link

@kwonoj kwonoj commented Oct 9, 2016

This PR updates behavior of getting git path for edge-case windows users, like using git executable from msys2 or similar (i.e, cygwin).

Since those git executable returns path as *nix style in windows system, it'll end up husky hook installation fails by

husky
setting up hooks in .git/hooks
fs.js:922
return binding.mkdir(pathModule._makeLong(path),
^

Error: ENOENT: no such file or directory, mkdir 'Z:\z\github\g.git\hooks'
at Error (native)
at Object.fs.mkdirSync (fs.js:922:18)
at Object.module.exports.create (Z:\github\g\node_modules\husky\src\index.js:123:33)
at Z:\github\g\node_modules\husky\bin\install.js:22:13
at Array.forEach (native)
at Z:\github\g\node_modules\husky\bin\install.js:20:11
at Z:\github\g\node_modules\husky\src\index.js:17:9
at ChildProcess.exithandler (child_process.js:193:7)
at emitTwo (events.js:106:13)
at ChildProcess.emit (events.js:191:7)

try to create hook under Z:\z\github\g\.git\hooks, instead of correct windows path Z:\github\g\.git\hooks

In changes, it borrows cygpath included in msys2 and cygwin and let it correct path to windows style paths.

@the0neWhoKnocks
Copy link

I was literally just working on a fix for this. My fix was

// src/index.js:17

if( stdout.indexOf('cygdrive') > -1 ){
  stdout = stdout.replace(/\/cygdrive\/([a-z])\//, '$1:/')
}

Yours seems more comprehensive. Any idea when this'll make it in? Would be nice to be able to use this with Cygwin.

@kwonoj
Copy link
Author

kwonoj commented Nov 8, 2016

@the0neWhoKnocks suggested regex will work on cygwin, but won't on msys2. I'd thought of direct string match-replace first, but choose to rely on cygpath instead.

@the0neWhoKnocks
Copy link

@kwonoj - correct hence my "more comprehensive" statement ;-).

@typicode - There an idea of when this'll get merged in?

@typicode
Copy link
Owner

Since cygwin doesn't seem to have a great support in Node, I have mixed feelings about adding cygwin specific commands/code in husky.
nodejs/node-v0.x-archive#6459
https://github.com/npm/npm#installing-on-cygwin

That said, I'm considering https://www.npmjs.com/package/find-parent-dir as an alternative way to find .git directory than git rev-parse.

I've not yet tested it on cygwin but maybe it wouldn't have the same issue with Z:\z\github\.git\hooks?

@kwonoj
Copy link
Author

kwonoj commented Nov 17, 2016

I've been previously suggested app-root-path module in other packages having similar purpose (lint-staged/lint-staged#19) which works gracefully on most windows configuration. This PR intended to amend existing behavior only, if you'd update behavior to not to use git it's fine to close PR without check in.

@typicode
Copy link
Owner

typicode commented Dec 9, 2016

I've just published a beta version that doesn't rely on git command and should work with cygwin. If you want to give it a try:

npm install husky@beta 

Let me know if you still have issues with it.

@the0neWhoKnocks
Copy link

@typicode - Gave the beta a whirl and things are working as they used to again. 👍

@typicode
Copy link
Owner

typicode commented Dec 9, 2016

Awesome thanks @kwonoj, @the0neWhoKnocks for the feedback and help.

@aminya
Copy link

aminya commented Sep 23, 2020

Isn't this related to #673 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants