-
Notifications
You must be signed in to change notification settings - Fork 162
feat(c,ci,go): add vcpkg-based build + ci, fix go imports #3591
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
base: main
Are you sure you want to change the base?
Changes from 9 commits
f885eba
e74127e
b07e5ae
5fff1b6
7da4cc6
51e8553
679be7d
4430bfe
490355d
cbd9d94
057af6f
27db1e6
1fe362c
b0b9dbd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| #!/usr/bin/env pwsh | ||
| # Licensed to the Apache Software Foundation (ASF) under one | ||
| # or more contributor license agreements. See the NOTICE file | ||
| # distributed with this work for additional information | ||
| # regarding copyright ownership. The ASF licenses this file | ||
| # to you under the Apache License, Version 2.0 (the | ||
| # "License"); you may not use this file except in compliance | ||
| # with the License. You may obtain a copy of the License at | ||
| # | ||
| # http://www.apache.org/licenses/LICENSE-2.0 | ||
| # | ||
| # Unless required by applicable law or agreed to in writing, | ||
| # software distributed under the License is distributed on an | ||
| # "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| # KIND, either express or implied. See the License for the | ||
| # specific language governing permissions and limitations | ||
| # under the License. | ||
|
|
||
| vcpkg_installed/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,20 @@ | |
| find_program(GO_BIN "go" REQUIRED) | ||
| message(STATUS "Detecting Go executable: Found ${GO_BIN}") | ||
|
|
||
| if(WIN32) | ||
| # Find tools for generating import libraries from Go DLLs since Go doesn't | ||
| # produce .lib files | ||
| find_program(GENDEF_BIN NAMES gendef) | ||
| find_program(DLLTOOL_BIN NAMES dlltool) | ||
| if(GENDEF_BIN AND DLLTOOL_BIN) | ||
| message(STATUS "Found gendef: ${GENDEF_BIN}") | ||
| message(STATUS "Found dlltool: ${DLLTOOL_BIN}") | ||
| else() | ||
| message(WARNING "gendef and/or dlltool not found - Go driver import libraries won't be automatically created") | ||
| message(WARNING "Install MinGW64 and add it to your PATH to make them available") | ||
| endif() | ||
| endif() | ||
|
|
||
| set(ADBC_GO_PACKAGE_INIT | ||
| [=[ | ||
| get_filename_component(_IMPORT_PREFIX "${CMAKE_CURRENT_LIST_FILE}" PATH) | ||
|
|
@@ -184,36 +198,85 @@ function(add_go_lib GO_MOD_DIR GO_LIBNAME) | |
| list(APPEND GO_ENV_VARS "GOARCH=arm64") | ||
| endif() | ||
|
|
||
| add_custom_command(OUTPUT "${LIBOUT_SHARED}.${ADBC_FULL_SO_VERSION}" | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reviewer note: The rest of this file I used a combination of Claude Code and the previous PR and comments therein to come up with something that worked. I want to go back over this tomorrow with fresh eyes but I thought I'd point it out in case it helps review. |
||
| WORKING_DIRECTORY ${GO_MOD_DIR} | ||
| DEPENDS ${ARG_SOURCES} | ||
| COMMAND ${CMAKE_COMMAND} -E env ${GO_ENV_VARS} ${GO_BIN} build | ||
| ${GO_BUILD_TAGS} "${GO_BUILD_FLAGS}" -o | ||
| ${LIBOUT_SHARED}.${ADBC_FULL_SO_VERSION} | ||
| -buildmode=c-shared ${GO_LDFLAGS} . | ||
| COMMAND ${CMAKE_COMMAND} -E remove -f | ||
| "${LIBOUT_SHARED}.${ADBC_SO_VERSION}.0.h" | ||
| COMMENT "Building Go Shared lib ${GO_LIBNAME}" | ||
| COMMAND_EXPAND_LISTS) | ||
| # Set platform-specific library output paths and generate header name | ||
| if(WIN32) | ||
| # On Windows, Go generates .dll and .lib with the base name (no version suffix) | ||
| set(GO_OUTPUT_LIB "${LIBOUT_SHARED}") | ||
| set(GO_OUTPUT_HEADER "${CMAKE_CURRENT_BINARY_DIR}/${GO_LIBNAME}.h") | ||
| set(LIBIMPLIB_SHARED | ||
| "${CMAKE_CURRENT_BINARY_DIR}/${CMAKE_IMPORT_LIBRARY_PREFIX}${GO_LIBNAME}${CMAKE_IMPORT_LIBRARY_SUFFIX}") | ||
| set(LIBDEF_OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/${GO_LIBNAME}.def") | ||
| else() | ||
| # On Unix-like systems, use version suffixes | ||
| set(GO_OUTPUT_LIB "${LIBOUT_SHARED}.${ADBC_FULL_SO_VERSION}") | ||
| set(GO_OUTPUT_HEADER "${LIBOUT_SHARED}.${ADBC_SO_VERSION}.0.h") | ||
| endif() | ||
|
|
||
| # Common Go build command | ||
| set(GO_BUILD_COMMAND | ||
| ${CMAKE_COMMAND} -E env ${GO_ENV_VARS} ${GO_BIN} build | ||
| ${GO_BUILD_TAGS} "${GO_BUILD_FLAGS}" -o ${GO_OUTPUT_LIB} | ||
| -buildmode=c-shared ${GO_LDFLAGS} .) | ||
|
|
||
| add_custom_command(OUTPUT "${LIBOUT_SHARED}.${ADBC_SO_VERSION}" "${LIBOUT_SHARED}" | ||
| DEPENDS "${LIBOUT_SHARED}.${ADBC_FULL_SO_VERSION}" | ||
| WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} | ||
| COMMAND ${CMAKE_COMMAND} -E create_symlink | ||
| "${LIB_NAME_SHARED}.${ADBC_FULL_SO_VERSION}" | ||
| "${LIB_NAME_SHARED}.${ADBC_SO_VERSION}" | ||
| COMMAND ${CMAKE_COMMAND} -E create_symlink | ||
| "${LIB_NAME_SHARED}.${ADBC_SO_VERSION}" | ||
| "${LIB_NAME_SHARED}") | ||
|
|
||
| add_custom_target(${GO_LIBNAME}_target ALL | ||
| DEPENDS "${LIBOUT_SHARED}.${ADBC_FULL_SO_VERSION}" | ||
| "${LIBOUT_SHARED}.${ADBC_SO_VERSION}" "${LIBOUT_SHARED}") | ||
| if(WIN32) | ||
| if(GENDEF_BIN AND DLLTOOL_BIN) | ||
| # Generate import library using gendef + dlltool | ||
| add_custom_command(OUTPUT "${GO_OUTPUT_LIB}" "${LIBIMPLIB_SHARED}" | ||
| WORKING_DIRECTORY ${GO_MOD_DIR} | ||
| DEPENDS ${ARG_SOURCES} | ||
| COMMAND ${GO_BUILD_COMMAND} | ||
| COMMAND ${CMAKE_COMMAND} -E chdir ${CMAKE_CURRENT_BINARY_DIR} ${GENDEF_BIN} ${GO_OUTPUT_LIB} -a | ||
| COMMAND ${DLLTOOL_BIN} --input-def ${LIBDEF_OUTPUT} --dllname ${LIB_NAME_SHARED} --output-lib ${LIBIMPLIB_SHARED} | ||
| COMMAND ${CMAKE_COMMAND} -E remove -f "${GO_OUTPUT_HEADER}" | ||
| COMMENT "Building Go Shared lib ${GO_LIBNAME}" | ||
| COMMAND_EXPAND_LISTS) | ||
| set(TARGET_DEPENDS "${GO_OUTPUT_LIB}" "${LIBIMPLIB_SHARED}") | ||
| else() | ||
| # Fallback: try to build without import library generation | ||
| add_custom_command(OUTPUT "${GO_OUTPUT_LIB}" | ||
| WORKING_DIRECTORY ${GO_MOD_DIR} | ||
| DEPENDS ${ARG_SOURCES} | ||
| COMMAND ${GO_BUILD_COMMAND} | ||
| COMMAND ${CMAKE_COMMAND} -E remove -f "${GO_OUTPUT_HEADER}" | ||
| COMMENT "Building Go Shared lib ${GO_LIBNAME}" | ||
| COMMAND_EXPAND_LISTS) | ||
| set(TARGET_DEPENDS "${GO_OUTPUT_LIB}") | ||
| endif() | ||
| else() | ||
| add_custom_command(OUTPUT "${GO_OUTPUT_LIB}" | ||
| WORKING_DIRECTORY ${GO_MOD_DIR} | ||
| DEPENDS ${ARG_SOURCES} | ||
| COMMAND ${GO_BUILD_COMMAND} | ||
| COMMAND ${CMAKE_COMMAND} -E remove -f "${GO_OUTPUT_HEADER}" | ||
| COMMENT "Building Go Shared lib ${GO_LIBNAME}" | ||
| COMMAND_EXPAND_LISTS) | ||
|
|
||
| add_custom_command(OUTPUT "${LIBOUT_SHARED}.${ADBC_SO_VERSION}" "${LIBOUT_SHARED}" | ||
| DEPENDS "${GO_OUTPUT_LIB}" | ||
| WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} | ||
| COMMAND ${CMAKE_COMMAND} -E create_symlink | ||
| "${LIB_NAME_SHARED}.${ADBC_FULL_SO_VERSION}" | ||
| "${LIB_NAME_SHARED}.${ADBC_SO_VERSION}" | ||
| COMMAND ${CMAKE_COMMAND} -E create_symlink | ||
| "${LIB_NAME_SHARED}.${ADBC_SO_VERSION}" | ||
| "${LIB_NAME_SHARED}") | ||
| set(TARGET_DEPENDS "${GO_OUTPUT_LIB}" "${LIBOUT_SHARED}.${ADBC_SO_VERSION}" "${LIBOUT_SHARED}") | ||
| endif() | ||
|
|
||
| # Create custom target with platform-specific dependencies | ||
| add_custom_target(${GO_LIBNAME}_target ALL DEPENDS ${TARGET_DEPENDS}) | ||
|
|
||
| # Create imported library with platform-specific properties | ||
| add_library(${GO_LIBNAME}_shared SHARED IMPORTED GLOBAL) | ||
| set_target_properties(${GO_LIBNAME}_shared | ||
| PROPERTIES IMPORTED_LOCATION | ||
| "${LIBOUT_SHARED}.${ADBC_FULL_SO_VERSION}" | ||
| IMPORTED_SONAME "${LIB_NAME_SHARED}") | ||
| if(WIN32) | ||
| set_target_properties(${GO_LIBNAME}_shared | ||
| PROPERTIES IMPORTED_LOCATION "${LIBOUT_SHARED}" | ||
| IMPORTED_IMPLIB "${LIBIMPLIB_SHARED}") | ||
| else() | ||
| set_target_properties(${GO_LIBNAME}_shared | ||
| PROPERTIES IMPORTED_LOCATION "${GO_OUTPUT_LIB}" | ||
| IMPORTED_SONAME "${LIB_NAME_SHARED}") | ||
| endif() | ||
| add_dependencies(${GO_LIBNAME}_shared ${GO_LIBNAME}_target) | ||
| if(ARG_OUTPUTS) | ||
| list(APPEND ${ARG_OUTPUTS} ${GO_LIBNAME}_shared) | ||
|
|
@@ -259,8 +322,7 @@ function(add_go_lib GO_MOD_DIR GO_LIBNAME) | |
| ${CMAKE_INSTALL_LIBDIR}) | ||
| endif() | ||
| if(WIN32) | ||
| # This symlink doesn't get installed | ||
| install(FILES "${LIBOUT_SHARED}.${ADBC_SO_VERSION}" TYPE BIN) | ||
| install(FILES "${LIBIMPLIB_SHARED}" TYPE LIB) | ||
| else() | ||
| install(FILES "${LIBOUT_SHARED}" "${LIBOUT_SHARED}.${ADBC_SO_VERSION}" TYPE LIB) | ||
| endif() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,6 +68,16 @@ foreach(LIB_TARGET ${ADBC_LIBRARIES}) | |
| if(NOT ADBC_DEFINE_COMMON_ENTRYPOINTS) | ||
| target_compile_definitions(${LIB_TARGET} PRIVATE ${ADBC_TARGET_COMPILE_DEFINITIONS}) | ||
| endif() | ||
|
|
||
| # On Windows, install sqlite3.dll alongside the driver | ||
| if(WIN32 AND CMAKE_VERSION VERSION_GREATER_EQUAL "3.21") | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this need a cmake version check?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This whole block was mainly to copy runtime deps, I came up with using RUNTIME_DEPENDENCY_SET like this with LLM help and RUNTIME_DEPENDENCY_SET is 3.21, see https://cmake.org/cmake/help/latest/command/install.html. I tried something similar before this and didn't get something working.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's probably a much better way to do this, I can keep working on this part.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess then the problem is it won't work on older CMake! Our minimum is 3.18 right now but maybe we can bump it up?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah I'll try to come up with something simpler in this PR. It boils down to copying one DLL in the case of sqlite and three in the case of postgresql. |
||
| install(RUNTIME_DEPENDENCY_SET ${LIB_TARGET}_runtime_deps | ||
| PRE_EXCLUDE_REGEXES "api-ms-" "ext-ms-" | ||
| POST_EXCLUDE_REGEXES ".*system32/.*\\.dll" | ||
| DESTINATION ${RUNTIME_INSTALL_DIR}) | ||
| install(TARGETS ${LIB_TARGET} | ||
| RUNTIME_DEPENDENCY_SET ${LIB_TARGET}_runtime_deps) | ||
| endif() | ||
| endforeach() | ||
|
|
||
| include(CheckTypeSize) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| { | ||
| "name": "adbc-driver-sqlite", | ||
| "version-string": "1.0.0a0", | ||
| "dependencies": [ | ||
| "sqlite3" | ||
| ] | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.