heroku / buildpacks-nodejs

Heroku's Cloud Native Buildpacks for Node.js applications.
BSD 3-Clause "New" or "Revised" License
2 stars 2 forks source link

Problem if node_modules is mounted as a volume #112

Open sawmurai opened 3 years ago

sawmurai commented 3 years ago

If node_modules is mounted as a volume during the build process, the build process breaks because the build script is trying to remove the node_modules folder.

Skipping 'restore' due to clearing cache
===> BUILDING
[builder] [INFO] Node.js Buildpack
[builder] rm: cannot remove 'node_modules': Device or resource busy
[builder] ERROR: failed to build: exit status 1
ERROR: failed to build: executing lifecycle. This may be the result of using an untrusted builder: failed with status code: 145
💥 Build failed

The responsible line is this one:

https://github.com/heroku/buildpacks-nodejs/blob/46dec4f3d9be21d320b6bb912061f1b7a0c63c05/buildpacks/yarn/bin/build#L15

Would it be possible to change that line to rm -rf "$build_dir/node_modules/*" or would the presence of an empty folder break succeeding code? Happy to provide a PR if that would be a feasible solution.

Edit: I guess the same goes for this line:

https://github.com/heroku/buildpacks-nodejs/blob/46dec4f3d9be21d320b6bb912061f1b7a0c63c05/buildpacks/npm/bin/build#L16

joshwlewis commented 3 years ago

When node_modules gets fairly large, it's possible that rm -rf "$build_dir/node_modules/*" gets expanded to a massive command and hits Argument list too long errors. So, I'm not a fan of this exact solution. I think an empty node_modules would serve the same purpose as it not existing, though.

What is driving you to mount node_modules? Is this so you don't have to reinstall modules you've already installed locally?

sawmurai commented 3 years ago

Oh, I wasn't aware of the expansion problem, thanks for pointing that out. The reason we want to keep the node_modules folder is indeed that we do not want to reinstall everything. But maybe that is necessary.