Ship prebuilt binaries for multiple platforms?

[spalger]

Would y'all be interested in using something like prebuildify to ship native modules for all platforms within the npm package, rather than relying on developers to build the native modules locally? We are looking to eliminate native module builds as much as possible from our project and it seems like using something like prebuildify is quick for authors and users, and should integrate well this this lmdb-store since it's already using a modern node-gyp-build version.

Thoughts?

[kriszyp]

lmdb-store is using prebuildify (you should see a prebuilds directory in the npm distributions). Unfortunately it is presently broken for the current node releases due to https://github.com/prebuild/prebuildify/issues/43 due to https://github.com/lgeiger/node-abi/issues/90, with a fix in https://github.com/lgeiger/node-abi/issues/90 that hopefully will be merged soon.

I had also omitted mac/darwin builds for the time being because I was looking into the distinction between building LMDB with posix semaphores (not process-robust) vs system v semaphores (limit of 10), but probably will return to just building that as well.

Also, I can workaround the broken abi versions by manually renaming files before an npm publish, if you need it srtl.

[spalger]

Oh, that's awesome! Didn't see it in package.json so I assumed it wasn't being used, but I don't know much about how it works yet.

We don't need it right away so no rush, thanks again :)

[kriszyp]

FWIW, node-gyp-build is the package that runs on the user side for prebuildify. I just have to install prebuildify on my computer as a global package (and anyone else that would create releases) to generate the builds .

[mistic]

@kriszyp https://github.com/lgeiger/node-abi/issues/90 was closed. Do you think we are already in a position where we can re-enable prebuildify builds again? Also do you think we could enable them both for node v12 and v14 at least for the current release as of today both are still Active LTS?

[kriszyp]

@mistic Yes, this will help, although I had already setup some manual file renaming scripts locally to create the right file names for prebuilds that I publish. I believe the last few lmdb-store releases should already have prebuilds of v12, v14 (and v15) with the right abi-version/filename for Windows, Mac, and Linux. Let me know if they aren't working for you...

However, LMDB also has upgraded database formats (incompatible with old format used in lmdb-store < 0.7), so in order to try to make this upgrade as seamless as possible, I created a migration script that uses a temporary lmdb-store-0.9 package that reads the legacy format to load records and re-save them into the new format to upgrade existing databases. I haven't put any effort into trying to make this temporary lmdb-store-0.9 package use prebuilds, because, well it is just temporarily needed for so everyone can get their dbs upgraded (I was planning on removing that package as a dependency of lmdb-store in the next major version, maybe in a few weeks). If you are seeing C compilation taking place on install, it is probably because of that package.

I could add prebuilds to the lmdb-store-0.9 package too, I suppose. Or I could easily make it an optional dependency (so if build fails, it is ignored). Anyway, let me know if any of these options would be helpful.

[mistic]

@kriszyp thank you for the detailed explanation 🎉

I could add prebuilds to the lmdb-store-0.9 package too, I suppose. Or I could easily make it an optional dependency (so if build fails, it is ignored). Anyway, let me know if any of these options would be helpful.

Humm I would say it is probably better if we can also add prebuilds for lmdb-store-0.9. What do you think?

[kriszyp]

OK, I published an lmdb-store 0.8.14 that uses a new lmdb-store-0.9 that should have prebuilds for the major versions, want to check to see if that works for you (without requiring compilation)?

[mistic]

@kriszyp I've checked the new version you published and something is not working as expected here. Looks like the node-gyp compilation tool place and additionally it also have failed:

error /test_project/node_modules/lmdb-store: Command failed.
Exit code: 1
Command: node-gyp-build
Arguments:
Directory: /test_project/node_modules/lmdb-store
Output:
gyp info it worked if it ends with ok
gyp info using node-gyp@5.1.0
gyp info using node@12.19.1 | darwin | x64
gyp info find Python using Python version 2.7.16 found at "/System/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python"
gyp info spawn /System/Library/Frameworks/Python.framework/Versions/2.7/Resources/Python.app/Contents/MacOS/Python
gyp info spawn args [
gyp info spawn args   '/.nvm/versions/node/v12.19.1/lib/node_modules/npm/node_modules/node-gyp/gyp/gyp_main.py',
gyp info spawn args   'binding.gyp',
gyp info spawn args   '-f',
gyp info spawn args   'make',
gyp info spawn args   '-I',
gyp info spawn args   '/test_project/node_modules/lmdb-store/build/config.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/.nvm/versions/node/v12.19.1/lib/node_modules/npm/node_modules/node-gyp/addon.gypi',
gyp info spawn args   '-I',
gyp info spawn args   '/Library/Caches/node-gyp/12.19.1/include/node/common.gypi',
gyp info spawn args   '-Dlibrary=shared_library',
gyp info spawn args   '-Dvisibility=default',
gyp info spawn args   '-Dnode_root_dir=/Library/Caches/node-gyp/12.19.1',
gyp info spawn args   '-Dnode_gyp_dir=/.nvm/versions/node/v12.19.1/lib/node_modules/npm/node_modules/node-gyp',
gyp info spawn args   '-Dnode_lib_file=/Library/Caches/node-gyp/12.19.1/<(target_arch)/node.lib',
gyp info spawn args   '-Dmodule_root_dir=/test_project/node_modules/lmdb-store',
gyp info spawn args   '-Dnode_engine=v8',
gyp info spawn args   '--depth=.',
gyp info spawn args   '--no-parallel',
gyp info spawn args   '--generator-output',
gyp info spawn args   'build',
gyp info spawn args   '-Goutput_dir=.'
gyp info spawn args ]
gyp info spawn make
gyp info spawn args [ 'BUILDTYPE=Release', '-C', 'build' ]
  CC(target) Release/obj.target/lmdb-store/dependencies/lmdb/libraries/liblmdb/mdb.o
  CC(target) Release/obj.target/lmdb-store/dependencies/lmdb/libraries/liblmdb/midl.o
  CC(target) Release/obj.target/lmdb-store/dependencies/lmdb/libraries/liblmdb/chacha8.o
  CC(target) Release/obj.target/lmdb-store/dependencies/lz4/lib/lz4.o
  CXX(target) Release/obj.target/lmdb-store/src/node-lmdb.o
  CXX(target) Release/obj.target/lmdb-store/src/env.o
../src/env.cpp:133:47: warning: field 'flags' will be initialized after field 'path' [-Wreorder-ctor]
      : Nan::AsyncWorker(callback), env(env), flags(flags), path(strdup(inPath)) {
                                              ^
../src/env.cpp:202:7: warning: field 'actions' will be initialized after field 'actionCount' [-Wreorder-ctor]
      actions(actions),
      ^
../src/env.cpp:204:7: warning: field 'putFlags' will be initialized after field 'env' [-Wreorder-ctor]
      putFlags(putFlags),
      ^
../src/env.cpp:206:7: warning: field 'keySpace' will be initialized after field 'results' [-Wreorder-ctor]
      keySpace(keySpace),
      ^
../src/env.cpp:212:23: warning: unused variable 'action' [-Wunused-variable]
            action_t* action = &actions[i];
                      ^
../src/env.cpp:459:11: warning: extra tokens at end of #endif directive [-Wextra-tokens]
    #endif;
          ^
          //
../src/env.cpp:559:5: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
    obj->Set(context, Nan::New<String>("pageSize").ToLocalChecked(), Nan::New<Number>(stat.ms_psize));
    ^~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/env.cpp:560:5: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
    obj->Set(context, Nan::New<String>("treeDepth").ToLocalChecked(), Nan::New<Number>(stat.ms_depth));
    ^~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/env.cpp:561:5: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
    obj->Set(context, Nan::New<String>("treeBranchPageCount").ToLocalChecked(), Nan::New<Number>(stat.ms_branch_pages));
    ^~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/env.cpp:562:5: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
    obj->Set(context, Nan::New<String>("treeLeafPageCount").ToLocalChecked(), Nan::New<Number>(stat.ms_leaf_pages));
    ^~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/env.cpp:563:5: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
    obj->Set(context, Nan::New<String>("entryCount").ToLocalChecked(), Nan::New<Number>(stat.ms_entries));
    ^~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/env.cpp:587:5: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
    obj->Set(context, Nan::New<String>("mapAddress").ToLocalChecked(), Nan::New<Number>((uint64_t) envinfo.me_mapaddr));
    ^~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/env.cpp:588:5: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
    obj->Set(context, Nan::New<String>("mapSize").ToLocalChecked(), Nan::New<Number>(envinfo.me_mapsize));
    ^~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/env.cpp:589:5: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
    obj->Set(context, Nan::New<String>("lastPageNumber").ToLocalChecked(), Nan::New<Number>(envinfo.me_last_pgno));
    ^~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/env.cpp:590:5: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
    obj->Set(context, Nan::New<String>("lastTxnId").ToLocalChecked(), Nan::New<Number>(envinfo.me_last_txnid));
    ^~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/env.cpp:591:5: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
    obj->Set(context, Nan::New<String>("maxReaders").ToLocalChecked(), Nan::New<Number>(envinfo.me_maxreaders));
    ^~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/env.cpp:592:5: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
    obj->Set(context, Nan::New<String>("numReaders").ToLocalChecked(), Nan::New<Number>(envinfo.me_numreaders));
    ^~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/env.cpp:742:9: warning: unused variable 'persistedIndex' [-Wunused-variable]
    int persistedIndex = 0;
        ^
../src/env.cpp:909:5: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
    exports->Set(Nan::GetCurrentContext(), Nan::New<String>("Compression").ToLocalChecked(), compressionTpl->GetFunction(Nan::GetCurrentContext()).ToLocalChecked());
    ^~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/env.cpp:912:5: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
    exports->Set(Nan::GetCurrentContext(), Nan::New<String>("Env").ToLocalChecked(), envTpl->GetFunction(Nan::GetCurrentContext()).ToLocalChecked());
    ^~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
20 warnings generated.
  CXX(target) Release/obj.target/lmdb-store/src/compression.o
../src/compression.cpp:92:81: warning: format specifies type 'unsigned int' but the argument has type 'size_t' (aka 'unsigned long') [-Wformat]
        fprintf(stderr, "Failed to decompress data %u %u %u %u\n", charData[0], data.mv_size, compressionHeaderSize, uncompressedLength);
                                                      ~~                        ^~~~~~~~~~~~
                                                      %zu
../src/compression.cpp:133:54: warning: shift count >= width of type [-Wshift-count-overflow]
            compressedData[2] = (uint8_t)(dataLength >> 40u);
                                                     ^  ~~~
../src/compression.cpp:134:54: warning: shift count >= width of type [-Wshift-count-overflow]
            compressedData[3] = (uint8_t)(dataLength >> 32u);
                                                     ^  ~~~
3 warnings generated.
  CXX(target) Release/obj.target/lmdb-store/src/ordered-binary.o
../src/ordered-binary.cpp:137:80: error: no member named 'Description' in 'v8::Symbol'
        Local<String> string = Local<String>::Cast(Local<Symbol>::Cast(jsKey)->Description());
                                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~  ^
../src/ordered-binary.cpp:129:67: warning: comparison of integers of different signs: 'size_t' (aka 'unsigned long') and 'int' [-Wsign-compare]
                Local<ArrayBufferView>::Cast(jsKey)->ByteLength() > bytesWritten) {
                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~
../src/ordered-binary.cpp:266:17: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
                resultsArray->Set(context, i + 1, resultsArray->Get(context, i).ToLocalChecked());
                ^~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../src/ordered-binary.cpp:270:13: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
            resultsArray->Set(context, 1, nextValue);
            ^~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~~~
../src/ordered-binary.cpp:272:9: warning: ignoring return value of function declared with 'warn_unused_result' attribute [-Wunused-result]
        resultsArray->Set(context, 0, value);
        ^~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~
../src/ordered-binary.cpp:289:10: warning: unused variable 'isValid' [-Wunused-variable]
    bool isValid = true;
         ^
5 warnings and 1 error generated.
make: *** [Release/obj.target/lmdb-store/src/ordered-binary.o] Error 1
gyp ERR! build error
gyp ERR! stack Error: `make` failed with exit code: 2
gyp ERR! stack     at ChildProcess.onExit (/.nvm/versions/node/v12.19.1/lib/node_modules/npm/node_modules/node-gyp/lib/build.js:194:23)
gyp ERR! stack     at ChildProcess.emit (events.js:314:20)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:275:12)
gyp ERR! System Darwin 19.6.0
gyp ERR! command "/.nvm/versions/node/v12.19.1/bin/node" "/.nvm/versions/node/v12.19.1/lib/node_modules/npm/node_modules/node-gyp/bin/node-gyp.js" "rebuild"
gyp ERR! cwd /test_project/node_modules/lmdb-store

[kriszyp]

Ah, I missed a compilation error that specifically affects v12. Published 0.8.15 to fix that, can you give that a try?

[mistic]

@kriszyp it looks like it is working for now. I will update you later with extra info. If everything is working as expected, bazel remote caches of code using lmbd-store should not be invalidated for each build run 😃