Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

ENOENT and ENOTEMPTY while rolling back failed optional dependencies #6043

Closed
othiym23 opened this issue Aug 26, 2014 · 31 comments
Closed

ENOENT and ENOTEMPTY while rolling back failed optional dependencies #6043

othiym23 opened this issue Aug 26, 2014 · 31 comments
Assignees
Labels
Milestone

Comments

@othiym23
Copy link
Contributor

npm install grunt-imagemin fails spectacularly on both master and v1.4 due to npm sometimes simultaneously trying to install and remove packages when optional dependencies fail to install. That makes me sad.

See imagemin/imagemin#53 for more details.

@ivanbrykov
Copy link

I'm suffering from these errors for several days already trying to install gulp-imagemin

@gabegorelick
Copy link

Is there an ETA on a fix? I see it's in progress.

@kole
Copy link

kole commented Sep 4, 2014

+1 on the ETA request

@othiym23
Copy link
Contributor Author

othiym23 commented Sep 4, 2014

This will be fixed before [email protected] is released, as it's the primary blocker for a 2.0 release. I would like to land it today, but the fix is relatively complex and because this bug is a regression caused by another fix, I want to make sure that it's not causing regressions itself. So, it'll be done when it's done?

@vladikoff
Copy link

Please backport the fix to 1.4.

isaacs added a commit that referenced this issue Sep 10, 2014
This ought to nail the ENOENT issues we're seeing in #6043,
albeit in a somewhat kludgey way.

Needs tests, ideally, but this is a tricky one to test for.
isaacs added a commit that referenced this issue Sep 12, 2014
This ought to nail the ENOENT issues we're seeing in #6043,
albeit in a somewhat kludgey way.

Needs tests, ideally, but this is a tricky one to test for.
isaacs added a commit that referenced this issue Sep 13, 2014
This ought to nail the ENOENT issues we're seeing in #6043 by deferring
all rollback unbuilds to the end of the install process. This also
depends on the upgrade to [email protected].

Includes a test with a very racy optional dependency situation. (*wink*)
isaacs added a commit that referenced this issue Sep 13, 2014
This ought to nail the ENOENT issues we're seeing in #6043 by deferring
all rollback unbuilds to the end of the install process. This also
depends on the upgrade to [email protected].

The test lives on master, but wasn't ported to this branch.
@othiym23
Copy link
Contributor Author

@isaacs and I have addressed this issue two ways:

  1. We changed slide's asyncMap function to only continue on to its final callback when all of the map calls have completed. It used to invoke the callback as soon as it had an error, which caused bad things to happen when the callback had side effects (like wiping out module directories) that other callbacks, that hadn't finished running yet, could be affected by.
  2. All rollbacks are now queued until the npm CLI process is ready to exit, at which point those directories are unbuilt. This also helps prevent race conditions when rolling back failed installations of optionalDependencies.

These changes are included in [email protected], which is currently labeled npm@next. Unless we find any showstoppers, it will be set to latest next Thursday. As requested, these two fixes have also been backported to the v1.4 branch, and are included in [email protected] (even though that version probably will not ever be published as latest).

Because [email protected] and [email protected] are not tagged as the latest releases of npm (yet), you will need to install them explicitly to test this fix:

npm -g install [email protected]

OR

npm -g install [email protected]

Please do so, and let us know if you're still having problems with this issue. We are reasonably confident we've put a stake through this one's heart, but if you have evidence otherwise, we want to know about it sooner rather than later.

@budmc29
Copy link

budmc29 commented Sep 24, 2014

Hello, i installed yeoman-webapp today and when i run grunt serve, grunt test or grunt it still displays the error:
Loading "imagemin.js" task...ERROR

Error: Cannot find module 'imagemin-gifsicle'.

npm --version is 1.4.28

I tried all the commands i see on #6043 and imagemin/imagemin#53.
I am new to grunt, so please excuse me if i don't know how to explain better. I took the steps on the yeoman getting started page without modifying anything.

@othiym23
Copy link
Contributor Author

@Chirica-Mugurel The issue you're running into is actually fixed by the resolution to #5920. To get the fix, you'll need to upgrade to [email protected]: npm install -g [email protected]. See #5920 for details on that issue.

@kuhnroyal
Copy link

Still having that problem.
npm_problem

@othiym23
Copy link
Contributor Author

@kuhnroyal npm is doing what it's supposed to in this case – the optional dependencies failed to install, it warned you, and then it finished running without crashing.

Do you have a C compiler installed? All of imagemin's optional dependencies require developer tools to be installed.

Also, pasting a screenshot is the least helpful way to get support. It cuts off useful information and isn't searchable (not to mention being hard to read).

@kuhnroyal
Copy link

Right, i'll write it next time.
GCC is installed, should be sufficient?

@othiym23
Copy link
Contributor Author

@kuhnroyal That's an issue for the imagemin team. I'm just pointing out that npm is doing its job and you're not encountering the issue the others were.

@kevva
Copy link

kevva commented Oct 18, 2014

@othiym23, they don't require anything. Only if the pre-built binary doesn't work. Then it'll try and compile if from source.

I'm seeing (but don't experiencing it myself) a lot of issues regarding the optional deps. It comes down (I think) to npm running the postinstall scripts before all the dependencies are actually installed, and therefore errors are happening (see the npm install stuff in https://travis-ci.org/google/web-starter-kit/builds/38239448).

Is this fixed in 2.1.5? It seems like it's still happening in 2.1.0.

@othiym23
Copy link
Contributor Author

@kevva If you look at the release notes for 2.1.x release, you'll see that all of

include patches for race conditions. I don't believe that we've eliminated all of the race conditions within npm, but I do believe we've fixed the ones most developers are ever likely to hit, including all of the problems that were causing problems with grunt-contrib-imagemin.

The bugs were due to lock contention, packages being written to and read from the cache non-atomically, and peerDependencies causing multiple build phases to be run for the same package at the same time during a single install, which sometimes led to the ENOENT issues seen here. One of @dylang's coworkers may be seeing a lingering issue related to this, but at this point I've been over the relevant cache and installation code so closely that it's hard for me to imagine that any serious problems could be remaining. There is very little locking being done by npm anymore, and we've taken great pains to avoid allowing the installer to do the same thing at the same time.

@iarna is also reworking the installer's internals right now to make the installer run in discrete stages, in a way that should be more deterministic and less liable to lead to the kinds of race conditions we've spent so much time on lately. It's our hope that this will help us tackle a whole host of issues at once.

@kevva
Copy link

kevva commented Oct 21, 2014

@othiym23, yo, thanks for your answer. Yeah, I know a lot of work were done to fix this, but it seems to fail so randomly at random versions. A few days ago this happened during Travis builds (using npm 1.4.28) but somehow that magically got fixed.

Now I'm seeing issues being created where their logs just say npm WARN optional dep failed, continuing [email protected] without even running the postinstall script from the looks of it.

I feel kinda bad for just having to advice people to update npm etc lol, but without any real errors there's really not much else I can do.

@kevva
Copy link

kevva commented Oct 21, 2014

Ok, so we managed to track down the issue to a deeply nested dependency relying on a git remote URL which caused npm to fail because of EACCESS errors in ~.npm/_git-remotes/_templates. Not related to this issue so ignore my previous comment.

@othiym23
Copy link
Contributor Author

@kevva what version of npm did you encounter that with? I recently landed a change that disabled templates altogether, so if you're seeing issues in you git repos, we should take another look at that.

@kevva
Copy link

kevva commented Oct 22, 2014

@othiym23, I didn't encounter it but it happened with 2.1.4 and 2.1.5 (because of this). I'm also using 2.1.5 and it's not happening for me.

@addyosmani
Copy link

To add to @kevva's comments, we're starting to run into an increasing numbers of folks being bitten by this but can't reproduce it reliably. e.g 2.1.5 is working fine for me, but on the issue linked above Paul (and outside of that thread, others) are not seeing that build address the problem.

@othiym23
Copy link
Contributor Author

My guess is this is a permissions issue caused by running sudo npm. We set the permissions on $HOME/.npm correctly when run via sudo, but we probably need to do the same when we're creating _templates. A simple workaround is just to chown $(whoami) $HOME/.npm/_git-remotes/_templates, but I'll file a bug to add a test / permissions fixup for the templates directory as well.

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

No branches or pull requests

9 participants