🐛 sha3@1.x produces node-gyp deprecation warnings and/or build error on install

[canterberry]

Reported in #32 for Node.js 12.7.0 on Mac OS X.

Confirmed deprecation warnings on Linux build environment with Node.js 12.4.0, 12.5.0, 12.6.0, and 12.7.0.
Confirmed build failure on Linux build environment with Node.js 12.3.0.

Upgrading to v2.x is not an option for most use cases because the project(s) do not use the sha3 library directly, but as a transitive dependency with a fuzzy version selector for sha3@1.x, for which a forced resolution of sha3@2.x will not resolve.

[canterberry]

Deprecation warnings in one version of Node.js indicate an imminent build error in a subsequent version. In some environments, warnings are treated as errors (e.g: the C compiler -Wall option), so a deprecation warning will already trigger a build error.

Two things should happen:

1. The Travis CI build for this repo should fail if any deprecation warnings are emitted.
2. All current deprecation warnings should be fixed.

💡 Note: Only the latest versions of the currently supported Node.js releases are officially supported by this library. Thus, Node.js 12.3.0 is not supported and is not in scope for this issue.

[canterberry]

One warning appears to be caused by a newly introduced C++17 feature "over-aligned allocation" recently in May 2019:

../src/addon.cpp: In static member function ‘static Nan::NAN_METHOD_RETURN_TYPE SHA3Hash::New(Nan::NAN_METHOD_ARGS_TYPE)’:
../src/addon.cpp:49:23: warning: ‘new’ of type ‘SHA3Hash’ with extended alignment 32 [-Waligned-new=]
    obj = new SHA3Hash();
                       ^
../src/addon.cpp:49:23: note: uses ‘void* operator new(std::size_t)’, which does not have an alignment parameter
../src/addon.cpp:49:23: note: use ‘-faligned-new’ to enable C++17 over-aligned new support

The other appears to be a revision to the V8 API:

../src/addon.cpp: In static member function ‘static void SHA3Hash::Init(Nan::ADDON_REGISTER_FUNCTION_ARGS_TYPE)’:
../src/addon.cpp:83:27: warning: ‘bool v8::Object::Set(v8::Local<v8::Value>, v8::Local<v8::Value>)’ is deprecated: Use maybe version [-Wdeprecated-declarations]
   target->Set(className, f);
                           ^

This is not my area of expertise, so I'd like to solicit the assistance of a seasoned C/C++ and/or Node.js add-ons developer to review and/or fix these warnings.

[canterberry]

The Node.js add-on examples follow the same constructor pattern for which the first (over-aligned new) warning applies. Given this warning is for what seems like a very rudimentary operation (i.e: calling a default constructor of a C++ object with no arguments), I'm surprised to have such difficulty in finding answers to how to address the warning.

[knoxcard2]

I am having this problem as well...

[mistersandman]

The build fails for Node.js version 12.3.0 due to a regression in Node.js itself, which was fixed in 12.3.1 (nodejs/node#27804).

The V8 API warning is due to the deprecation of a function, which was removed in Node.js 13, turning this warning into a build failure for version 13 and onwards. PR #68 provides a fix for this.

The alignment warning points out a bug that is fixed with PR #69.

[canterberry]

Fixed in #68 and #69.

[canterberry]

Published fix in v1.2.4.

[mistersandman]

Thank you for releasing the fixes so quickly! 👌

[cgewecke]

[EDIT - Apologies, this problem turned out to be Trusty related, please ignore.]

Hi @canterberry :)

Am seeing 1.2.4 error on Node 8 & Node 10 in a TravisCI Trusty container during node-gyp rebuild

> sha3@1.2.4 install /home/travis/build/cgewecke/buidler-gas-reporter/node_modules/sha3
> node-gyp rebuild
make: Entering directory `/home/travis/build/cgewecke/buidler-gas-reporter/node_modules/sha3/build'
  CXX(target) Release/obj.target/sha3/src/addon.o
../src/addon.cpp: In constructor ‘SHA3Hash::SHA3Hash()’:
../src/addon.cpp:32:23: error: ‘align’ is not a member of ‘std’
   void* aligned_buf = std::align(std::alignment_of<hashState>::value, sizeof(hashState), buf, buf_size);
                       ^
make: *** [Release/obj.target/sha3/src/addon.o] Error 1
make: Leaving directory `/home/travis/build/cgewecke/buidler-gas-reporter/node_modules/sha3/build'
gyp ERR! build error 
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/home/travis/.nvm/versions/node/v8.16.2/lib/node_modules/npm/node_modules/node-gyp/lib/build.js:262:23)
gyp ERR! stack     at emitTwo (events.js:126:13)
gyp ERR! stack     at ChildProcess.emit (events.js:214:7)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:198:12)
gyp ERR! System Linux 4.4.0-104-generic
gyp ERR! command "/home/travis/.nvm/versions/node/v8.16.2/bin/node" "/home/travis/.nvm/versions/node/v8.16.2/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
gyp ERR! cwd /home/travis/build/cgewecke/buidler-gas-reporter/node_modules/sha3
gyp ERR! node -v v8.16.2
gyp ERR! node-gyp -v v3.8.0
gyp ERR! not ok 
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! sha3@1.2.4 install: `node-gyp rebuild`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the sha3@1.2.4 install script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.
npm ERR! A complete log of this run can be found in:
npm ERR!     /home/travis/.npm/_logs/2019-12-02T03_01_09_153Z-debug.log
The command "npm install" failed and exited with 1 during .

Node 8 example
Node 10 example

[jonathan-meier]

@cgewecke glad you could resolve the issue on your side. The problem is the g++ version (4.8.4) shipped with trusty. Unfortunately, std::align, which I used for a fix in the latest version, is only available starting from g++ version 5.1. Therefore, the latest version of this package can't be easily installed on trusty anymore. @canterberry, please let me know if you want me to fix this. It's easy to implement std::align ourselves.

[canterberry]

The fix in #69 introduced the dependency on std::align, which required #72 to upgrade the Travis CI build (intentionally) from g++ 4.8 to whatever the default version is on the Ubuntu base image for Travis CI's node_js runners (which happens to be 5.2).

@jonathan-meier If you have a fix in mind to get this to work in g++ 4.8 and 5.1+, I'd very much appreciate it!

[canterberry]

@cgewecke Looks like you were able to resolve the issue by removing the dist: trusty in your CI build. Public maintenance has officially ended for Ubuntu 14.04 LTS, although Ubuntu Advantage customers are still in ESM for security updates. I haven't been able to find any numbers on how common 14.04 LTS is in practice in order to inform a decision of how much of a concern this may be to other consumers of this package.

If possible, I would recommend upgrading to v2.x of this package, which has 100% compatibility with v1.x but is a pure JavaScript implementation with none of the build-time issues experienced with the v1.x release line.

[cgewecke]

If possible, I would recommend upgrading to v2.x of this package, which has 100% compatibility with v1.x but is a pure JavaScript implementation with none of the build-time issues experienced with the v1.x release line.

Thanks so much @canterberry, I will pass this along.

[mistersandman]

@canterberry sorry for the confusion, I commented with the wrong Github user. I‘ll prepare a PR that compiles with g++ 4.8.

[canterberry]

@cgewecke et al, this has been fixed in the latest v1.x release, v1.2.6.
Thanks @mistersandman!