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

npm install from master broken #1180

Closed
intensite opened this issue Feb 2, 2016 · 18 comments
Closed

npm install from master broken #1180

intensite opened this issue Feb 2, 2016 · 18 comments

Comments

@intensite
Copy link

Hello,

It appears that the change to remove the lib directory is causing issues when knex is installed from master using npm.

PS C:\projets\test_mssql> npm install
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"
npm ERR! node v4.2.3
npm ERR! npm  v2.14.7
npm ERR! path C:\projets\test_mssql\node_modules\knex\lib\bin\cli.js
npm ERR! code ENOENT
npm ERR! errno -4058
npm ERR! syscall chmod

npm ERR! enoent ENOENT: no such file or directory, chmod 'C:\projets\test_mssql\node_modules\knex\lib\bin\cli.js'
npm ERR! enoent This is most likely not a problem with npm itself
npm ERR! enoent and is related to npm not being able to find a file.
npm ERR! enoent

npm ERR! Please include the following file with any support request:
npm ERR!     C:\projets\test_mssql\npm-debug.log

As you can see this is from a Windows installation. I am using the version from the master repo as I need MSSQL support.

Can this commit be the cause: #1149

@elhigu
Copy link
Member

elhigu commented Feb 3, 2016

This seems to be the problem https://github.com/tgriesser/knex/blob/master/package.json#L59 npm tries to install knex client to .binbefore prepublish script is ran.

@elhigu elhigu added the bug label Feb 3, 2016
@intensite
Copy link
Author

Is there a quick fix for the time being?

@elhigu
Copy link
Member

elhigu commented Feb 3, 2016

You could probably fork the repo and remove the bin attribute from package.json in your own branch. We are resolving if this is problem of npm or should us write work around for it.

@rhys-vdw
Copy link
Member

rhys-vdw commented Feb 6, 2016

Tracking issue: npm/npm#11395.
Thanks @elhigu

@rhys-vdw
Copy link
Member

rhys-vdw commented Feb 6, 2016

So it looks like the build command is not even being run, so it may not be an npm problem after all.

@rhys-vdw
Copy link
Member

rhys-vdw commented Feb 6, 2016

Okay, so I've been looking at this for over three hours and I'm going to leave this for a bit. Seems that prepublish is only called when you explicitly run npm install in the actual knex directory. (eg. /myProject/node_modules/knex.)

We can get build running automatically in postinstall, but definitely don't want this happening when installing from the npm repository, because it will require installing babel.

There does not seem to be a trivial way to do this cleanly.

If we make a proxy cli.js file, as I have here, then we can at least dodge the ENOENT error, which is start. It may even be that postinstall is called before the bin symlink is made (haven't determined yet).

Perhaps it would be acceptable to require users to actually navigate into the node_modules/knex dir and call npm install babel && npm run build if they wish to depend on a specific commit? The assumption here would be that it would just be for testing and that production should always depend on the published package?

Otherwise we could add a postinstall script that first checks for the presence of /lib, then builds the project if it not yet there.

Thoughts?

@elhigu
Copy link
Member

elhigu commented Feb 7, 2016

@rhys-vdw tldr; Agreed.

I think it is valid approach to require people to do some extra steps when installing knex directly from github... After installing from github one has to install also the dev dependencies and build the library in node_modules. Everyone who is doing it may put those steps to their own projects postinstall script if they wish.

To prevent install from github from failing I suppose that the proxy solution is good enough. At least I don't have any better ideas to make it happen.

If someone send us pull request for the automated solution which checks if library has already been built and install dev deps and build it on demand I would accept it too. In either way the proxy cli.js is needed to make installation to work.

@elhigu
Copy link
Member

elhigu commented Feb 7, 2016

@rhys-vdw Actually since this issue doesn't affect to npm release and github install gets fixed right away when fix for this is done... we could make the npm release right away and fix this afterwards?

@intensite
Copy link
Author

I am sorry having apparently opened a can of worms with this issue.
As I said, the only reason for me to install from Master was that the support for MSSQL wasn't in the official npm release (don't know if it available as we speak).

I have been using the master version from github for a while but since the #1149 pull, everything stop working for the reasons you explained above.

As @elhigu mentioned, I really don't mind doing a few extra steps to install dependencies to do my own build when installing knex directly from github. Especially if those steps can be simplified to a few one liners.

Thanks guys!

@elhigu
Copy link
Member

elhigu commented Feb 9, 2016

@intensite Ok, good to know that this is not a show stopper for you. However I believe we should fix at least the error part of copying cli.js before closing the issue. Btw. 0.10.0-rc1 is in npm already so no need to use github anymore.

@vellotis
Copy link
Contributor

Long quiteness while problemous release in npm... 😟

@elhigu
Copy link
Member

elhigu commented Feb 25, 2016

@vellotis npm release is working fine. Problem in this issue is when one is installing e.g. master directly from github instead from npm release.

If you are having right now problem with this, easiest way will be if you fork this repo, remove "bin" attribute from package.json and refer to your branch when you are installing knex directly from github with npm.

If you are into putting a bit more effort to this, pull request is welcome, which moves cli.js proxy somewhere in repo which then requires lib/bin/cli.js and update package.json to refer that always existing cli.js file.

@vellotis
Copy link
Contributor

Strange is that I see the ENOENT issue while installing from npm. https://gist.github.com/vellotis/aafdb17aa63884e5fd3e

No "knex" folder gets created to node_modules folder. I think it is beacuse the install command fails. Any suggestions?

@vellotis
Copy link
Contributor

Another strange thing is that on another environment it works like a charm. Both using using v0.12.7 and npm v2.11.3. I will investigate further...

@vellotis
Copy link
Contributor

Seems that by using "sudo" in my one of the environments causes ENOENT. So it is somekind environment issue. I will call it false alarm. Sorry.

@jbrumwell
Copy link

I am having the same issue;

npm i knex@0.10.0

works but

npm i knex

fails

npm ERR! node v5.0.0
npm ERR! npm  v3.8.0
npm ERR! path /package/node_modules/knex/lib/bin/cli.js

maxcnunes added a commit to maxcnunes/knex that referenced this issue Mar 29, 2016
wolfgang42 added a commit to wolfgang42/knex that referenced this issue Apr 22, 2016
@statyan
Copy link
Contributor

statyan commented Jul 13, 2016

Updated to the latest knex, the problem is still there. Temporarily fix the problem with npm link in knex dir and npm link knex in my local project directory.

mmorgalocabal added a commit to mmorgalocabal/knex that referenced this issue Aug 23, 2016
@jeffreybrowning
Copy link

@statyan you clever beast

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

No branches or pull requests

7 participants