lidavidm commented on code in PR #2858:
URL: https://github.com/apache/arrow-adbc/pull/2858#discussion_r2119877340
##########
c/version.rc.in:
##########
@@ -0,0 +1,48 @@
+#include <windows.h>
+
+// Licensed to the Apache Software Foundation (ASF) under one
Review Comment:
Is it not possible to put the license header at the top?
##########
c/driver/flightsql/CMakeLists.txt:
##########
@@ -63,6 +63,18 @@ if(ADBC_BUILD_TESTS)
adbc_driver_common
adbc_validation
${TEST_LINK_LIBS})
+
+ if(ADBC_TEST_LINKAGE STREQUAL "shared")
+ # The go build is not copying the DLL to the test location, causing the
test to fail on Windows.
Review Comment:
Hmm, how is this handled for the C/C++ builds? I wouldn't think CMake copies
the DLL over...
##########
c/cmake_modules/GoUtils.cmake:
##########
@@ -180,36 +201,79 @@ 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}"
- 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)
+ if(WIN32)
Review Comment:
We're now fully duplicating the build paths...if Windows doesn't want the
version suffix, can it just rename the file afterwards instead?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
