search

vercel / git-hooks

No nonsense Git hook management
MIT License
203 stars 10 forks source link

cross OS usage #10

Open advanced-media opened 3 years ago

advanced-media commented 3 years ago

On Windows it's possible to install the package using an admin console (since .git folders are read-only for normal users). Otherwise symlink creation will fail due to missing permissions (.cjs files -however- are created, so creating and removing symlinks seems to need more permissions, than file creation):\ Error: EPERM: operation not permitted, symlink './_do_hook.cjs' -> '[path\to\hooks]\applypatch-msg' (install.js:122:5)

Result in hooks with proper credentials (admin) will be e.g.:

06.09.2021  05:09    <DIR>          .
06.09.2021  05:09    <DIR>          ..
06.09.2021  05:09    <SYMLINK>      applypatch-msg [.\_do_hook.cjs]
01.09.2021  18:18               478 applypatch-msg.sample
06.09.2021  05:09    <SYMLINK>      commit-msg [.\_do_hook.cjs]
01.09.2021  18:18               896 commit-msg.sample
06.09.2021  05:09    <SYMLINK>      post-applypatch [.\_do_hook.cjs]
06.09.2021  05:09    <SYMLINK>      post-checkout [.\_do_hook.cjs]
...

But uninstall is failing on removing symlinks due to different os path separator (baked into symlink string during creation).\ (codepointer)

△  @vercel/git-hooks: hook 'applypatch-msg' appears to be a user-defined hook; skipping: [project\path]\.git\hooks\applypatch-msg

fs.readlinkSync(hookPath) gives '.\\_do_hook.cjs' ('.\\_do_hook' before #7)

Solution could be: adding different path sepatators as character class to RegExp literal [\\/]

const isOneOfOurs = fs.lstatSync(hookPath).isSymbolicLink() && fs.readlinkSync(hookPath).match(/\.[\\/]_do_hook(\.cjs)?/);

or (better readable): using os path separator from path module without RegExp

const isOneOfOurs = fs.lstatSync(hookPath).isSymbolicLink() && [`.${path.sep}_do_hook`, `.${path.sep}_do_hook.cjs`].includes(fs.readlinkSync(hookPath));

btw. trying to include dynamic path separator into RegExp constructor string is a bit cumbersome due to a lack of proper build-in escaping for RegExp...\

new RegExp("."+needToRegExpEscape(path.sep)+"_do_hook(\\.cjs)?")

Would be nice to see a patch on this soon. And maybe you can mention this troubleshooting (admin console on Windows) in the README. Also, I had to guess where to put the config JSON, would be nice to mention the package.json in the README as well.

kind regards.