chore: Github action for Node spannerlib wrapper - #896
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the package.json for the Node.js wrapper to include build/Release/libspanner.* in the published files. The reviewer recommends explicitly listing the platform-specific shared library files (.so, .dylib, and .dll) instead of using a wildcard to avoid including unnecessary build artifacts.
b4fe6ef to
450ffb8
Compare
surbhigarg92
changed the title
450ffb8 to
25d17bf
Compare
| os_name: darwin | ||
| cpu_name: arm64 | ||
| pkg_name: "@google-cloud/spannerlib-node-darwin-arm64" | ||
| - os: ubuntu-latest |
There was a problem hiding this comment.
We should probably use ubuntu-22.04 here. That ensures that the build uses glibc v2.35. That ensures that it can run on many more linux versions than if you build this on ubuntu-24.
Also, we should change the build mode for the shared library from c-shared to c-archive. That will create just one file, instead of multiple files. So like this:
go build -C "$SHARED_LIB_DIR" -o libspanner.a -buildmode=c-archive shared_lib.go
And then also update binding.gyp to reference this new file instead of the list of files.
There was a problem hiding this comment.
Implemented this for mac and linux . However c-archive cant be used for Windows.
| "dependencies": { | ||
| "node-addon-api": "^8.0.0", | ||
| "bindings": "^1.5.0", | ||
| "@google-cloud/spanner": "^8.7.1" |
There was a problem hiding this comment.
It would be good if we could remove or at least 'reduce' this dependency. All we need are the protobuf definitions, right?
There was a problem hiding this comment.
Yes, this is another TODO in the list, currently node does not publish a separate package for protos, so there is no alternate for us. Another option which I explored was to generate the protos for this, but that was also not straight forward to just generate for spanner.
There was a problem hiding this comment.
Makes sense. We should try to find a solution, because adding the Spanner client library here, adds a significant amount of files (and MBs) to the totals set of files that a customer installs when installing the node driver.
974c8e3 to
e1ea413
Compare
20d6cf3 to
747c0e7
Compare
| "dependencies": { | ||
| "node-addon-api": "^8.0.0", | ||
| "bindings": "^1.5.0", | ||
| "@google-cloud/spanner": "^8.7.1" |
There was a problem hiding this comment.
Makes sense. We should try to find a solution, because adding the Spanner client library here, adds a significant amount of files (and MBs) to the totals set of files that a customer installs when installing the node driver.
747c0e7 to
90d3c63
Compare
2 participants
Description
This PR introduces the automated build and release pipeline for the Node N-API wrapper, alongside structural improvements to how the native addon is linked and distributed.
Key Changes:
release-node-wrapper.yml) to build, package, and publish platform-specific binaries (macOS, Linux, Windows) to the Wombat registry.c-shared) to static linking (c-archive) for the Go library on macOS and Linux. This embeds the library directly into the N-API module, simplifying distribution and removing the need forrpathpatching.gendef/dlltoolfor MSVC.libgeneration.>=22and movesnode-addon-apitodevDependencies(as we now distribute pre-built binaries).