shelljs / shx

Portable Shell Commands for Node
MIT License
1.73k stars 45 forks source link

Unable to use in postinstall script on Windows #88

Closed m0onspell closed 8 years ago

m0onspell commented 8 years ago

package.json:

"devDependencies": {
  "shx": "^0.1.4"
},
"scripts": {
  "postinstall": "npm install shx && shx ln -s $(pwd) node_modules/self"
}

Works fine on Linux. On Windows:

node_modules/self was unexpected at this time.

C:\Users\M0on75e1l\node_modules\some>  "C:\Users\M0on75e1l\node_modules\some\node_modules\.bin\\node.exe"  "C:\Users\M0on75e1l\node_modules\some\node_modules\.bin\\..\shx\lib\cli.js" ln -s $(pwd) node_modules/self
npm WARN ENOENT ENOENT: no such file or directory, open 'C:\Users\M0on75e1l\package.json'
npm WARN EPACKAGEJSON M0on75e1l No description
npm WARN EPACKAGEJSON M0on75e1l No repository field.
npm WARN EPACKAGEJSON M0on75e1l No README data
npm WARN EPACKAGEJSON M0on75e1l No license field.
npm ERR! Windows_NT 6.1.7601
npm ERR! argv "C:\\Program Files\\nodejs\\node.exe" "C:\\Program Files\\nodejs\\node_modules\\npm\\bin\\npm-cli.js" "install" "some@1.0.0-beta.3"
npm ERR! node v5.3.0
npm ERR! npm  v3.3.12
npm ERR! code ELIFECYCLE

npm ERR! some@1.0.0-beta.3 postinstall: `npm install shx && shx ln -s $(pwd) node_modules/self`
npm ERR! Exit status 255
npm ERR!
npm ERR! Failed at the some@1.0.0-beta.3 postinstall script 'npm install shx && shx ln -s $(pwd) node_modules/self'.
nfischer commented 8 years ago

Does this work if you add shx as a dev dependency?

m0onspell commented 8 years ago

@nfischer It's already added as a dev dependency. I'll update my explanation.

nfischer commented 8 years ago

First of all, you don't want to put npm install shx && in there. If shx is a devDependency, it will already be installed by the time you reach postinstall (this is what I observed on Linux).

First thing I see is that $(pwd) won't work on Windows. That works on Unix because bash already expands it before shx sees it. So shx would see ln -s /home/user/path/to/cwd node_modules/self as its input arguments on Unix, but sees ln -s $(pwd) node_modules/self on Windows.

If I were you, I would change the postinstall to be shx ln -fs ../.. node_modules/self. This removes an existing node_modules/self if you re-install. It also uses a relative path for the symlink.

Test coverage for shell.ln() is pretty low, so we might have some bugs there. Would you be able to do a clone of shelljs and see if npm test passes on your Windows machine?

m0onspell commented 8 years ago

Hmm. How it can be already installed when npm don't installs devDependencies of package by default? Only dependency list of xyz package is installed when doing npm install xyz. But I'll try that, whatever. Also will run your tests on Windows and post results here. I just wonder if there should be some note in the docs about this usage limitation?

levithomason commented 8 years ago

NPM does install devDependencies by default, unless you have NODE_ENV set to production. Then it only installs dependencies. In that case, you can force it to install devDependencies:

npm install --dev

Worth noting you can also force it to install production as well:

npm install --production
m0onspell commented 8 years ago

@levithomason that's what I said. And I don't want to force users of my package to make this kind of things:

npm install --dev

So I have my postinstall script like "npm install shx && ..."

levithomason commented 8 years ago

Hm. If you're package depends on shx and you're installing it manually when a user installs your package (postinstall), why not add it to dependencies instead and remove the manual postinstall install?

And I don't want to force users of my package to make this kind of things:

Also, npm i --dev does not install your dependencies devDepdendencies it installs your own dev deps. I didn't realize you were trying to get shx to your users. In that case, it should be a regular dependency of your package.

m0onspell commented 8 years ago

Hm. If you're package depends on shx and you're installing it manually when a user installs your package (postinstall), why not add it to dependencies instead and remove the manual postinstall install?

It's hard to tell whether its'a devDependency or just dependency. For me dependencies are libraries which my package uses on runtime. But in that case I'll probably just add it to dependencies to simplify postinstall script, however it won't fix the issue.

Also, npm i --dev does not install your dependencies devDepdendencies it installs your own dev deps. I didn't realize you were trying to get shx to your users. In that case, it should be a regular dependency of your package.

That's right, they will have to do even more and cd to my package root folder beforehand, so I never considered that variant.

@nfischer I tried your solution:

"dependencies": {
  "shx": "^0.1.4",
},
"scripts": {
  "postinstall": "shx ln -fs ../.. node_modules/self"
}

Then I published it and then: npm install some

On Linux, pasting only shx error output:

ln: ENOENT: no such file or directory, symlink '../..' -> '/home/vagrant/node_modules/some/node_modules/self'

On Windows:

ln: ENOENT: no such file or directory, symlink 'C:\Users\M0on75e1l\node_modules' -> 'C:\Users\M0on75e1l\node_modules\some\node_modules\self;'

That's because there is no node_modules/some/node_modules. Since we added shx to dependencies, it will be authomatically installed by npm to node_modules/shx and not to node_modues/some/node_modules/shx, so npm won't even create node_modules/some/node_modules. One possible solution would be to return to previous method with edition:

"devDependencies": {
  "shx": "^0.1.4",
},
"scripts": {
  "postinstall": "npm install shx && shx ln -fs ../.. node_modules/self"
}

And it works on both Linux and Windows. However the symlinc is incorrect. First of all, it seems like when running script with shx, the root folder will be node_modules/some/node_modules instead of node_modules/some. And I wonder why, because when I used my old postinstall script without shx:

"scripts": {
  "postinstall": "ln -s $(pwd) node_modules/self"
}

$(pwd) was exactly package root node_modules/some, and with shx it runs from node_modules/some/node_modules. I wonder if it's correct behaviour. So it's like:

vagrant@php7dev:/home/vagrant/node_modules/some/node_modules> ln -fs ../.. node_modules/self

And the resulting symlinc is two directories up: /home/vagrant/node_modules

The final solution:

"devDependencies": {
  "shx": "^0.1.4",
},
"scripts": {
  "postinstall": "npm install shx && shx ln -fs ../ node_modules/self"
}

And then it works. Maybe I'll play around to make it look better, but it's not in the scope of this issue. Thanks for your help.

levithomason commented 8 years ago

There is no guarantee that your module will be at node_modules/self. NPM 2 and 3 install modules differently. Also if there are any version conflicts between modules, then NPM 3 will also install recursively nested node_modules to support multiple versions of the same module.

In order to find the location of a module, you should use require.resolve('my-module') which will return the absolute path to a node module. When a script becomes more complicated like this, you should pull it into a file.

https://nodejs.org/api/globals.html#globals_require_resolve

nfischer commented 8 years ago

There is no guarantee that your module will be at node_modules/self

@levithomason node_modules/self is a folder he's creating in his postinstall script, not via installing a package dependency. Since npm is out of the picture here, I think it's safe to rely on this behavior.

If shx is a devDependency, it will already be installed by the time you reach postinstall (this is what I observed on Linux).

Sorry, I was thinking of running npm install locally in the git repo (with default configuration). I forgot about the case where you're releasing it to users. Shx should indeed be a regular dep. Also, I doubt the extra npm install shx is really necessary, but it shouldn't be too dangerous to keep it in there for now. But if you can remove it safely, I would recommend it.

"postinstall": "npm install shx && shx ln -fs ../ node_modules/self"

Yeah, I just had ../.. instead of ../. Off-by-one error with the directories, but you get the general idea I was suggesting. I'm glad you were able to figure out a working solution, and hopefully the shx ln behavior is consistent on both Windows and Unix. If you think ln is doing the wrong thing, feel free to file a bug against ShellJS :smile: