jzaefferer/commitplease

Issue running npm install when using node via homebrew

Closed this issue · 19 comments

ipaintcode commented

Terminal output ( {pathToProject } is in place of actual path ) when running npm install (node v5.1.0)

commitplease@2.2.3 install {pathToProject}/node_modules/commitplease
node install

Detected an existing git commit-msg hook
Remove it and install this package again to install commitplease properly

Mark: git status
fatal: This operation must be run in a work tree

Due to {pathToProject}/.git/hooks/commit-msg -> #!/bin/evn node

Seems to be since homebrew doesn't use /usr/bin for node or even a symlink you have to replace:

!/bin/evn node

with

!/usr/local/bin/node

The issue is installing on a fresh machine or having to reclone said project using this npm. Is there any chance of updating this so the CLI doesn't break with homebrew's install of node? It also seems in El Capitan you can't force symlinks in the /usr/bin directory, well maybe if you disabled root user but rather not go down that road.

Example, here's the ln command and output when trying to force a symlink to that directory in El Cap:

sudo ln -sf /usr/local/bin/node /usr/bin/node
ln: /usr/bin/node: Operation not permitted

Thanks in advance.

jzaefferer commented

I have no idea why I was using "#!/bin/evn node" in the first place, replacing that seems fine. Just to be sure: Did the issue only occur when running the commit hook, or already during installation of the npm package? Your description seems to indicate the latter, but I don't understand why that line would cause an issue during installation.

rxaviers commented

Using #!/usr/local/bin/node is not cross-OS compatible, because you don't know where node is installed in someone's OS.

jzaefferer commented

To verify the suggested (or other) solutions work, you can check out the repo, patch the one line, then use npm install path/to/checkout to do an install from the checkout (don't use npm-link, that doesn't work here).

arschmitz commented

we should dynamically generate this path with process.execPath

jzaefferer commented

http://stackoverflow.com/questions/20638520/appropriate-hashbang-for-node-js-scripts says that #!/usr/bin/env node is the right hashbang for node, which is currently in use.

I'm still not sure what the actual issue is, since this file isn't executed while installing.

jzaefferer commented

The install script in package.json just references node: https://github.com/jzaefferer/commitplease/blob/master/package.json#L7 - if that isn't in your path, it won't work, but there's nothing about a hashbang going on there.

scottgonzalez commented

@jzaefferer is correct. #!/user/bin/env node is the correct shebang line to have in the commit hook, and the shebang line doesn't even come into play on install. So based on the details in this bug report, there is nothing to change inside commitplease.

arschmitz commented

I talked to @ipaintcode about this last night the problem is on commit not on install. but still not sure whats going on since #!/user/bin/env node should work anywhere

arschmitz commented

even more interesting the grunt-cli uses this exact same shebang and works for them fine. There has to be something else going on here.

ipaintcode commented

Looks like @arschmitz found the culprit, a big thanks to everyone (thanks again @arschmitz) for checking into this. I did do a fresh install on notebook and seems to have fixed the issue. It was a very weird one off that seemed to be breaking unless I changed the #! to the ladder I posted.

arschmitz commented

I did a little more looking into this and it turns out this is kind of a real issue. It actually prevents the usage of commit please using any OSX App like the github client, source-tree, sublime git plugin, atom git plugin etc. Basically OSX apps don't have access to the PATH

Im really really surprised this has not come up previously on one of the jQuery projects

jzaefferer commented

@arschmitz could you provide some steps to reproduce with one of those apps?

mgol commented

@jzaefferer Steps are as follows:

  1. Make sure you're on the latest El Capitan (10.11.3)
  2. Uninstall Node.js
  3. Install nvm
  4. Install any Node version using nvm, e.g. nvm install 5
  5. Try to use commitplease from the terminal (e.g. by commiting in the jQuery repo), it works
  6. Install SourceTree, add the jQuery repo to bookmarks, open it & try to commit

It won't find Node as the PATH it sees is only /usr/bin:/bin:/usr/sbin:/sbin.

mgol commented

Hardcoding the PATH would have its own problems, though (jquery/jquery#2915). Currently the jQuery repo uses both husky & commitplease so gets the worst of both as both problems exist. :-)

jzaefferer commented

Thanks for the steps. I wonder if you could create a symbolic link in /usr/bin, pointing at nvm's default node version (set with nvm alias default ...). That way SourceTree should be able to find the node binary, and it updates as you change your default alias.

mgol commented

I wonder if you could create a symbolic link in /usr/bin

I can't. El Capitan's System Integrity Protection prevents you from writing to most system directories, including those 4 ones that are in the PATH for GUI apps. Even root can't do that.

arschmitz commented

We originally ran into this at my company the work around we settled on is documenting that if you use a GUI for committing to open the commit hook and set the path manually. Since this is within the .git folder it will remain untouched even when reinstalling etc. we thought about submitting a PR for other solutions but felt they all had other significant draw backs as discussed in jquery/jquery#2915

I would just add a note to the documentation about GUI apps and how to fix them and about updating if you bump node versions.

Just thinking out loud but maybe we could add a command to do this for the user not real sure about the post install thing so this might be a middle ground

jzaefferer commented

Can't tell if this is still an issue, since its been a year, I'm closing.