This repository has been archived by the owner. It is now read-only.

/tmp files not cleaned up for shrinkwrapped packages #6855

Closed
ianbamforth opened this Issue Dec 5, 2014 · 69 comments

Comments

Projects
None yet
@ianbamforth

ianbamforth commented Dec 5, 2014

It appears that npm install cleans out /tmp only if the package you're installing is not shrinkwrapped.

To test, I created a very basic package (with a single dependency), publish it as 'unwrapped', then wrap (using npm shrinkwrap) and rename as wrapped, and publish again. npm install unwrapped results in a clean /tmp folder. npm install wrapped results in a npm- folder being left in /tmp.

Either the code is wrong (i.e. the /tmp/npm- folder should be deleted) or the documentation needs updating to reflect the behaviour, preferably with an explanation as I can't think why they'd be treated differently!

@smikes

This comment has been minimized.

Show comment
Hide comment
@smikes

smikes Dec 8, 2014

Contributor

Thanks for bringing this up, Ian. I have confirmed that there is a problem, at least with npm@2.1.10. For now if you are running into disk space problems, can you work around it by more aggressively cleaning up $TMPDIR ?

Repro steps:

mkdir bletch; cd bletch; npm init --force
npm install --save nopt
npm pack
npm version patch
npm shrinkwrap
npm pack
cd ..; mkdir bar; cd bar
mkdir tmp tmp-s
TMPDIR=tmp npm i ../bletch/bletch-1.0.0.tgz
TMPDIR=tmp-s npm i ../bletch/bletch-1.0.1.tgz

At this point, both of the last two commands have succeeded so I expect both tmp and tmp-s to be empty. Instead I find

$ find tmp tmp-s
tmp
tmp/npm-90441-6d266403
tmp/npm-90441-6d266403/unpack-c7312112f262
tmp/npm-90441-6d266403/unpack-c7312112f262/package.json
tmp-s
tmp-s/npm-90444-095bb31f
tmp-s/npm-90444-095bb31f/registry.npmjs.org
tmp-s/npm-90444-095bb31f/registry.npmjs.org/abbrev
tmp-s/npm-90444-095bb31f/registry.npmjs.org/abbrev/-
tmp-s/npm-90444-095bb31f/registry.npmjs.org/abbrev/-/abbrev-1.0.5.tgz
tmp-s/npm-90444-095bb31f/registry.npmjs.org/nopt
tmp-s/npm-90444-095bb31f/registry.npmjs.org/nopt/-
tmp-s/npm-90444-095bb31f/registry.npmjs.org/nopt/-/nopt-3.0.1.tgz
tmp-s/npm-90444-095bb31f/unpack-15b21448cfa2
tmp-s/npm-90444-095bb31f/unpack-15b21448cfa2/bletch-1.0.0.tgz
tmp-s/npm-90444-095bb31f/unpack-15b21448cfa2/npm-shrinkwrap.json
tmp-s/npm-90444-095bb31f/unpack-15b21448cfa2/package.json
tmp-s/npm-90444-095bb31f/unpack-600ceb005c34
tmp-s/npm-90444-095bb31f/unpack-600ceb005c34/abbrev.js
tmp-s/npm-90444-095bb31f/unpack-600ceb005c34/CONTRIBUTING.md
tmp-s/npm-90444-095bb31f/unpack-600ceb005c34/LICENSE
tmp-s/npm-90444-095bb31f/unpack-600ceb005c34/package.json
tmp-s/npm-90444-095bb31f/unpack-600ceb005c34/README.md
tmp-s/npm-90444-095bb31f/unpack-600ceb005c34/test.js
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/.npmignore
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/bin
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/bin/nopt.js
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/examples
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/examples/my-program.js
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/lib
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/lib/nopt.js
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/LICENSE
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/package.json
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/README.md
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/test
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/test/basic.js

/cc @rlidwka @othiym23 @iarna

Contributor

smikes commented Dec 8, 2014

Thanks for bringing this up, Ian. I have confirmed that there is a problem, at least with npm@2.1.10. For now if you are running into disk space problems, can you work around it by more aggressively cleaning up $TMPDIR ?

Repro steps:

mkdir bletch; cd bletch; npm init --force
npm install --save nopt
npm pack
npm version patch
npm shrinkwrap
npm pack
cd ..; mkdir bar; cd bar
mkdir tmp tmp-s
TMPDIR=tmp npm i ../bletch/bletch-1.0.0.tgz
TMPDIR=tmp-s npm i ../bletch/bletch-1.0.1.tgz

At this point, both of the last two commands have succeeded so I expect both tmp and tmp-s to be empty. Instead I find

$ find tmp tmp-s
tmp
tmp/npm-90441-6d266403
tmp/npm-90441-6d266403/unpack-c7312112f262
tmp/npm-90441-6d266403/unpack-c7312112f262/package.json
tmp-s
tmp-s/npm-90444-095bb31f
tmp-s/npm-90444-095bb31f/registry.npmjs.org
tmp-s/npm-90444-095bb31f/registry.npmjs.org/abbrev
tmp-s/npm-90444-095bb31f/registry.npmjs.org/abbrev/-
tmp-s/npm-90444-095bb31f/registry.npmjs.org/abbrev/-/abbrev-1.0.5.tgz
tmp-s/npm-90444-095bb31f/registry.npmjs.org/nopt
tmp-s/npm-90444-095bb31f/registry.npmjs.org/nopt/-
tmp-s/npm-90444-095bb31f/registry.npmjs.org/nopt/-/nopt-3.0.1.tgz
tmp-s/npm-90444-095bb31f/unpack-15b21448cfa2
tmp-s/npm-90444-095bb31f/unpack-15b21448cfa2/bletch-1.0.0.tgz
tmp-s/npm-90444-095bb31f/unpack-15b21448cfa2/npm-shrinkwrap.json
tmp-s/npm-90444-095bb31f/unpack-15b21448cfa2/package.json
tmp-s/npm-90444-095bb31f/unpack-600ceb005c34
tmp-s/npm-90444-095bb31f/unpack-600ceb005c34/abbrev.js
tmp-s/npm-90444-095bb31f/unpack-600ceb005c34/CONTRIBUTING.md
tmp-s/npm-90444-095bb31f/unpack-600ceb005c34/LICENSE
tmp-s/npm-90444-095bb31f/unpack-600ceb005c34/package.json
tmp-s/npm-90444-095bb31f/unpack-600ceb005c34/README.md
tmp-s/npm-90444-095bb31f/unpack-600ceb005c34/test.js
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/.npmignore
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/bin
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/bin/nopt.js
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/examples
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/examples/my-program.js
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/lib
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/lib/nopt.js
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/LICENSE
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/package.json
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/README.md
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/test
tmp-s/npm-90444-095bb31f/unpack-70102dc6589f/test/basic.js

/cc @rlidwka @othiym23 @iarna

@ianbamforth

This comment has been minimized.

Show comment
Hide comment
@ianbamforth

ianbamforth Dec 8, 2014

Yep, I have put a cronjob in to tidy up anything matching nmp-* and over an hour old, so it's not holding me back!

ianbamforth commented Dec 8, 2014

Yep, I have put a cronjob in to tidy up anything matching nmp-* and over an hour old, so it's not holding me back!

@andiby

This comment has been minimized.

Show comment
Hide comment
@andiby

andiby Dec 9, 2014

Additionally for "git:" dependencies the complete repository stays in the npm.tmp directory.
On our build server this is currently hundreds of MB garbage per run.
And windows never cleans up the temp directory on its own.
So, please clean up the npm.tmp folder before exit.

andiby commented Dec 9, 2014

Additionally for "git:" dependencies the complete repository stays in the npm.tmp directory.
On our build server this is currently hundreds of MB garbage per run.
And windows never cleans up the temp directory on its own.
So, please clean up the npm.tmp folder before exit.

@avdd

This comment has been minimized.

Show comment
Hide comment
@avdd

avdd Jan 13, 2015

Just a thought: it's fair enough to defer to the user/OS regarding cleaning up tempfiles, but it would be polite to put them in a subdirectory (e.g. /tmp/npm-$USER) so they don't pollute the shared space.

avdd commented Jan 13, 2015

Just a thought: it's fair enough to defer to the user/OS regarding cleaning up tempfiles, but it would be polite to put them in a subdirectory (e.g. /tmp/npm-$USER) so they don't pollute the shared space.

@ianbamforth

This comment has been minimized.

Show comment
Hide comment
@ianbamforth

ianbamforth Jan 13, 2015

avdd - not sure I agree, neither Windows nor Ubuntu will clean these up for you automatically, and in a successful install the tempfiles have no meaning / utility. It's basically littering!

ianbamforth commented Jan 13, 2015

avdd - not sure I agree, neither Windows nor Ubuntu will clean these up for you automatically, and in a successful install the tempfiles have no meaning / utility. It's basically littering!

@peterjmit

This comment has been minimized.

Show comment
Hide comment
@peterjmit

peterjmit Feb 9, 2015

To add a data point, we turned on shrinkwrap.json and within a couple of days our builds started failing. After some investigation we found >15g of files from npm in our /tmp directory, and then we found this issue.

I am not sure I agree with the labelling of this issue as a feature-request - on a reasonable sized app it takes 2/3 days for this to cause a failure, and there is no documentation for shrinkwrap or install that provides a warning or suggested resolution for this failure. As such I had no reasonable chance of avoiding this failure and nor will users in the future.

peterjmit commented Feb 9, 2015

To add a data point, we turned on shrinkwrap.json and within a couple of days our builds started failing. After some investigation we found >15g of files from npm in our /tmp directory, and then we found this issue.

I am not sure I agree with the labelling of this issue as a feature-request - on a reasonable sized app it takes 2/3 days for this to cause a failure, and there is no documentation for shrinkwrap or install that provides a warning or suggested resolution for this failure. As such I had no reasonable chance of avoiding this failure and nor will users in the future.

@andiby

This comment has been minimized.

Show comment
Hide comment
@andiby

andiby Feb 9, 2015

Yes feature-request is wrong.
Either it is a bug that the Temp files are not deleted upon successful exit or the wrong documentation is a bug:
"Temp files are given a unique folder under this root for each run of the program, and are deleted upon successful exit." (npm-folders.md)
Already mentioned in #6853

andiby commented Feb 9, 2015

Yes feature-request is wrong.
Either it is a bug that the Temp files are not deleted upon successful exit or the wrong documentation is a bug:
"Temp files are given a unique folder under this root for each run of the program, and are deleted upon successful exit." (npm-folders.md)
Already mentioned in #6853

@othiym23

This comment has been minimized.

Show comment
Hide comment
@othiym23

othiym23 Feb 9, 2015

Contributor

This isn't specific to npm shrinkwrap -- npm does a lot of stuff in $TMPDIR, and very frequently doesn't clean up after itself. I'm not sure that documentation has ever been correct, but if it ever was, it hasn't been for a long time now. In general, Unixy tools don't clean up after their own usage of /tmp, but most tools don't use tmp as much as npm does. A workaround is to look at using tmpwatch or a similar tool that will automatically clean up /tmp. Either way, this is more a feature-request than a bug, and a patch that adds automatic cleanup would be welcome.

Contributor

othiym23 commented Feb 9, 2015

This isn't specific to npm shrinkwrap -- npm does a lot of stuff in $TMPDIR, and very frequently doesn't clean up after itself. I'm not sure that documentation has ever been correct, but if it ever was, it hasn't been for a long time now. In general, Unixy tools don't clean up after their own usage of /tmp, but most tools don't use tmp as much as npm does. A workaround is to look at using tmpwatch or a similar tool that will automatically clean up /tmp. Either way, this is more a feature-request than a bug, and a patch that adds automatic cleanup would be welcome.

@andiby

This comment has been minimized.

Show comment
Hide comment
@andiby

andiby Feb 9, 2015

I don't find a lot of stuff. There are only three places where npm.tmp is used:

  • lib/cache/add-remote-tarball.js addRemoteTarball() with url as folder
  • lib/cache/add-local-tarball.js addTmpTarball() for unpack-*
  • lib/cache/add-remote-git.js cache() for git-cache-*

andiby commented Feb 9, 2015

I don't find a lot of stuff. There are only three places where npm.tmp is used:

  • lib/cache/add-remote-tarball.js addRemoteTarball() with url as folder
  • lib/cache/add-local-tarball.js addTmpTarball() for unpack-*
  • lib/cache/add-remote-git.js cache() for git-cache-*
@othiym23

This comment has been minimized.

Show comment
Hide comment
@othiym23

othiym23 Feb 9, 2015

Contributor

The thing is, we don't want to remove stuff right when we're done with it, because that will slow down the installation process significantly (and that may not sound like a major consideration when you're talking about a package manager, but npm's performance does matter, especially when doing large installs in build and CI environments). The way to handle this is probably to enqueue all of the things to be removed at process exit and then do it all at once. npm already does something similar for installation rollbacks when npm runs into install problems, but this would be simpler, because it's just a set of calls to rimraf rather than an unbuild step.

Like I said, I don't have time to get to this immediately, but it doesn't seem like a particularly hard problem to solve, and there is a workaround in the meantime.

Contributor

othiym23 commented Feb 9, 2015

The thing is, we don't want to remove stuff right when we're done with it, because that will slow down the installation process significantly (and that may not sound like a major consideration when you're talking about a package manager, but npm's performance does matter, especially when doing large installs in build and CI environments). The way to handle this is probably to enqueue all of the things to be removed at process exit and then do it all at once. npm already does something similar for installation rollbacks when npm runs into install problems, but this would be simpler, because it's just a set of calls to rimraf rather than an unbuild step.

Like I said, I don't have time to get to this immediately, but it doesn't seem like a particularly hard problem to solve, and there is a workaround in the meantime.

@andiby

This comment has been minimized.

Show comment
Hide comment
@andiby

andiby Feb 9, 2015

Just an easy and fast way would be to remove the npm.tmp directory on exit if the exitCode is 0:

function rmdirSync(dir) {
    var list = fs.readdirSync(dir);
    for (var i = 0, l = list.length; i < l; i++) {
        var fileName = list[i];
        if (fileName == "." || fileName == "..")
            continue
        var filePath = path.join(dir, fileName);
        if (fs.statSync(filePath).isDirectory())
            rmdirSync(filePath);
        else
            fs.unlinkSync(filePath);
    }
    fs.rmdirSync(dir);
};
process.on("exit", function (code) {
  if (!code && tmpFolder)
    rmdirSync(npm.tmp);
});

Put it directly in lib/npm.js after the Object.defineProperty(npm, "tmp",...}) and it's fine.

andiby commented Feb 9, 2015

Just an easy and fast way would be to remove the npm.tmp directory on exit if the exitCode is 0:

function rmdirSync(dir) {
    var list = fs.readdirSync(dir);
    for (var i = 0, l = list.length; i < l; i++) {
        var fileName = list[i];
        if (fileName == "." || fileName == "..")
            continue
        var filePath = path.join(dir, fileName);
        if (fs.statSync(filePath).isDirectory())
            rmdirSync(filePath);
        else
            fs.unlinkSync(filePath);
    }
    fs.rmdirSync(dir);
};
process.on("exit", function (code) {
  if (!code && tmpFolder)
    rmdirSync(npm.tmp);
});

Put it directly in lib/npm.js after the Object.defineProperty(npm, "tmp",...}) and it's fine.

@smikes

This comment has been minimized.

Show comment
Hide comment
@smikes

smikes Feb 10, 2015

Contributor

@andiby That will cause npm to block at exit until all the temp directories are cleaned up. Something more like:

process.on("exit", function (code) {
  if (!code && tmpFolder) {
      require('child_process').spawn(path.resolve(__dirname, "node_modules", ".bin", "rimraf"),
                                     [tmpFolder],
                                     { cwd: npm.config.get("tmp"),
                                       detached: true,
                                       stdio: "ignore" })
  }
})
Contributor

smikes commented Feb 10, 2015

@andiby That will cause npm to block at exit until all the temp directories are cleaned up. Something more like:

process.on("exit", function (code) {
  if (!code && tmpFolder) {
      require('child_process').spawn(path.resolve(__dirname, "node_modules", ".bin", "rimraf"),
                                     [tmpFolder],
                                     { cwd: npm.config.get("tmp"),
                                       detached: true,
                                       stdio: "ignore" })
  }
})
@ianbamforth

This comment has been minimized.

Show comment
Hide comment
@ianbamforth

ianbamforth Feb 10, 2015

I’d say that blocking at exit until clean-up is complete could be considered desirable. Clean-up should be considered part of the process, and as such any failure to clean up should result in non-zero exit code, therefore you have to wait for clean-up.

ianbamforth commented Feb 10, 2015

I’d say that blocking at exit until clean-up is complete could be considered desirable. Clean-up should be considered part of the process, and as such any failure to clean up should result in non-zero exit code, therefore you have to wait for clean-up.

@othiym23

This comment has been minimized.

Show comment
Hide comment
@othiym23

othiym23 Feb 11, 2015

Contributor

Depending on the complexity of the install, this could have a major impact on npm's perceived importance, which as a command-line tool, is basically the same as its actual performance. I agree that @smikes's approach is less robust than waiting for deletion to complete and notifying on errors, but it returns users to the prompt and lets them move on to their next tasks faster.

Contributor

othiym23 commented Feb 11, 2015

Depending on the complexity of the install, this could have a major impact on npm's perceived importance, which as a command-line tool, is basically the same as its actual performance. I agree that @smikes's approach is less robust than waiting for deletion to complete and notifying on errors, but it returns users to the prompt and lets them move on to their next tasks faster.

@othiym23 othiym23 added the next-minor label Feb 26, 2015

@othiym23

This comment has been minimized.

Show comment
Hide comment
@othiym23

othiym23 Feb 26, 2015

Contributor

@ianbamforth, after reviewing @smike's candidate PR, #7324, I think I've come around to agreeing with you. If something goes wrong during cleanup, there's no clean way of communicating to users that they might have an issue that needs reviewing unless it's reported right then and there.

But I also think that for systems where administrators have control over temporary directory cleanup, if npm is going to clean up after its own temporary directories, it should be configurable. Something like remove-tmpdir could be set to true by default, so CI processes could run npm install --no-remove-tmpdir if the ops team knows that tmpwatch is on the case. Otherwise, npm would make a single call to rimraf on exit. But be aware that this would have a significant impact on build speed in precisely those cases where it's most important – removing a bunch of git repos and the other detritus from a big, complicated build could take up to a minute on shared hardware like an EC2 instance using EBS.

I think this is worth adding as described above, and have labeled it as such. The trick is that it may be a while before @iarna or I have time to get to this, so if somebody else wants to give it a shot, they would have my gratitude and assistance.

As more general context, @isaacs and I discussed this issue, and we're in agreement that npm has not really changed how much it relies on the temporary directory, it's just that builds have gotten a great deal more complicated over time, and it's now starting to add up.

Contributor

othiym23 commented Feb 26, 2015

@ianbamforth, after reviewing @smike's candidate PR, #7324, I think I've come around to agreeing with you. If something goes wrong during cleanup, there's no clean way of communicating to users that they might have an issue that needs reviewing unless it's reported right then and there.

But I also think that for systems where administrators have control over temporary directory cleanup, if npm is going to clean up after its own temporary directories, it should be configurable. Something like remove-tmpdir could be set to true by default, so CI processes could run npm install --no-remove-tmpdir if the ops team knows that tmpwatch is on the case. Otherwise, npm would make a single call to rimraf on exit. But be aware that this would have a significant impact on build speed in precisely those cases where it's most important – removing a bunch of git repos and the other detritus from a big, complicated build could take up to a minute on shared hardware like an EC2 instance using EBS.

I think this is worth adding as described above, and have labeled it as such. The trick is that it may be a while before @iarna or I have time to get to this, so if somebody else wants to give it a shot, they would have my gratitude and assistance.

As more general context, @isaacs and I discussed this issue, and we're in agreement that npm has not really changed how much it relies on the temporary directory, it's just that builds have gotten a great deal more complicated over time, and it's now starting to add up.

@smikes

This comment has been minimized.

Show comment
Hide comment
@smikes

smikes Apr 2, 2015

Contributor

Bringing this note back over here, so it doesn't get lost in the dead pull-request:

an async rimraf of a /tmp git repo could be fired off as soon as we know we don't need it anymore, i.e., as soon as the git-clone-to-cache completes.

Contributor

smikes commented Apr 2, 2015

Bringing this note back over here, so it doesn't get lost in the dead pull-request:

an async rimraf of a /tmp git repo could be fired off as soon as we know we don't need it anymore, i.e., as soon as the git-clone-to-cache completes.

@nemisj

This comment has been minimized.

Show comment
Hide comment
@nemisj

nemisj Apr 15, 2015

We put an oneliner in the postinstall scripts which will cleanup the folder.

{
    "scripts": {
        "postinstall": "ppid=$(ps -p ${1:-$$} -o ppid=;); ppid=$(echo ${ppid}|tr -d '[[:space:]]'); if [ -z ${npm_config_tmp} ]; then npm_config_tmp=/tmp; fi; rm -rf \"${npm_config_tmp}\"/npm-${ppid}*"
    }
}

https://gist.github.com/nemisj/e27c942439863dfb4375#file-package-json

nemisj commented Apr 15, 2015

We put an oneliner in the postinstall scripts which will cleanup the folder.

{
    "scripts": {
        "postinstall": "ppid=$(ps -p ${1:-$$} -o ppid=;); ppid=$(echo ${ppid}|tr -d '[[:space:]]'); if [ -z ${npm_config_tmp} ]; then npm_config_tmp=/tmp; fi; rm -rf \"${npm_config_tmp}\"/npm-${ppid}*"
    }
}

https://gist.github.com/nemisj/e27c942439863dfb4375#file-package-json

@kleini

This comment has been minimized.

Show comment
Hide comment
@kleini

kleini May 21, 2015

We need to have that immediately fixed because that problem kills our build infrastructure. /tmp directories on build machines grow about 6G per day.

kleini commented May 21, 2015

We need to have that immediately fixed because that problem kills our build infrastructure. /tmp directories on build machines grow about 6G per day.

@nemisj

This comment has been minimized.

Show comment
Hide comment
@nemisj

nemisj commented May 21, 2015

@kleini: https://gist.github.com/nemisj/11f6d01ef9638af283d3 though it works only on *nix machines

@kleini

This comment has been minimized.

Show comment
Hide comment
@kleini

kleini May 21, 2015

@nemisj This would require to modify every project using npm. That is not feasible with about 50 projects each having 5 or more branches.

kleini commented May 21, 2015

@nemisj This would require to modify every project using npm. That is not feasible with about 50 projects each having 5 or more branches.

@nemisj

This comment has been minimized.

Show comment
Hide comment
@nemisj

nemisj May 21, 2015

@kleini, that explains

nemisj commented May 21, 2015

@kleini, that explains

@ianbamforth

This comment has been minimized.

Show comment
Hide comment
@ianbamforth

ianbamforth May 21, 2015

@kleini presumably you're ok using a cronjob to clean up your /tmp folder though? I'm using:

#!/bin/bash

# Removes any folder starting with npm- in the /tmp folder.
sudo find /tmp/ -name 'npm-*' -type d -mmin +10 -exec rm -rf {} +

which removes anything older than 10 minutes that starts with npm-, adjust as required.

ianbamforth commented May 21, 2015

@kleini presumably you're ok using a cronjob to clean up your /tmp folder though? I'm using:

#!/bin/bash

# Removes any folder starting with npm- in the /tmp folder.
sudo find /tmp/ -name 'npm-*' -type d -mmin +10 -exec rm -rf {} +

which removes anything older than 10 minutes that starts with npm-, adjust as required.

@binarykitchen

This comment has been minimized.

Show comment
Hide comment
@binarykitchen

binarykitchen Jun 17, 2015

Killed our test infrastructure too so yes, would be good to get this fixed. I wouldn't label this as a minor issue ...

binarykitchen commented Jun 17, 2015

Killed our test infrastructure too so yes, would be good to get this fixed. I wouldn't label this as a minor issue ...

@othiym23

This comment has been minimized.

Show comment
Hide comment
@othiym23

othiym23 Jun 26, 2015

Contributor

This is at the very least improved in npm@3 (look in the TINY JEWELS section down near the end), so give that a shot and let us know if the beta is working more like how you'd expect.

Contributor

othiym23 commented Jun 26, 2015

This is at the very least improved in npm@3 (look in the TINY JEWELS section down near the end), so give that a shot and let us know if the beta is working more like how you'd expect.

@Yvem

This comment has been minimized.

Show comment
Hide comment
@Yvem

Yvem Feb 16, 2016

@TheSavior I respectfully disagree. In my company, we use node v4 LTS (which uses npm 2) and, of course, shrinkwrap. Right now, we failed at deploying, for our integration server is, once again, full. Because of npm-* left in /tmp. I'm writing a cleanup cron script right now, but it's definitely a serious bug.

Yvem commented Feb 16, 2016

@TheSavior I respectfully disagree. In my company, we use node v4 LTS (which uses npm 2) and, of course, shrinkwrap. Right now, we failed at deploying, for our integration server is, once again, full. Because of npm-* left in /tmp. I'm writing a cleanup cron script right now, but it's definitely a serious bug.

@supersonicclay

This comment has been minimized.

Show comment
Hide comment
@supersonicclay

supersonicclay Mar 5, 2016

Those looking for a workaround on WIndows (using PowerShell):

Get-ChildItem "$env:LOCALAPPDATA\Temp\" -Filter npm* | `
Foreach-Object{

    if ($_.LastWriteTime -lt (Get-Date).AddDays(-1)) {
        Write-Host "Deleting $_..."
        Remove-Item -Recurse -Force $_.FullName
    }
}

This will delete folders/files beginning with npm if they're older than 1 day.

supersonicclay commented Mar 5, 2016

Those looking for a workaround on WIndows (using PowerShell):

Get-ChildItem "$env:LOCALAPPDATA\Temp\" -Filter npm* | `
Foreach-Object{

    if ($_.LastWriteTime -lt (Get-Date).AddDays(-1)) {
        Write-Host "Deleting $_..."
        Remove-Item -Recurse -Force $_.FullName
    }
}

This will delete folders/files beginning with npm if they're older than 1 day.

@ankurp

This comment has been minimized.

Show comment
Hide comment
@ankurp

ankurp Apr 28, 2016

@othiym23 Its not entirely fixed in npm3. It only outputs git cache file in npm tmp folder but does not clean it after npm process ends.

ankurp commented Apr 28, 2016

@othiym23 Its not entirely fixed in npm3. It only outputs git cache file in npm tmp folder but does not clean it after npm process ends.

@ankurp

This comment has been minimized.

Show comment
Hide comment
@ankurp

ankurp Apr 28, 2016

@othiym23 Try installing with a npm-shrinkwrap file containing following node_module pm2@0.14.7

ankurp commented Apr 28, 2016

@othiym23 Try installing with a npm-shrinkwrap file containing following node_module pm2@0.14.7

@paglias

This comment has been minimized.

Show comment
Hide comment
@paglias

paglias Jun 20, 2016

This bug is biting us with npm@3: using shrinkwrap we're unable to deploy to our EC2 instances (c4.2xlarge) while without it deploys are fine.

paglias commented Jun 20, 2016

This bug is biting us with npm@3: using shrinkwrap we're unable to deploy to our EC2 instances (c4.2xlarge) while without it deploys are fine.

maggu2810 added a commit to maggu2810/smarthome that referenced this issue Jun 28, 2016

place npm cache and tmp to the build directory
Related to: npm/npm#6855

Bug: eclipse#1758
Signed-off-by: Markus Rathgeb <maggu2810@gmail.com>

kaikreuzer added a commit to eclipse/smarthome that referenced this issue Jun 28, 2016

place npm cache and tmp to the build directory (#1761)
Related to: npm/npm#6855

Bug: #1758
Signed-off-by: Markus Rathgeb <maggu2810@gmail.com>
@othiym23

This comment has been minimized.

Show comment
Hide comment
@othiym23

othiym23 Oct 5, 2016

Contributor

After looking at the code, the CLI team believes the only remaining place where npm@>=3 leaves behind files in $TMPDIR is when cloning hosted Git repositories. The simplest way to fix this would be to fire off a rimraf(tmpdir, function () { log.debug('updateSubmodules', 'removed', tmpdir) }) (or maybe something slightly less take-no-prisoners) inside the call to addLocal() within updateSubmodules in lib/cache/add-remote-git.js. (This is written specifically to avoid blocking the completion of installs on the rimraf's completion while still ensuring that the files will get removed before the process exits, which is both a little gross but also takes advantage of concurrency in a not-too-invasive way.)

If somebody wants to send us a patch to that effect, we'd be happy to merge it in. Given the other tasks on the team's to-do list, though, we're unlikely to have time to deal with this in the near- to medium-term. (Also, npm@2 is entering maintenance mode, and will not be patched to include this functionality – investigate one of the many OS-level packages for cleaning up temporary directories if you're restricted to using npm@2.)

Because the CLI team isn't going to have time to do this themselves, I'm going to close this feature request. Because we'd be happy to merge a patch to npm's master branch cleaning up Git clones, I'm marking it patch-welcome. Thanks to all for their time, patience, and discussion.

Contributor

othiym23 commented Oct 5, 2016

After looking at the code, the CLI team believes the only remaining place where npm@>=3 leaves behind files in $TMPDIR is when cloning hosted Git repositories. The simplest way to fix this would be to fire off a rimraf(tmpdir, function () { log.debug('updateSubmodules', 'removed', tmpdir) }) (or maybe something slightly less take-no-prisoners) inside the call to addLocal() within updateSubmodules in lib/cache/add-remote-git.js. (This is written specifically to avoid blocking the completion of installs on the rimraf's completion while still ensuring that the files will get removed before the process exits, which is both a little gross but also takes advantage of concurrency in a not-too-invasive way.)

If somebody wants to send us a patch to that effect, we'd be happy to merge it in. Given the other tasks on the team's to-do list, though, we're unlikely to have time to deal with this in the near- to medium-term. (Also, npm@2 is entering maintenance mode, and will not be patched to include this functionality – investigate one of the many OS-level packages for cleaning up temporary directories if you're restricted to using npm@2.)

Because the CLI team isn't going to have time to do this themselves, I'm going to close this feature request. Because we'd be happy to merge a patch to npm's master branch cleaning up Git clones, I'm marking it patch-welcome. Thanks to all for their time, patience, and discussion.

@TheSavior

This comment has been minimized.

Show comment
Hide comment
@TheSavior

TheSavior Oct 5, 2016

@othiym23 If you meant to tag this with patch-welcome, you may have unintentionally tagged this with platform-integration

TheSavior commented Oct 5, 2016

@othiym23 If you meant to tag this with patch-welcome, you may have unintentionally tagged this with platform-integration

@othiym23

This comment has been minimized.

Show comment
Hide comment
@othiym23

othiym23 Oct 5, 2016

Contributor

It already had patch-welcome, and given that there are concerns tied to the operating system and distro, it's very much a platform integration problem.

Contributor

othiym23 commented Oct 5, 2016

It already had patch-welcome, and given that there are concerns tied to the operating system and distro, it's very much a platform integration problem.

@idautocfator

This comment has been minimized.

Show comment
Hide comment
@idautocfator

idautocfator commented Nov 3, 2016

@twexler

This comment has been minimized.

Show comment
Hide comment
@twexler

twexler Nov 3, 2016

+1 Yarn looks cool. Maybe will have a better set of collaborators who care about running large scale systems.

twexler commented Nov 3, 2016

+1 Yarn looks cool. Maybe will have a better set of collaborators who care about running large scale systems.

@jarig

This comment has been minimized.

Show comment
Hide comment
@jarig

jarig Apr 7, 2017

Started killing our infra as well, had to workaround with forceful cleanups...

jarig commented Apr 7, 2017

Started killing our infra as well, had to workaround with forceful cleanups...

@dinomite

This comment has been minimized.

Show comment
Hide comment
@dinomite

dinomite Jun 1, 2017

For anyone whose Jenkins runs on Ubuntu or Debian and it hangs on boot, this is likely why. I have two headless Jenkins instances that rebooted because of power outage and both hung on startup at the line A start job is running for Create Volatile Files and Directories, which is a task that is unfortunately not time limited. The fix is to boot directly into bash and clear /tmp.

The response from NPM maintainers is pretty sad. Playing off this bug as a feature request and saying, "we're too busy, look at our to do list!" is cowardly avoidance of accepting a mistake. Worse, claiming that it would slow things down too much—why don't you use my database that stores everything in /dev/null, it's super fast!

dinomite commented Jun 1, 2017

For anyone whose Jenkins runs on Ubuntu or Debian and it hangs on boot, this is likely why. I have two headless Jenkins instances that rebooted because of power outage and both hung on startup at the line A start job is running for Create Volatile Files and Directories, which is a task that is unfortunately not time limited. The fix is to boot directly into bash and clear /tmp.

The response from NPM maintainers is pretty sad. Playing off this bug as a feature request and saying, "we're too busy, look at our to do list!" is cowardly avoidance of accepting a mistake. Worse, claiming that it would slow things down too much—why don't you use my database that stores everything in /dev/null, it's super fast!

@twifty

This comment has been minimized.

Show comment
Hide comment
@twifty

twifty Jun 5, 2017

Supposedly fixed in npm@3, yet running npm@4.6.1 and my \tmp folder is still full of npm's leftovers.

twifty commented Jun 5, 2017

Supposedly fixed in npm@3, yet running npm@4.6.1 and my \tmp folder is still full of npm's leftovers.

@mcstover

This comment has been minimized.

Show comment
Hide comment
@mcstover

mcstover Sep 1, 2017

Still an issue in NPM5 as well when installing using shrinkwrap.

mcstover commented Sep 1, 2017

Still an issue in NPM5 as well when installing using shrinkwrap.

@diggabyte

This comment has been minimized.

Show comment
Hide comment
@diggabyte

diggabyte Sep 5, 2017

I've confirmed that when using git hosted dependencies in a package.json, the git repo files do NOT get removed (using npm@3.10.10) after package installation.

This contradicts the explanation given here:
#6855 (comment)

There is a workaround if you are the owner of the git hosted package (i.e. your own package or a fork or whatever) that is causing the /tmp files to be left behind, you can add a postinstall script to clean up after itself:
https://gist.github.com/nemisj/11f6d01ef9638af283d3

diggabyte commented Sep 5, 2017

I've confirmed that when using git hosted dependencies in a package.json, the git repo files do NOT get removed (using npm@3.10.10) after package installation.

This contradicts the explanation given here:
#6855 (comment)

There is a workaround if you are the owner of the git hosted package (i.e. your own package or a fork or whatever) that is causing the /tmp files to be left behind, you can add a postinstall script to clean up after itself:
https://gist.github.com/nemisj/11f6d01ef9638af283d3

@kyanh-huynh

This comment has been minimized.

Show comment
Hide comment
@kyanh-huynh

kyanh-huynh Sep 13, 2017

this is disk killer feature in our company. I'm not sure if the installation process and/or the shrinkwrap tool is creating them. "(

kyanh-huynh commented Sep 13, 2017

this is disk killer feature in our company. I'm not sure if the installation process and/or the shrinkwrap tool is creating them. "(

@nemisj

This comment has been minimized.

Show comment
Hide comment
@nemisj

nemisj Sep 27, 2017

@diggabyte it's not nice to steal gists from other people - https://gist.github.com/nemisj/11f6d01ef9638af283d3 :) at least, maybe change content a bit. BTW, there is already a package, which works also in windows - https://www.npmjs.com/package/clean-npm-tmp

nemisj commented Sep 27, 2017

@diggabyte it's not nice to steal gists from other people - https://gist.github.com/nemisj/11f6d01ef9638af283d3 :) at least, maybe change content a bit. BTW, there is already a package, which works also in windows - https://www.npmjs.com/package/clean-npm-tmp

@diggabyte

This comment has been minimized.

Show comment
Hide comment
@diggabyte

diggabyte Sep 27, 2017

@nemisj thanks, hadn't seen that package yet. I'll edit previous post to point to your gist as well

diggabyte commented Sep 27, 2017

@nemisj thanks, hadn't seen that package yet. I'll edit previous post to point to your gist as well

@nemisj

This comment has been minimized.

Show comment
Hide comment
@nemisj

nemisj Sep 27, 2017

@diggabyte , sure, no problem :))

nemisj commented Sep 27, 2017

@diggabyte , sure, no problem :))

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.