Re: [PR] GH-49219: [C++][FlightRPC] Enable static ODBC build on macOS [arrow]
conbench-apache-arrow[bot] commented on PR #49220: URL: https://github.com/apache/arrow/pull/49220#issuecomment-4003799699 After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 8e625d0a34e6b668674b2e1be526752000d5c462. There were no benchmark performance regressions. 🎉 The [full Conbench report](https://github.com/apache/arrow/runs/65852608981) has more details. It also includes information about 7 possible false positives for unstable benchmarks that are known to sometimes produce them. -- 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]
Re: [PR] GH-49219: [C++][FlightRPC] Enable static ODBC build on macOS [arrow]
kou merged PR #49220: URL: https://github.com/apache/arrow/pull/49220 -- 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]
Re: [PR] GH-49219: [C++][FlightRPC] Enable static ODBC build on macOS [arrow]
alinaliBQ commented on code in PR #49220:
URL: https://github.com/apache/arrow/pull/49220#discussion_r2885907861
##
cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt:
##
@@ -25,29 +25,57 @@ set(ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS
../../example/sqlite_server.cc
../../example/sqlite_tables_schema_batch_reader.cc)
+set(ARROW_FLIGHT_SQL_ODBC_TEST_SRCS
+odbc_test_suite.cc
+odbc_test_suite.h
+columns_test.cc
+connection_attr_test.cc
+connection_info_test.cc
+connection_test.cc
+errors_test.cc
+get_functions_test.cc
+statement_attr_test.cc
+statement_test.cc
+tables_test.cc
+type_info_test.cc
+# Enable Protobuf cleanup after test execution
+# GH-46889: move protobuf_test_util to a more common location
+../../../../engine/substrait/protobuf_test_util.cc)
+
+if(ARROW_TEST_LINKAGE STREQUAL "static")
+ set(ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS arrow_flight_sql_odbc_static
+ ${ARROW_TEST_STATIC_LINK_LIBS})
+else()
+ set(ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS arrow_flight_sql_odbc_shared
+ ${ARROW_TEST_SHARED_LINK_LIBS})
Review Comment:
I needed to replace `ARROW_TEST_LINK_LIBS` with `ARROW_TEST_< STATIC |
SHARED >_LINK_LIBS` here, because `ARROW_TEST_LINK_LIBS`Â contains unneeded
variables. I based my code change on here:
https://github.com/apache/arrow/blob/cfbbf708082c84f3a7cb1246d83ef134c2ac27c4/cpp/src/arrow/acero/CMakeLists.txt#L110-L114
--
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]
Re: [PR] GH-49219: [C++][FlightRPC] Enable static ODBC build on macOS [arrow]
alinaliBQ commented on code in PR #49220:
URL: https://github.com/apache/arrow/pull/49220#discussion_r2885907861
##
cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt:
##
@@ -25,29 +25,57 @@ set(ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS
../../example/sqlite_server.cc
../../example/sqlite_tables_schema_batch_reader.cc)
+set(ARROW_FLIGHT_SQL_ODBC_TEST_SRCS
+odbc_test_suite.cc
+odbc_test_suite.h
+columns_test.cc
+connection_attr_test.cc
+connection_info_test.cc
+connection_test.cc
+errors_test.cc
+get_functions_test.cc
+statement_attr_test.cc
+statement_test.cc
+tables_test.cc
+type_info_test.cc
+# Enable Protobuf cleanup after test execution
+# GH-46889: move protobuf_test_util to a more common location
+../../../../engine/substrait/protobuf_test_util.cc)
+
+if(ARROW_TEST_LINKAGE STREQUAL "static")
+ set(ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS arrow_flight_sql_odbc_static
+ ${ARROW_TEST_STATIC_LINK_LIBS})
+else()
+ set(ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS arrow_flight_sql_odbc_shared
+ ${ARROW_TEST_SHARED_LINK_LIBS})
Review Comment:
I needed to replace `ARROW_TEST_LINK_LIBS` with `ARROW_TEST_< STATIC |
SHARED >_LINK_LIBS` here, because `ARROW_TEST_LINK_LIBS`Â contains unneeded
variables and was causing linking issues. I based my code change on here:
https://github.com/apache/arrow/blob/cfbbf708082c84f3a7cb1246d83ef134c2ac27c4/cpp/src/arrow/acero/CMakeLists.txt#L110-L114
--
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]
Re: [PR] GH-49219: [C++][FlightRPC] Enable static ODBC build on macOS [arrow]
kou commented on PR #49220: URL: https://github.com/apache/arrow/pull/49220#issuecomment-3987930715 Could you check the "ODBC Windows" job failure? https://github.com/apache/arrow/actions/runs/22595843931/job/65465841457?pr=49220 -- 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]
Re: [PR] GH-49219: [C++][FlightRPC] Enable static ODBC build on macOS [arrow]
alinaliBQ commented on PR #49220: URL: https://github.com/apache/arrow/pull/49220#issuecomment-3987239284 @kou I have addressed all comments and rebased onto `main` -- 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]
Re: [PR] GH-49219: [C++][FlightRPC] Enable static ODBC build on macOS [arrow]
alinaliBQ commented on PR #49220: URL: https://github.com/apache/arrow/pull/49220#issuecomment-3961723486 > Could you rebase on main to fix CI failures? I have rebased, I think some of the failures come from my fork repo and it's not due to code changes: ``` unable to get image 'ghcr.io/Bit-Quill/arrow-dev:amd64-cpp-jni-66c0373dc7fca549e5803087b9487edfe3aca0a1': Error response from daemon: invalid reference format: repository name (Bit-Quill/arrow-dev) must be lowercase ``` Link to run: https://github.com/Bit-Quill/arrow/actions/runs/22412737436/job/64890327853 I can turn off the fork repo runs so it doesn't show in the CI -- 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]
Re: [PR] GH-49219: [C++][FlightRPC] Enable static ODBC build on macOS [arrow]
kou commented on code in PR #49220:
URL: https://github.com/apache/arrow/pull/49220#discussion_r2850519168
##
cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt:
##
@@ -25,29 +25,51 @@ set(ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS
../../example/sqlite_server.cc
../../example/sqlite_tables_schema_batch_reader.cc)
+if(ARROW_TEST_LINKAGE STREQUAL "static")
+ set(ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS arrow_flight_sql_odbc_static
+ ${ARROW_TEST_STATIC_LINK_LIBS})
+else()
+ set(ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS arrow_flight_sql_odbc_shared
+ ${ARROW_TEST_SHARED_LINK_LIBS})
+endif()
Review Comment:
Hmm. It seems that switching `STATIC_LINK_LIBS` and `EXTRA_LINK_LIBS` for
static/shared is strange...
Does this work?
```cmake
if(ARROW_TEST_LINKAGE STREQUAL "static")
set(ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS arrow_flight_sql_odbc_static
${ARROW_TEST_LINK_LIBS})
else()
set(ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS arrow_flight_sql_odbc_shared
${ARROW_TEST_LINK_LIBS})
endif()
# ...
add_arrow_test(flight_sql_odbc_test
# ...
STATIC_LINK_LIBS ${ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS}
EXTRA_LINK_LIBS ${ODBC_LIBRARIES} ...)
```
##
.github/workflows/cpp_extra.yml:
##
@@ -395,6 +414,10 @@ jobs:
export ARROW_CMAKE_ARGS="-DODBC_INCLUDE_DIR=$ODBC_INCLUDE_DIR"
export CXXFLAGS="$CXXFLAGS -I$ODBC_INCLUDE_DIR"
ci/scripts/cpp_build.sh $(pwd) $(pwd)/build
+ - name: Check ODBC Dependency
+run: |
+ # ODBC dependency should not include Arrow or third party libraries
+ ! otool -L $(pwd)/build/cpp/debug/libarrow_flight_sql_odbc.dylib |
grep -Ei 'arrow_flight_sql\.|arrow_flight\.|arrow_compute|boost|iodbc|grpc'
Review Comment:
Can we use `archery linking check-dependencies`?
See also:
https://github.com/apache/arrow-java/blob/39b0593ac53c1444d0eb05f698fc3c0aa300f30b/ci/scripts/jni_macos_build.sh#L97-L115
--
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]
Re: [PR] GH-49219: [C++][FlightRPC] Enable static ODBC build on macOS [arrow]
alinaliBQ commented on code in PR #49220:
URL: https://github.com/apache/arrow/pull/49220#discussion_r2849354886
##
cpp/src/arrow/flight/sql/odbc/CMakeLists.txt:
##
@@ -61,33 +66,56 @@ if(WIN32)
list(APPEND ARROW_FLIGHT_SQL_ODBC_SRCS odbc.def install/versioninfo.rc)
endif()
-add_arrow_lib(arrow_flight_sql_odbc
- CMAKE_PACKAGE_NAME
- ArrowFlightSqlOdbc
- PKG_CONFIG_NAME
- arrow-flight-sql-odbc
- OUTPUTS
- ARROW_FLIGHT_SQL_ODBC_LIBRARIES
- SOURCES
- ${ARROW_FLIGHT_SQL_ODBC_SRCS}
- DEPENDENCIES
- arrow_flight_sql
- DEFINITIONS
- UNICODE
- SHARED_LINK_FLAGS
- ${ARROW_VERSION_SCRIPT_FLAGS} # Defined in
cpp/arrow/CMakeLists.txt
- SHARED_LINK_LIBS
- arrow_flight_sql_shared
- SHARED_INSTALL_INTERFACE_LIBS
- ArrowFlight::arrow_flight_sql_shared
- STATIC_LINK_LIBS
- arrow_flight_sql_static
- STATIC_INSTALL_INTERFACE_LIBS
- ArrowFlight::arrow_flight_sql_static
- SHARED_PRIVATE_LINK_LIBS
- ODBC::ODBC
- ${ODBCINST}
- arrow_odbc_spi_impl)
+# On Windows, dynmaic build for ODBC is supported.
+# On unix systems, static build for ODBC is supported, all libraries are
linked statically on unix.
+if(WIN32)
+ add_arrow_lib(arrow_flight_sql_odbc
+CMAKE_PACKAGE_NAME
+ArrowFlightSqlOdbc
+PKG_CONFIG_NAME
+arrow-flight-sql-odbc
+OUTPUTS
+ARROW_FLIGHT_SQL_ODBC_LIBRARIES
+SOURCES
+${ARROW_FLIGHT_SQL_ODBC_SRCS}
+DEFINITIONS
+UNICODE
+SHARED_LINK_FLAGS
+${ARROW_VERSION_SCRIPT_FLAGS} # Defined in
cpp/arrow/CMakeLists.txt
+SHARED_LINK_LIBS
+arrow_flight_sql_shared
+arrow_odbc_spi_impl
+SHARED_INSTALL_INTERFACE_LIBS
+ArrowFlight::arrow_flight_sql_shared
+STATIC_LINK_LIBS
+arrow_flight_sql_static
+STATIC_INSTALL_INTERFACE_LIBS
+ArrowFlight::arrow_flight_sql_static
+SHARED_PRIVATE_LINK_LIBS
+ODBC::ODBC
+${ODBCINST})
+else()
+ # Unix
+ add_arrow_lib(arrow_flight_sql_odbc
+CMAKE_PACKAGE_NAME
+ArrowFlightSqlOdbc
+PKG_CONFIG_NAME
+arrow-flight-sql-odbc
+OUTPUTS
+ARROW_FLIGHT_SQL_ODBC_LIBRARIES
+SOURCES
+${ARROW_FLIGHT_SQL_ODBC_SRCS}
+DEFINITIONS
+UNICODE
+SHARED_LINK_FLAGS
+${ARROW_VERSION_SCRIPT_FLAGS} # Defined in
cpp/arrow/CMakeLists.txt
+STATIC_LINK_LIBS
+iodbc
+ODBC::ODBC
+${ODBCINST}
+SHARED_LINK_LIBS
+arrow_odbc_spi_impl)
+endif()
Review Comment:
Sure, I have changed to use one `add_arrow_lib()` for easier maintenance
##
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##
@@ -3023,8 +3039,8 @@ function(build_cares)
if(APPLE)
# libresolv must be linked from c-ares version 1.16.1
find_library(LIBRESOLV_LIBRARY NAMES resolv libresolv REQUIRED)
-set_target_properties(c-ares::cares PROPERTIES INTERFACE_LINK_LIBRARIES
- "${LIBRESOLV_LIBRARY}")
+set_target_properties(c-ares PROPERTIES INTERFACE_LINK_LIBRARIES
+"${LIBRESOLV_LIBRARY}")
Review Comment:
Yup, fixed
--
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]
Re: [PR] GH-49219: [C++][FlightRPC] Enable static ODBC build on macOS [arrow]
alinaliBQ commented on code in PR #49220:
URL: https://github.com/apache/arrow/pull/49220#discussion_r2843893130
##
.github/workflows/cpp_extra.yml:
##
@@ -430,6 +450,7 @@ jobs:
CMAKE_INSTALL_PREFIX: /usr
VCPKG_BINARY_SOURCES: 'clear;nugettimeout,600;nuget,GitHub,readwrite'
VCPKG_DEFAULT_TRIPLET: x64-windows
+ CMAKE_CXX_STANDARD: "20"
Review Comment:
I have removed this line
##
.github/workflows/cpp_extra.yml:
##
@@ -357,6 +357,10 @@ jobs:
ARROW_BUILD_TESTS: ON
ARROW_FLIGHT_SQL_ODBC: ON
ARROW_HOME: /tmp/local
+ ARROW_DEPENDENCY_USE_SHARED: OFF
+ ARROW_DEPENDENCY_SOURCE: BUNDLED
Review Comment:
yup, fixed list to be in alphabetical order
##
cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt:
##
@@ -25,29 +25,60 @@ set(ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS
../../example/sqlite_server.cc
../../example/sqlite_tables_schema_batch_reader.cc)
-add_arrow_test(flight_sql_odbc_test
- SOURCES
- odbc_test_suite.cc
- odbc_test_suite.h
- columns_test.cc
- connection_attr_test.cc
- connection_info_test.cc
- connection_test.cc
- errors_test.cc
- get_functions_test.cc
- statement_attr_test.cc
- statement_test.cc
- tables_test.cc
- type_info_test.cc
- # Enable Protobuf cleanup after test execution
- # GH-46889: move protobuf_test_util to a more common location
- ../../../../engine/substrait/protobuf_test_util.cc
- ${ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS}
- EXTRA_LINK_LIBS
- ${ODBC_LIBRARIES}
- ${ODBCINST}
- ${SQLite3_LIBRARIES}
- arrow_odbc_spi_impl)
+if(ARROW_TEST_LINKAGE STREQUAL "static")
+ set(ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS arrow_flight_sql_odbc_static
+ ${ARROW_TEST_STATIC_LINK_LIBS})
+else()
+ set(ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS arrow_flight_sql_odbc_shared
+ ${ARROW_TEST_SHARED_LINK_LIBS})
+endif()
+
+set(ARROW_FLIGHT_SQL_ODBC_TEST_SRCS
+odbc_test_suite.cc
+odbc_test_suite.h
+columns_test.cc
+connection_attr_test.cc
+connection_info_test.cc
+connection_test.cc
+errors_test.cc
+get_functions_test.cc
+statement_attr_test.cc
+statement_test.cc
+tables_test.cc
+type_info_test.cc
+# Enable Protobuf cleanup after test execution
+# GH-46889: move protobuf_test_util to a more common location
+../../../../engine/substrait/protobuf_test_util.cc)
+
+# On Windows, dynamic linking ODBC is supported, tests link libraries
dynamically.
+# On unix systems, static linking ODBC is supported, thus tests link libraries
statically.
+if(WIN32)
+ add_arrow_test(flight_sql_odbc_test
+ SOURCES
+ ${ARROW_FLIGHT_SQL_ODBC_TEST_SRCS}
+ ${ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS}
+ DEFINITIONS
+ UNICODE
+ EXTRA_LINK_LIBS
+ ${ODBC_LIBRARIES}
+ ${ODBCINST}
+ ${SQLite3_LIBRARIES}
+ ${ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS})
+elseif(APPLE)
+ # macOS
Review Comment:
Yup, removed this comment
##
cpp/src/arrow/flight/sql/odbc/CMakeLists.txt:
##
@@ -19,6 +19,11 @@
# GH-44792: Arrow will switch to C++ 20
set(CMAKE_CXX_STANDARD 20)
+if(APPLE)
+ # CMAKE_FIND_LIBRARY_SUFFIXES.
+ set(APPEND CMAKE_FIND_LIBRARY_SUFFIXES ".a")
+endif()
+
Review Comment:
I have revered this change, and removed redundant `set(CMAKE_CXX_STANDARD
20)` command.
I thought `set(APPEND CMAKE_FIND_LIBRARY_SUFFIXES .a)` was required for
static build, but after test I think it isn't needed
--
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]
Re: [PR] GH-49219: [C++][FlightRPC] Enable static ODBC build on macOS [arrow]
kou commented on code in PR #49220:
URL: https://github.com/apache/arrow/pull/49220#discussion_r2796882148
##
.github/workflows/cpp_extra.yml:
##
@@ -357,6 +357,10 @@ jobs:
ARROW_BUILD_TESTS: ON
ARROW_FLIGHT_SQL_ODBC: ON
ARROW_HOME: /tmp/local
+ ARROW_DEPENDENCY_USE_SHARED: OFF
+ ARROW_DEPENDENCY_SOURCE: BUNDLED
Review Comment:
Could you keep this list in alphabetical order?
##
.github/workflows/cpp_extra.yml:
##
@@ -430,6 +450,7 @@ jobs:
CMAKE_INSTALL_PREFIX: /usr
VCPKG_BINARY_SOURCES: 'clear;nugettimeout,600;nuget,GitHub,readwrite'
VCPKG_DEFAULT_TRIPLET: x64-windows
+ CMAKE_CXX_STANDARD: "20"
Review Comment:
Why do we need this? We use C++20 by default.
##
cpp/src/arrow/flight/sql/odbc/tests/CMakeLists.txt:
##
@@ -25,29 +25,60 @@ set(ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS
../../example/sqlite_server.cc
../../example/sqlite_tables_schema_batch_reader.cc)
-add_arrow_test(flight_sql_odbc_test
- SOURCES
- odbc_test_suite.cc
- odbc_test_suite.h
- columns_test.cc
- connection_attr_test.cc
- connection_info_test.cc
- connection_test.cc
- errors_test.cc
- get_functions_test.cc
- statement_attr_test.cc
- statement_test.cc
- tables_test.cc
- type_info_test.cc
- # Enable Protobuf cleanup after test execution
- # GH-46889: move protobuf_test_util to a more common location
- ../../../../engine/substrait/protobuf_test_util.cc
- ${ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS}
- EXTRA_LINK_LIBS
- ${ODBC_LIBRARIES}
- ${ODBCINST}
- ${SQLite3_LIBRARIES}
- arrow_odbc_spi_impl)
+if(ARROW_TEST_LINKAGE STREQUAL "static")
+ set(ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS arrow_flight_sql_odbc_static
+ ${ARROW_TEST_STATIC_LINK_LIBS})
+else()
+ set(ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS arrow_flight_sql_odbc_shared
+ ${ARROW_TEST_SHARED_LINK_LIBS})
+endif()
+
+set(ARROW_FLIGHT_SQL_ODBC_TEST_SRCS
+odbc_test_suite.cc
+odbc_test_suite.h
+columns_test.cc
+connection_attr_test.cc
+connection_info_test.cc
+connection_test.cc
+errors_test.cc
+get_functions_test.cc
+statement_attr_test.cc
+statement_test.cc
+tables_test.cc
+type_info_test.cc
+# Enable Protobuf cleanup after test execution
+# GH-46889: move protobuf_test_util to a more common location
+../../../../engine/substrait/protobuf_test_util.cc)
+
+# On Windows, dynamic linking ODBC is supported, tests link libraries
dynamically.
+# On unix systems, static linking ODBC is supported, thus tests link libraries
statically.
+if(WIN32)
+ add_arrow_test(flight_sql_odbc_test
+ SOURCES
+ ${ARROW_FLIGHT_SQL_ODBC_TEST_SRCS}
+ ${ARROW_FLIGHT_SQL_MOCK_SERVER_SRCS}
+ DEFINITIONS
+ UNICODE
+ EXTRA_LINK_LIBS
+ ${ODBC_LIBRARIES}
+ ${ODBCINST}
+ ${SQLite3_LIBRARIES}
+ ${ARROW_FLIGHT_SQL_ODBC_TEST_LINK_LIBS})
+elseif(APPLE)
+ # macOS
Review Comment:
It seems that this is redundant after the `elseif(APPLE)`:
```suggestion
```
##
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##
@@ -3023,8 +3039,8 @@ function(build_cares)
if(APPLE)
# libresolv must be linked from c-ares version 1.16.1
find_library(LIBRESOLV_LIBRARY NAMES resolv libresolv REQUIRED)
-set_target_properties(c-ares::cares PROPERTIES INTERFACE_LINK_LIBRARIES
- "${LIBRESOLV_LIBRARY}")
+set_target_properties(c-ares PROPERTIES INTERFACE_LINK_LIBRARIES
+"${LIBRESOLV_LIBRARY}")
Review Comment:
Could you try this?
```suggestion
target_link_libraries(c-ares INTERFACE ${LIBRESOLV_LIBRARY})
```
##
cpp/src/arrow/flight/sql/odbc/CMakeLists.txt:
##
@@ -61,33 +66,56 @@ if(WIN32)
list(APPEND ARROW_FLIGHT_SQL_ODBC_SRCS odbc.def install/versioninfo.rc)
endif()
-add_arrow_lib(arrow_flight_sql_odbc
- CMAKE_PACKAGE_NAME
- ArrowFlightSqlOdbc
- PKG_CONFIG_NAME
- arrow-flight-sql-odbc
- OUTPUTS
- ARROW_FLIGHT_SQL_ODBC_LIBRARIES
- SOURCES
- ${ARROW_FLIGHT_SQL_ODBC_SRCS}
- DEPENDENCIES
- arrow_flight_sql
- DEFINITIONS
- UNICODE
- SHARED_LINK_FLAGS
- ${ARROW_VERSION_SCRIPT_FLAGS} # Defined in
cpp/arrow/CMakeLists.txt
- SHARED_LINK_LIBS
- arrow_flight_sql_sh
Re: [PR] GH-49219: [C++][FlightRPC] Enable static ODBC build on macOS [arrow]
alinaliBQ commented on PR #49220: URL: https://github.com/apache/arrow/pull/49220#issuecomment-3887942756 Hi @lidavidm and @kou. This PR is ready for review. I’ll be OOO starting Feb 13 and will be back on Feb 23. I will address the review comments once I’m back. -- 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]
Re: [PR] GH-49219: [C++][FlightRPC] Enable static ODBC build on macOS [arrow]
alinaliBQ commented on code in PR #49220:
URL: https://github.com/apache/arrow/pull/49220#discussion_r2795221993
##
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##
@@ -3023,8 +3028,8 @@ function(build_cares)
if(APPLE)
# libresolv must be linked from c-ares version 1.16.1
find_library(LIBRESOLV_LIBRARY NAMES resolv libresolv REQUIRED)
-set_target_properties(c-ares::cares PROPERTIES INTERFACE_LINK_LIBRARIES
- "${LIBRESOLV_LIBRARY}")
+set_target_properties(c-ares PROPERTIES INTERFACE_LINK_LIBRARIES
+"${LIBRESOLV_LIBRARY}")
endif()
Review Comment:
This change fixes error:
```
CMake Error at cmake_modules/ThirdpartyToolchain.cmake:3032
(set_target_properties):
set_target_properties can not be used on an ALIAS target.
```
Without this change, we have failed run with `c-ares::cares` being used.
Log:
https://github.com/Bit-Quill/arrow/actions/runs/21459770235/job/61809740997?pr=153#step:7:540
--
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]
Re: [PR] GH-49219: [C++][FlightRPC] Enable static ODBC build on macOS [arrow]
alinaliBQ commented on code in PR #49220:
URL: https://github.com/apache/arrow/pull/49220#discussion_r2795221993
##
cpp/cmake_modules/ThirdpartyToolchain.cmake:
##
@@ -3023,8 +3028,8 @@ function(build_cares)
if(APPLE)
# libresolv must be linked from c-ares version 1.16.1
find_library(LIBRESOLV_LIBRARY NAMES resolv libresolv REQUIRED)
-set_target_properties(c-ares::cares PROPERTIES INTERFACE_LINK_LIBRARIES
- "${LIBRESOLV_LIBRARY}")
+set_target_properties(c-ares PROPERTIES INTERFACE_LINK_LIBRARIES
+"${LIBRESOLV_LIBRARY}")
endif()
Review Comment:
This change fixes error:
```
CMake Error at cmake_modules/ThirdpartyToolchain.cmake:3032
(set_target_properties):
set_target_properties can not be used on an ALIAS target.
```
Without this change, we have failed run with c-ares::cares. Log:
https://github.com/Bit-Quill/arrow/actions/runs/21459770235/job/61809740997?pr=153#step:7:540
--
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]
[PR] GH-49219: [C++][FlightRPC] Enable static ODBC build on macOS [arrow]
alinaliBQ opened a new pull request, #49220: URL: https://github.com/apache/arrow/pull/49220 ### Rationale for this change #49219 ### What changes are included in this PR? - Add unix-specific CMake build commands in CMakeLists.txt to link ODBC DYLIB statically to dependencies. - Header file changes. - Test fixes for static macOS linking ### Are these changes tested? Build is tested in CI. ### Are there any user-facing changes? N/A -- 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]
