NodeJS: switch to using package_build, and add support to BUILD_NODE to Makefile #5691
Conversation
… types are constant
| @@ -5,10 +5,4 @@ set -x | |||
|
|
|||
There was a problem hiding this comment.
We can probably just kill this file, right?
There was a problem hiding this comment.
I kept it around for backwards compatibility purposes - but I suppose we could replace it with the configure.py as well
| @@ -9,15 +9,16 @@ | |||
| "src/connection.cpp", | |||
There was a problem hiding this comment.
Is it the template version of this file that we're shipping? Have we checked that the built tar works as expected?
There was a problem hiding this comment.
I would suspect the tar build works fine as long as we are copying over the source files, but it will not work when doing an incremental build. Does this happen as part of the node-pre-gyp package command? Or is the tar file uploaded elsewhere separately?
There was a problem hiding this comment.
I tested out npm pack followed by npm install <tarball> locally and it works - but if the tarball is built and uploaded as part of the CI we need to ensure the tarball is created from a fresh configure, and not from a configure that is linked to an existing build.
|
@Mytherin it looks like the dev source tarballs that are being uploaded to npm are missing the duckdb source code? Are you able to have a look at this? |
|
It also looks like the paths on the agent have been hardcoded as well: https://www.npmjs.com/package/duckdb/v/0.6.2-dev670.0?activeTab=explore |
This PR switches the NodeJS clients' build system to use the
package_buildscript, similar to how the R and Python clients are setup. The advantages to this are as follows:BUILD_NODEcommand - speeding up CI significantlyThe way this works is that the
binding.gypfile is moved to abinding.gyp.infile. A new script,configure.py, is added that uses the sharedpackage_buildlibrary to add source files, include directories, and defines. By default all required source files are copied into a directory (tools/nodejs/src/duckdb) and compiled from there. There is an option to link to an existing build instead. This is used when compiling using theBUILD_NODEcommand, e.g.:The
CMakewill then set up a number of environment variables (DUCKDB_NODE_BINDIR,DUCKDB_NODE_CFLAGS,DUCKDB_NODE_LIBS) that are used to link to an existing build.The
BUILD_NODEcommand is also used in the CI to greatly speed up the MacOS and Linux CI runs. By linking to an existing DuckDB build the builds can be cached using ccache and re-used between different node versions. This allows all node versions to be build in a minute each, instead of taking 20-30 minutes for each node version.