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

Update dockerfile to explictly add node-gyp #845

Merged
merged 1 commit into from
Nov 24, 2021

Conversation

HighMileage
Copy link
Contributor

What it does

This may be a known issue or have an undocumented workaround but in following along with DOCKER.md and running docker_build.sh, I was seeing a non-zero exit status with the following output:

Step 17/25 : RUN yarn global add heroku
 ---> Running in ae2440267056                                                                     
yarn global v1.22.10                                
[1/4] Resolving packages...                                                                                          
warning heroku > [email protected]: Debug versions >=3.2.0 <3.2.7 || >=4 <4.3.1 have a low-severity ReDos regression when used in a Node.js environment. It is recommended you upgrade to 3.2.7 or 4.3.1. (https://github.com/visionmedia/debug/issues/797)
warning heroku > @heroku-cli/plugin-git > [email protected]: Debug versions >=3.2.0 <3.2.7 || >=4 <4.3.1 have a low-severity ReDos regression when used in a Node.js environment. It is recommended you upgrade to 3.2.7 or 4.3.1. (https://github.com/visionmedia/debug/issues/797)
warning heroku > [email protected]: Please upgrade  to version 7 or higher.  Older versions may use Math.random() in certain circumstances, which is known to be problematic.  See https://v8.dev/blog/math-random for details.
warning heroku > @heroku-cli/plugin-pg-v5 > [email protected]: Renamed to `strip-final-newline` to better represent its functionality.
warning heroku > @heroku-cli/plugin-ps-exec > heroku-exec-util > [email protected]: Please upgrade  to version 7 or higher.  Older versions may use Math.random() in certain circumstances, which is known to be problematic.  See https://v8.dev/blog/math-random for details.
[2/4] Fetching packages...                                                                                                                                                                                                                                                                                                                                                                                                                                                           
[3/4] Linking dependencies...           
[4/4] Building fresh packages...       
info This package requires node-gyp, which is not currently installed. Yarn will attempt to automatically install it. If this fails, you can run "yarn global add node-gyp" to manually install it.                                       
[1/4] Resolving packages...                                    
[2/4] Fetching packages...     
[3/4] Linking dependencies...                                                                                                                                                                                                             
[4/4] Building fresh packages...                               
success Installed "[email protected]" with binaries:                                                 
      - node-gyp                                                                                                                                                                                                                          
info This module is OPTIONAL, you can safely ignore this error
warning Error running install script for optional dependency: "/usr/local/share/.config/yarn/global/node_modules/cpu-features: Couldn't find the binary node-gyp rebuild"
info Visit https://yarnpkg.com/en/docs/cli/global for documentation about this command.
error /usr/local/share/.config/yarn/global/node_modules/tunnel-ssh/node_modules/ssh2: Couldn't find the binary node install.js
ERROR: Service 'web' failed to build: The command '/bin/sh -c yarn global add heroku' returned a non-zero code: 1

It looks like explicitly updating the Dockerfile to add this node-gyp dependency fixes this error.

Why it is important

It's nice to meet developers where they are at. Fixing this script ensure that developers can choose to develop dockerized or not 🎉

UI Change Screenshot

Not applicable

Implementation notes

I was able to find others talking about similar issues that required doing a yarn global add node-gyp -- this issue is a nice inroad into lots of similar complaints. I'd personally like to understand the root of this a bit more. There was some talk about this being a concurrency problem but adding CHILD_CONCURRENCY=1 to the yarn global add heroku produced the same error state. Since following the suggestion of the command output itself fixes this, I'm willing to just close my eyes and move on. Very happy to take feedback that this needs more investigation.

Your bandwidth for additional changes to this PR

Please choose one of the following to help the project maintainers provide the appropriate level of support:

  • I have the time and interest to make additional changes to this PR based on feedback.
  • I am interested in feedback but don't need to make the changes myself.
  • I don't have time or interest in making additional changes to this work.
  • Other or not sure (please describe):

docker_build.sh was failing to automatically add this missing node-gyp
dependency and recommends installing it manually
Copy link
Member

@jim jim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fixed the node-gyp issue for me under Ubunto on WSL.

@jim jim merged commit e4ecd8c into chicago-tool-library:main Nov 24, 2021
@HighMileage HighMileage deleted the fix-docker-build-script branch November 27, 2021 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants