anothersmith / node-duckdb

DuckDB NodeJS bindings
MIT License
48 stars 12 forks source link

Windows build? #137

Open antonycourtney opened 2 years ago

antonycourtney commented 2 years ago

Love the simplicity of the API and nice packaging (like including parquet support) of node-duckdb!

I'm developing a packaged application (Tad, a tabular data viewer). I've got it working nicely using node-duckdb on Mac and Linux, but I'd also like to include a Windows build as I did for past releases that were built on sqlite.

Has anyone attempted to build node-duckdb for Windows? Any known / expected compatibility issues? How dependent is the build process on being able to run Unix-like commands and shell scripts?

rprovodenko commented 2 years ago

Hey, thank you!

There is no windows support currently and it wouldn't work out of the box on windows natively, but you could try using the Windows Linux Subsystem! Otherwise, you could use docker or create a PR for native windows support. This PR would involve changing the scripts in package.json, adding an appveyor job (I will do that once there is a PR), and changing the CMakeLists to add windows support, no other changes should be needed.

antonycourtney commented 2 years ago

Hi @rprovodenko Thanks for getting back to me, and I'm delighted to hear you are open to a PR for this! I am using node-duckdb in a cross-platform application I've developed (Tad) that already works great on Linux and Mac. I hired a freelancer to do the bulk of the work (CMake changes), and gave it a little push myself to get it over the finish line, which we just achieved a few days ago. Here is the PR with the changes for getting this working on Windows.

One thing to note is that this downloads and builds the latest duckdb master branch directly from the duckdb github repo instead of using the snapshot used to build node-duckdb. So this branch also includes some changes to result_iterator.cc in node-duckdb addons to accommodate some minor changes made to duckdb since the snapshot used in the download-duckdb script. We had to go this route because duckdb master has a few portability fixes (particularly in the ipfs extension) that allow it to build on Windows.

I'd love to see this work land in the main node-duck repo so that others can benefit from this. A first step is probably to get node-duckdb updated to use the latest duckdb code from the master branch Do you have plans to do that work? Would you like me to submit just the changes for that in a PR?

Once node-duckdb is up to date with duckdb master, I can rebase my windows-minimal branch, get rid of any code changes in the addons in that branch, and then submit a clean PR that just adds Windows support.

I'm totally open to other ways to do it, please just let me know how you'd like to proceed!

rprovodenko commented 2 years ago

Awesome @antonycourtney 👍 You'll have to bear with me while I get some free time to address this mate