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

node_modules should be rebuilt if version of node changes between builds #153

Closed
3 tasks done
fg-j opened this issue Nov 18, 2020 · 3 comments · Fixed by #219
Closed
3 tasks done

node_modules should be rebuilt if version of node changes between builds #153

fg-j opened this issue Nov 18, 2020 · 3 comments · Fixed by #219
Assignees
Labels
bug Something isn't working status/prioritized This issue has been triaged and resolving it is a priority

Comments

@fg-j
Copy link

fg-j commented Nov 18, 2020

What happened?

Similar to paketo-buildpacks/yarn-install#144, buildpack reuses cached version of node_modules even when version of node has changed between builds. For apps with native extensions, this can cause failures at runtime though the build succeeds.

Reproduction Steps

Using

 ‣ pack --version
0.13.1+git-4134cc6.build-1135
  1. Clone this sample app git@github.com:fg-j/native-extensions-sample.git
  2. Build the app (and force the cache to be cleared) pack build with-bcrypt -b gcr.io/paketo-buildpacks/nodejs:0.0.10 --builder paketobuildpacks/builder:0.1.20-base --clear-cache
  3. Run the app docker run -d --env PORT=8080 --publish 8080:8080 with-bcrypt
  4. Curl the app endpoint curl 0.0.0.0:8080. Expected output is: Hello, World!
  5. Update the desired version of node by changing buildpack.yml to:
---
nodejs:
  version: ^14.0.0
  1. Rebuild the app (but don't force the cache to be cleared) pack build with-bcrypt -b gcr.io/paketo-buildpacks/nodejs:0.0.10 --builder paketobuildpacks/builder:0.1.20-base The output should contain:
[builder] Paketo Node Engine Buildpack 0.1.2
[builder]   Resolving Node Engine version
[builder]     Candidate version sources (in priority order):
[builder]       buildpack.yml -> "^14.0.0"
[builder]                     -> ""
[builder]       <unknown>     -> "*"
[builder]
[builder]     Selected Node Engine version (using buildpack.yml): 14.13.0
...
...
...
[builder] Paketo NPM Install Buildpack 0.2.1
[builder]   Resolving installation process
[builder]     Process inputs:
[builder]       node_modules      -> "Not found"
[builder]       npm-cache         -> "Not found"
[builder]       package-lock.json -> "Found"
[builder]
[builder]     Selected NPM build process: 'npm ci'
[builder]
[builder]   Reusing cached layer /layers/paketo-buildpacks_npm-install/modules

This indicates that the cached version of the node_modules has been reused, rather than rebuilding for the new node version.

  1. Stop the previously running instance of the app (e.g. docker stop 83e251bd0c449a7fbfea20b742ffd5b4b5dbf885307e3c26da0a0c8d7a647254)
  2. Run the newly built version of the app: docker run -d --env PORT=8080 --publish 8080:8080 with-bcrypt
  3. Curl the app endpoint curl 0.0.0.0:8080. Desired output is: Hello, World! but actual output is:
curl: (7) Failed to connect to 0.0.0.0 port 8080: Connection refused
  1. Get logs for the container (e.g. docker logs f025c0b9c318) Expected output:
internal/modules/cjs/loader.js:1144
  return process.dlopen(module, path.toNamespacedPath(filename));
                 ^

Error: The module '/layers/paketo-buildpacks_npm-install/modules/node_modules/bcrypt/lib/binding/bcrypt_lib.node'
was compiled against a different Node.js version using
NODE_MODULE_VERSION 72. This version of Node.js requires
NODE_MODULE_VERSION 83. Please try re-compiling or re-installing
the module (for instance, using `npm rebuild` or `npm install`).
    at Object.Module._extensions..node (internal/modules/cjs/loader.js:1144:18)
    at Module.load (internal/modules/cjs/loader.js:950:32)
    at Function.Module._load (internal/modules/cjs/loader.js:791:14)
    at Module.require (internal/modules/cjs/loader.js:974:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at Object.<anonymous> (/layers/paketo-buildpacks_npm-install/modules/node_modules/bcrypt/bcrypt.js:6:16)
    at Module._compile (internal/modules/cjs/loader.js:1085:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1114:10)
    at Module.load (internal/modules/cjs/loader.js:950:32)
    at Function.Module._load (internal/modules/cjs/loader.js:791:14)

This shows that the app fails to run as expected because bcrypt was compiled with a different version of node than the one that is installed in the app container.

Checklist

Please confirm the following:

  • I have included log output.
  • The log output includes an error message.
  • I have included steps for reproduction.
@ForestEckhardt
Copy link
Contributor

I was casually checking this out and found the npm config list command as a way to potentially conveniently cache the NPM configuration, which includes the node version in some way.

@arjun024 arjun024 added the bug Something isn't working label Mar 15, 2021
@arjun024 arjun024 added the status/possible-priority This issue is ready to work and should be considered as a potential priority label Apr 1, 2021
@fg-j fg-j added status/prioritized This issue has been triaged and resolving it is a priority and removed status/possible-priority This issue is ready to work and should be considered as a potential priority labels Apr 2, 2021
@andymoe
Copy link
Member

andymoe commented Apr 20, 2021

I'll give this a go.

@andymoe
Copy link
Member

andymoe commented May 5, 2021

This just took way longer to get to than I wanted. I'm out the rest of the week and there are still unit tests to fix (unfortunately we need some magic numbers right now) I'm not going to get to them before I leave tomorrow morning. Happy to finish up when I get back but if anyone is itching to get this done here is the brach that needs attention (and squashed): https://github.com/andymoe/npm-install/tree/rebuild-on-node-version-change_153

andymoe pushed a commit to andymoe/npm-install that referenced this issue May 11, 2021
fixes paketo-buildpacksgh-153

Signed-off-by: Arjun Sreedharan <asreedharan@vmware.com>
andymoe pushed a commit to andymoe/npm-install that referenced this issue May 17, 2021
fixes paketo-buildpacksgh-153

Signed-off-by: Arjun Sreedharan <asreedharan@vmware.com>
andymoe pushed a commit to andymoe/npm-install that referenced this issue May 17, 2021
- Address feedback and switch to using npm get user-agent
  instead of npm config list to determine if node version
  has changed and layer should be rebuilt. npm config list
  can be unreliable due to messages re: availabity of new
  npm versions in the output and pottentially other values
  output affected by the local environment.

paketo-buildpacksgh-153
thitch97 added a commit that referenced this issue May 18, 2021
* Rebuild layer if npm config/node version changes

fixes gh-153

Signed-off-by: Arjun Sreedharan <asreedharan@vmware.com>

* call npm get-user-agent to ensure correct layer rebuild

- Address feedback and switch to using npm get user-agent
  instead of npm config list to determine if node version
  has changed and layer should be rebuilt. npm config list
  can be unreliable due to messages re: availabity of new
  npm versions in the output and pottentially other values
  output affected by the local environment.

gh-153

Co-authored-by: Tim Hitchener <thitch97@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working status/prioritized This issue has been triaged and resolving it is a priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants