-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
[DOCS] npm install does not run scripts in directory it was ran in on npm v7+ #3123
Comments
You’re using npm 6 on windows, what happens if you use npm 7 on windows? |
Aha, this seems to be an NPM issue somewhere in v7 as it also points to the temp cache directory and fails using npm 7.10.0 on Windows. I've updated the original issue to reflect this. |
In that case, it might be an unintended regression in npm 7. |
I just downgraded npm to 6.14.13 on macOS and it worked, so it does seem to be a regression in npm. |
Especially bad because npm 7.10.0 now is delivered by default "with" nodejs 16 |
@Apollon77 npm 7 is there by default in node 15 as well, and since node 16 isn't LTS, that means it's no worse than it was prior to 16 being released. |
Yea and no ;-)) nodejs 15 is a development version by definition in my eyes, so more acceptable because that Version habe a mich lower adoption rate. In the eyes of users NodeJs 16 will "become LTS", so they have an other view on this version and start adopt to that version faster. But yes you are right, we formally have some time until nodejs 16 goes LTS official. |
How are you adding the dependency in step 5? As a git dep, tarball, published semver version/range, etc? |
@isaacs Sorry for the delay, I am adding the dependency by using a git dep. |
I have a very esy example:
This is trying to execute a "setup command". This all wors file if installed from npm (npm i iobroker.js-controller), but fails when istalling the same from github |
|
is there any status updte on this one? |
@darcyclarke if this is classified as "not a regression", is there another recommended way for pre/post-install scripts to know the path to the directory that it was run in ? by "Investigate further into whether this is a regression" do you want more example logs, or are you thinking about something else ? |
This regression seems to have been introduced by npm/config#12 while fixing another related regression it went from not enough INIT_CWD reset, to too much INIT_CWD reset |
nodejs 16 becomes LTS very soon and this issue is still not fixed :-( Any ETA on it? |
The errors I'm seeing installing ioBroker from git currently (ie, using the latest npm v8) do not appear to be related to this issue. It still isn't working, but for different reasons, which appear to be related to the way in which its many internal link dependencies are being built. There's still something wrong here, and it's a bug we'll fix whether node 16 is LTS or not, but it's not this bug. (I'll post a new issue for that right now.) Also, if we're handling INIT_CWD improperly, we'll fix that as well, LTS or not. It would not be a semver-major change, as far as I can tell. If anyone has a repro case they can share that shows improper INIT_CWD handling, please share it, and we'll certainly reopen this. |
@isaacs thank you for checking. In fact only projects (and not only about iobroker) are affected that have a postinstall script which is executed ... because these projects with get the above error when being installed from GitHub! It is only in this case ... but this is still a "ocmmon way" to deliver dev-versions to users for certain fixes without the need to publish prereleases to npm for any single change |
Current issue affecting ioBroker: #3918 (maybe related? but seems like it isn't, since the dep gets placed in node_modules, but then breaks later, when building all its local link dependencies.) @Apollon77 Do you have a repo that demonstrates the issue? If it is just a matter of when and how consistently we set INIT_CWD, as @nmss suggests, then it should be straightforward to comb through it and fix, once we have a repro case. |
try npm i https://github.com/ioBroker/ioBroker.js-controller#3.3.x
PS: This is the branch of the current stable version of the project ... In future it is a monorepo and so it is not thaat relevant, but I think it would be just a matter of time until someone else uses a "install" script |
@Apollon77 I get the same error just running
It looks like the issue is that it's inferring its module name from the project folder basename, so if that doesn't match, it's going to always fail, because the conf file isn't there. In my case, it's expecting to find I'd recommend setting |
Yes thats basically true ... in fact it was working all the time till npm 7 :-) But honestly ... ok for me. I tested around other cases with install scripts and they all work with npm 8.1.0 ... so for me it is ok - also because we will change to monorepo and so this exact topic is no longer relevant. Thank you for your support. I leave it up to you if you close here or leave open to maybe update documentations because the expectation from before that the "dirname to get is the one where the npm command was executed" is now no longer true and it might be a "tmp dir somewhere" could be worth documenting somewhere |
PR that copies $INIT_CWD explanation from |
I've run into this issue with NPM 9.5.1 when installing a dependency from a GitHub repo that is large enough that the I'm not sure what the best solution would be, but it seems like paying attention to whether INIT_CWD is already defined prior to defining it could make the situation better with very little code changes. |
@jtiee if you have a reproducible case please open a new issue |
Current Behavior:
npm install
runs in a temp cache directory (somewhere inUsers/user/.npm/_cacache/tmp/my_dep/
) when installing dependencies onmacOSnpm 7.10.0. This causes errors when your pre/post-install scripts relies on checking for certain files/folders in the directory the command was ran in. However, on Windows, it does run them in the current directory.Expected Behavior:
npm install
should run the scripts in the directory that it was ran in,like Windowslike it was on npm 6, or provide some way to retrieve this directory. Note that theINIT_CWD
and other env variables do not point to this directory, but rather the cache directory.Steps To Reproduce:
scripts
sectionpre.js
, with the contentsnpm install
.Environment:
macOS:
Windows:
The text was updated successfully, but these errors were encountered: