Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft PR: Windows Support #1

Draft
wants to merge 17 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 21 additions & 8 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ cmake_policy(SET CMP0042 NEW)
project(node-duckdb-addon)
set(CMAKE_CXX_STANDARD 11)
set(CMAKE_POSITION_INDEPENDENT_CODE ON)
file(GLOB DUCKDBIN
"./duckdb/build/release/src/libduckdb.dylib"
"./duckdb/build/release/src/libduckdb.so"
"./duckdb/build/release/extension/parquet/libparquet_extension.a"
"./duckdb/build/release/extension/httpfs/libhttpfs_extension.a"
)

if(WIN32)
# Expecting duckdb to be downloaded at the parent directory
file(GLOB DUCKDBIN "./duckdb/build/src/release/duckdb_static.lib" "./duckdb/build/extension/httpfs/release/httpfs_extension.lib" "./duckdb/build/extension/parquet/release/parquet_extension.lib" "./duckdb/build/third_party/fmt/release/duckdb_fmt.lib" "./duckdb/build/third_party/libpg_query/release/duckdb_pg_query.lib" "./duckdb/build/third_party/re2/release/duckdb_re2.lib" "./duckdb/build/third_party/miniz/Release/duckdb_miniz.lib" "./duckdb/build/third_party/utf8proc/Release/duckdb_utf8proc.lib" "./duckdb/build/third_party/hyperloglog/Release/duckdb_hyperloglog.lib" "./duckdb/build/third_party/fastpforlib/Release/duckdb_fastpforlib.lib")
else()
file(GLOB DUCKDBIN "./duckdb/build/release/src/libduckdb.dylib" "./duckdb/build/release/src/libduckdb.so" "./duckdb/build/release/extension/parquet/libparquet_extension.a" "./duckdb/build/release/extension/httpfs/libhttpfs_extension.a")
endif()

file(COPY ${DUCKDBIN} DESTINATION ${CMAKE_CURRENT_BINARY_DIR}/Release)
include_directories(${CMAKE_JS_INC} ./duckdb/src/include ./duckdb/extension/parquet/include ./duckdb/extension/httpfs/include)
link_directories(${CMAKE_CURRENT_BINARY_DIR}/Release)
Expand All @@ -17,7 +19,11 @@ add_library(${PROJECT_NAME} SHARED ${SOURCE_FILES} ${CMAKE_JS_SRC})
set_target_properties(${PROJECT_NAME} PROPERTIES PREFIX "" SUFFIX ".node")
find_package(OpenSSL REQUIRED)

if (WIN32)
target_link_libraries(${PROJECT_NAME} ${CMAKE_JS_LIB} duckdb_static duckdb_fmt duckdb_pg_query duckdb_re2 duckdb_miniz duckdb_utf8proc duckdb_hyperloglog duckdb_fastpforlib parquet_extension httpfs_extension ${OPENSSL_CRYPTO_LIBRARY} ${OPENSSL_SSL_LIBRARY})
else ()
target_link_libraries(${PROJECT_NAME} ${CMAKE_JS_LIB} duckdb parquet_extension httpfs_extension ${OPENSSL_CRYPTO_LIBRARY} ${OPENSSL_SSL_LIBRARY})
endif()
if(APPLE)
message("Building for MacOS")
set_target_properties(${PROJECT_NAME} PROPERTIES LINK_FLAGS "-Wl,-rpath,@loader_path/.")
Expand All @@ -26,14 +32,21 @@ if(UNIX AND NOT APPLE)
message("Building for Linux")
set_target_properties(${PROJECT_NAME} PROPERTIES LINK_FLAGS "-Wl,-rpath,'$ORIGIN'")
endif()

if(WIN32)
message("Building for Windows")
set_target_properties(${PROJECT_NAME} PROPERTIES PREFIX "" SUFFIX ".node")
endif()
# Include N-API wrappers
execute_process(COMMAND node -p "require('node-addon-api').include"
WORKING_DIRECTORY ${CMAKE_SOURCE_DIR}
OUTPUT_VARIABLE NODE_ADDON_API_DIR
)

#This CMakeLists.txt is not expected to be called directly through CMake.
#Doing so can throw parameter mismatch for string in the below two lines.
string(REPLACE "\n" "" NODE_ADDON_API_DIR ${NODE_ADDON_API_DIR})
string(REPLACE "\"" "" NODE_ADDON_API_DIR ${NODE_ADDON_API_DIR})
string(REPLACE "\"" "" NODE_ADDON_API_DIR ${NODE_ADDON_API_DIR})

target_include_directories(${PROJECT_NAME} PRIVATE ${NODE_ADDON_API_DIR})

# define NPI_VERSION
Expand Down
15 changes: 15 additions & 0 deletions WindowsBuildHelper.bat
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
@ECHO off

if not exist "openssl-1.1.1m" (
@echo Downloading OpenSSL...
curl -L https://mirror.firedaemon.com/OpenSSL/openssl-1.1.1m.zip > openssl-1.1.1m.zip
@echo Unpacking OpenSSL...
mkdir .\openssl-1.1.1m
tar -xf openssl-1.1.1m.zip -C .\openssl-1.1.1m --strip-components=1
@echo Deleting openssl-1.1.1m.zip file
del openssl-1.1.1m.zip
)

@echo Starting the build...
set OPENSSL_ROOT_DIR=%cd%\openssl-1.1.1m\x64
npm install
18 changes: 8 additions & 10 deletions addon/result_iterator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ Napi::Value ResultIterator::getCellValue(Napi::Env env, duckdb::idx_t col_idx) {
}

Napi::Value ResultIterator::getMappedValue(Napi::Env env, duckdb::Value value) {
if (value.is_null) {
if (value.IsNull()) {
return env.Null();
}

Expand Down Expand Up @@ -183,11 +183,7 @@ Napi::Value ResultIterator::getMappedValue(Napi::Env env, duckdb::Value value) {
case duckdb::LogicalTypeId::VARCHAR:
return Napi::String::New(env, value.GetValue<string>());
case duckdb::LogicalTypeId::BLOB: {
int array_length = value.str_value.length();
char char_array[array_length + 1];
// TODO: multiple copies, improve
strcpy(char_array, value.str_value.c_str());
return Napi::Buffer<char>::Copy(env, char_array, array_length);
return Napi::Buffer<char>::Copy(env, value.GetValue<string>().c_str(), value.GetValue<string>().length() + 1);
}
case duckdb::LogicalTypeId::TIMESTAMP: {
if (value.type().InternalType() != duckdb::PhysicalType::INT64) {
Expand Down Expand Up @@ -215,19 +211,21 @@ Napi::Value ResultIterator::getMappedValue(Napi::Env env, duckdb::Value value) {
return Napi::Number::New(env, value.GetValue<int64_t>());
case duckdb::LogicalTypeId::LIST: {
auto array = Napi::Array::New(env);
for (size_t i = 0; i < value.list_value.size(); i++) {
auto &element = value.list_value[i];
auto& elements = duckdb::ListValue::GetChildren(value);
for (size_t i = 0; i < elements.size(); i++) {
auto &element = elements[i];
auto mapped_value = getMappedValue(env, element);
array.Set(i, mapped_value);
}
return array;
}
case duckdb::LogicalTypeId::STRUCT: {
auto object = Napi::Object::New(env);
for (size_t i = 0; i < value.struct_value.size(); i++) {
auto structobject = duckdb::StructValue::GetChildren(value);
for (size_t i = 0; i < structobject.size(); i++) {
auto &child_types = duckdb::StructType::GetChildTypes(value.type());
auto &key = child_types[i].first;
auto &element = value.struct_value[i];
auto &element = structobject[i];
auto child_value = getMappedValue(env, element);
object.Set(key, child_value);
}
Expand Down
4 changes: 3 additions & 1 deletion jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ module.exports = {
collectCoverage: true,
coverageReporters: ["lcov", "text"],
roots: ["<rootDir>"],
testMatch: ["<rootDir>/dist/**/?(*.)+(test).js"],
testMatch: ["<rootDir>/dist/**/__tests__/**/*.[jt]s?(x)",
"<rootDir>/dist/**/*(*.)@(spec|test).[tj]s?(x)"
],
setupFilesAfterEnv: ["jest-extended"],
testEnvironment: "node",
};
75 changes: 40 additions & 35 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -32,19 +32,24 @@
},
"scripts": {
"audit:fix": "yarn-audit-fix",
"build": "yarn build:duckdb && yarn build:addon && yarn build:ts",
"build": "run-script-os",
"build:windows": "yarn build:duckdb:windows && yarn build:addon && yarn build:ts",
"build:default": "yarn build:duckdb && yarn build:addon && yarn build:ts",
"build:addon": "rimraf build && cmake-js compile --CDnapi_build_version=6",
"build:duckdb": "cd duckdb && make BUILD_HTTPFS=1 && cd -",
"build:duckdb": "cd duckdb && make BUILD_HTTPFS=1 && cd -",
"build:duckdb:windows": "cd duckdb && rimraf build && mkdir build && cd build && cmake -Ax64 .. -DBUILD_HTTPFS_EXTENSION=1 -DBUILD_PARQUET_EXTENSION=1 -DCMAKE_INSTALL_PREFIX=./Release && cmake --build . --config=Release",
"build:test:watch": "nodemon --exec 'yarn build && yarn jest --testTimeout=60000'",
"build:ts": "rimraf dist && ttsc",
"build:ts": "rimraf dist && tsc",
"clang:check": "yarn clang-format --dry-run --Werror addon/**",
"clang:fix": "yarn clang-format -i addon/**",
"cleanup:binaries": "rm -rf build prebuilds duckdb",
"download-duckdb": "rm -rf duckdb && curl -L https://github.com/cwida/duckdb/archive/c07e229072595e086dc0adfac72638e8cc72de40.tar.gz > duckdb.tar.gz && tar xf duckdb.tar.gz && mv duckdb-c07e229072595e086dc0adfac72638e8cc72de40 duckdb && rm duckdb.tar.gz",
"cleanup:binaries": "rimraf build prebuilds duckdb",
"download-duckdb": "rimraf duckdb && curl -L https://github.com/duckdb/duckdb/tarball/master > duckdb.tar.gz && mkdir duckdb && tar xf duckdb.tar.gz -C ./duckdb --strip-components=1 && rimraf duckdb.tar.gz",
"eslint:check": "eslint --ext .js,.json,.ts ./",
"eslint:fix": "eslint --fix --ext .js,.json,.ts ./",
"generate-doc": "yarn build:ts && rm -rf temp etc && mkdir etc && yarn api-extractor run --local --verbose && yarn api-documenter markdown -i temp -o docs/api && ./docs/replace.sh",
"install": "prebuild-install --verbose -d -r napi || (yarn download-duckdb && yarn build:duckdb && yarn prebuild:current-target)",
"generate-doc": "yarn build:ts && rimraf temp etc && mkdir etc && yarn api-extractor run --local --verbose && yarn api-documenter markdown -i temp -o docs/api && ./docs/replace.sh",
"install": "run-script-os",
"install:windows": "prebuild-install --verbose -d -r napi || (yarn download-duckdb && yarn build:duckdb:windows && yarn prebuild:current-target)",
"install:default": "prebuild-install --verbose -d -r napi || (yarn download-duckdb && yarn build:duckdb && yarn prebuild:current-target)",
"lint:check": "yarn prettier:check && yarn eslint:check && yarn clang:check",
"lint:fix": "yarn prettier:fix && yarn eslint:fix && yarn clang:fix",
"prebuild:all-targets": "yarn install && yarn prebuild:linux",
Expand All @@ -65,45 +70,45 @@
"trim-newlines": ">=3.0.1 <4.0.0 || >=4.0.1"
},
"dependencies": {
"cmake-js": "^6.1.0",
"node-addon-api": "^3.0.2",
"prebuild": "^10.0.1",
"prebuild-install": "^6.0.0"
"cmake-js": "^6.3.0",
"node-addon-api": "^4.3.0",
"prebuild": "^11.0.2",
"prebuild-install": "^7.0.1"
},
"devDependencies": {
"@microsoft/api-documenter": "^7.11.3",
"@microsoft/api-extractor": "^7.13.2",
"@types/jest": "^26.0.14",
"@types/lodash": "^4.14.167",
"@types/parquetjs": "^0.10.2",
"@typescript-eslint/eslint-plugin": "^4.3.0",
"@microsoft/api-documenter": "^7.15.1",
"@microsoft/api-extractor": "^7.19.4",
"@types/jest": "^27.4.0",
"@types/lodash": "^4.14.178",
"@types/parquetjs": "^0.10.3",
"@typescript-eslint/eslint-plugin": "^5.10.1",
"@zerollup/ts-transform-paths": "^1.7.18",
"clang-format": "^1.4.0",
"eslint": "^7.10.0",
"eslint-config-deepcrawl": "^5.6.0",
"eslint-config-prettier": "^6.11.0",
"eslint-import-resolver-typescript": "^2.0.0",
"eslint-plugin-array-func": "^3.1.6",
"clang-format": "^1.6.0",
"eslint": "^8.8.0",
"eslint-config-prettier": "^8.3.0",
"eslint-import-resolver-typescript": "^2.5.0",
"eslint-plugin-array-func": "^3.1.7",
"eslint-plugin-clean-code": "^0.1.12",
"eslint-plugin-filenames": "^1.3.2",
"eslint-plugin-import": "^2.20.2",
"eslint-plugin-jest": "^23.13.1",
"eslint-plugin-import": "^2.25.4",
"eslint-plugin-jest": "^26.0.0",
"eslint-plugin-json-format": "^2.0.1",
"eslint-plugin-no-loops": "^0.3.0",
"eslint-plugin-node": "^11.1.0",
"eslint-plugin-promise": "^4.2.1",
"eslint-plugin-sonarjs": "^0.5.0",
"jest": "^26.2.2",
"jest-extended": "^0.11.5",
"eslint-plugin-promise": "^6.0.0",
"eslint-plugin-sonarjs": "^0.11.0",
"jest": "^27.4.7",
"jest-extended": "^2.0.0",
"lodash": "^4.17.21",
"nodemon": "^2.0.4",
"nodemon": "^2.0.15",
"parquetjs": "^0.11.2",
"prettier": "^2.0.5",
"prettier": "^2.5.1",
"rimraf": "^3.0.2",
"standard-version": "^9.1.1",
"ttypescript": "^1.5.12",
"typescript": "^4.0.3",
"yarn-audit-fix": "6.3.2"
"run-script-os": "^1.1.6",
"standard-version": "^9.3.2",
"ttypescript": "^1.5.13",
"typescript": "^4.5.5",
"yarn-audit-fix": "9.0.10"
},
"engines": {
"node": ">= 12.17.0"
Expand Down
58 changes: 58 additions & 0 deletions support_for_windows.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
# Supporting node-duckdb on Windows

## Specifically
- Windows 10 with Visual Studio 2019
- DuckDB from HEAD
- Build natively using yarn

## Prerequisites
- Visual Studio 2019 (although not tested, community edition should work)
- NodeJS LTS or latest version installed
- Download prebuilt OpenSSL from https://mirror.firedaemon.com/OpenSSL/openssl-1.1.1m.zip and extract to somedir\openssl-1.1<br>
(Note: It is advisable to build OpenSSL from source using VCPkg)
- Create an environment variable OPENSSL_ROOT_DIR and point to somedir\openssl-1.1\x64<br>
(Note: If VCPkg was used, set it to wherever\vcpkg\buildtrees\openssl\x64-windows-rel)
- Open x64 Native tools command prompt of visual studio and make sure OPENSSL_ROOT_DIR variable is available in it.

## Building node-duckdb
> There are just two steps to build duckdb for windows in the Visual Studio command prompt
> Download this repository and issue the following commands:
> + yarn download-duckdb
> + yarn build:windows

## Pitfalls
- You don't have to manually install CMake. CMake comes bundled with current versions of Visual Studio.

- If you find errors like <br>
<code>
node:internal/modules/cjs/loader:936
throw err;
^

Error: Cannot find module 'node-addon-api'
Require stack:
D:\Work\Antony\windows-asis\[eval]
at Function.Module._resolveFilename (node:internal/modules/cjs/loader:933:15)
at Function.Module._load (node:internal/modules/cjs/loader:778:27)
at Module.require (node:internal/modules/cjs/loader:1005:19)
at require (node:internal/modules/cjs/helpers:102:18)
at [eval]:1:1
</code>
<br>Make sure NODE_PATH is set correctly.

- While running the tests if you find errors like <br>
<code>
FAIL dist/tests/result-stream.test.js
● Test suite failed to run

The specified module could not be found.
\\?\path\node-duckdb\build\Release\node-duckdb-addon.node

10 | // lambda doesn't work with npm module bindings
11 | // eslint-disable-next-line node/no-unpublished-require, @typescript-eslint/no-var-requires
12 | const { Connection } = require("../../build/Release/node-duckdb-addon.node");
</code>
<br>Make sure both libcrypto-1_1-x64.dll and libssl-1_1-x64.dll are in the path

## Important Disclaimer:
We have not been able to get the tests running on Windows. This work is underway.