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 node.js dependencies to the latest versions to pull in newer keccak module #31

Merged
merged 1 commit into from
Jul 8, 2020

Conversation

cameel
Copy link
Member

@cameel cameel commented Jul 4, 2020

This resolves the issue with ethereumjs-util pulling in an old version of keccak version that does not build on node.js >= 12. The problem was not making npm install fail because there's a fallback to the pure JS implementation but it ran slower because of that. After the update the script runs in ~5 minutes in a github action, compared to ~8 minutes before.

npm install output before this PR

> keccak@1.4.0 rebuild /home/runner/work/solc-bin/solc-bin/node_modules/keccak
> node-gyp rebuild

make: Entering directory '/home/runner/work/solc-bin/solc-bin/node_modules/keccak/build'
  CXX(target) Release/obj.target/keccak/src/addon.o
../src/addon.cc: In static member function ‘static Nan::NAN_METHOD_RETURN_TYPE KeccakWrapper::Initialize(Nan::NAN_METHOD_ARGS_TYPE)’:
../src/addon.cc:37:47: error: no matching function for call to ‘v8::Value::IntegerValue()’
     unsigned int rate = info[0]->IntegerValue();
                                               ^

(...)

make: *** [Release/obj.target/keccak/src/addon.o] Error 1
keccak.target.mk:129: recipe for target 'Release/obj.target/keccak/src/addon.o' failed
make: Leaving directory '/home/runner/work/solc-bin/solc-bin/node_modules/keccak/build'
gyp ERR! build error 
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/usr/local/lib/node_modules/npm/node_modules/node-gyp/lib/build.js:194:23)
gyp ERR! stack     at ChildProcess.emit (events.js:315:20)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:275:12)
gyp ERR! System Linux 5.3.0-1031-azure
gyp ERR! command "/usr/local/bin/node" "/usr/local/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
gyp ERR! cwd /home/runner/work/solc-bin/solc-bin/node_modules/keccak
gyp ERR! node -v v12.18.1
gyp ERR! node-gyp -v v5.1.0
gyp ERR! not ok 
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! keccak@1.4.0 rebuild: `node-gyp rebuild`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the keccak@1.4.0 rebuild script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm WARN Local package.json exists, but node_modules missing, did you mean to install?

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/runner/.npm/_logs/2020-07-03T22_17_39_459Z-debug.log
Keccak bindings compilation fail. Pure JS implementation will be used.

npm install output after this PR

> keccak@3.0.0 install /home/runner/work/solc-bin/solc-bin/node_modules/keccak
> node-gyp-build || exit 0

@cameel cameel requested review from axic, ekpyron and chriseth July 4, 2020 00:34
@chriseth
Copy link
Contributor

chriseth commented Jul 6, 2020

This creates a nonzero diff in soljson_latest.js

@cameel
Copy link
Member Author

cameel commented Jul 6, 2020

Yeah, it's likely the same old issue with files being truncated by the update script. It was happening in #28 too - it passed there after retriggering the checks twice. I'm working on fixing that script now so, we can wait and see if it fixing it also makes the checks pass here.

BTW, note that it is a PR to the master branch that I created recently (on the same commit where gh-pages is) so merging it won't affect the files that are served to tools. Please let me know if I should be submitting PRs to gh-pages instead.

@cameel
Copy link
Member Author

cameel commented Jul 7, 2020

Rebased on master. The checks should be passing now that #32 got merged.

I bumped the version of standard a bit more which also fixes the warnings mentioned in #33 (comment).

@cameel cameel changed the base branch from master to gh-pages July 7, 2020 10:11
@cameel
Copy link
Member Author

cameel commented Jul 7, 2020

Changing the target branch to gh-pages.

@cameel cameel self-assigned this Jul 7, 2020
- This fixes the problem with keccak module failing to build its native bindings and falling back to pure JS implementation
@chriseth chriseth merged commit 93f4e51 into gh-pages Jul 8, 2020
@cameel cameel deleted the update-dependencies branch July 8, 2020 17:36
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