lidavidm commented on code in PR #3591:
URL: https://github.com/apache/arrow-adbc/pull/3591#discussion_r2443708088


##########
c/driver/sqlite/CMakeLists.txt:
##########
@@ -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")

Review Comment:
   Why does this need a cmake version check?



##########
c/driver_manager/CMakeLists.txt:
##########
@@ -98,9 +98,15 @@ if(ADBC_BUILD_TESTS)
   target_compile_features(adbc-driver-manager-test PRIVATE cxx_std_17)
 
   if(ADBC_DRIVER_SQLITE)
-    target_compile_definitions(adbc-driver-manager-test
-                               PRIVATE 
ADBC_DRIVER_MANAGER_TEST_LIB="${CMAKE_BINARY_DIR}/driver/sqlite/libadbc_driver_sqlite${CMAKE_SHARED_LIBRARY_SUFFIX}"
-    )
+    if(WIN32)
+      target_compile_definitions(adbc-driver-manager-test
+                                PRIVATE 
ADBC_DRIVER_MANAGER_TEST_LIB="${CMAKE_BINARY_DIR}/driver/sqlite/adbc_driver_sqlite${CMAKE_SHARED_LIBRARY_SUFFIX}"
+      )
+    else()
+      target_compile_definitions(adbc-driver-manager-test
+                                PRIVATE 
ADBC_DRIVER_MANAGER_TEST_LIB="${CMAKE_BINARY_DIR}/driver/sqlite/libadbc_driver_sqlite${CMAKE_SHARED_LIBRARY_SUFFIX}"
+      )
+    endif()

Review Comment:
   
https://cmake.org/cmake/help/latest/variable/CMAKE_SHARED_LIBRARY_PREFIX.html 
is probably the easier way to deal with this



-- 
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]

Reply via email to