From 682b8b66fb22109281c57b50b77b8f56fc7d43aa Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Mon, 9 May 2022 17:03:47 +0200 Subject: [PATCH 01/27] nolonger remove gtest --- ci/appveyor-cpp-setup.bat | 5 ----- dev/release/verify-release-candidate.bat | 4 ---- 2 files changed, 9 deletions(-) diff --git a/ci/appveyor-cpp-setup.bat b/ci/appveyor-cpp-setup.bat index 936306296f6..db2bc250633 100644 --- a/ci/appveyor-cpp-setup.bat +++ b/ci/appveyor-cpp-setup.bat @@ -79,11 +79,6 @@ if "%JOB%" NEQ "Build_Debug" ( "python=%PYTHON%" ^ || exit /B - @rem On Windows, GTest is always bundled from source instead of using - @rem conda binaries, avoid any interference between the two versions. - if "%JOB%" == "Toolchain" ( - mamba uninstall -n arrow -q -y -c conda-forge gtest || exit /B - ) ) @rem diff --git a/dev/release/verify-release-candidate.bat b/dev/release/verify-release-candidate.bat index 30bb69a2253..396f0475e29 100644 --- a/dev/release/verify-release-candidate.bat +++ b/dev/release/verify-release-candidate.bat @@ -69,10 +69,6 @@ call conda create --no-shortcuts -c conda-forge -f -q -y -p %_VERIFICATION_CONDA call activate %_VERIFICATION_CONDA_ENV% || exit /B 1 -@rem With MSVC we always bundle a more recent GTest which causes a link conflict -@rem for more context, see https://issues.apache.org/jira/browse/ARROW-15378 -call conda remove -y gtest gmock || exit /B 1 - set GENERATOR=Visual Studio 16 2019 set ARCHITECTURE=x64 set CONFIGURATION=release From 18804300c15f44e73413b2399b3510eb646493d9 Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Mon, 9 May 2022 19:00:50 +0200 Subject: [PATCH 02/27] Add check for conda and set gtest to use bundled --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 158be06d7d9..e9235897c8e 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -78,6 +78,11 @@ set(ARROW_THIRDPARTY_DEPENDENCIES ZLIB zstd) +# cmake does not detect the gtest version installed by conda so we havte to use the bundled version +if(ARROW_DEPENDENCY_SOURCE STREQUAL "CONDA" AND MSVC AND "${GTest_SOURCE}" STREQUAL "") + set(GTest_SOURCE "BUNDLED") +endif() + # For backward compatibility. We use "BOOST_SOURCE" if "Boost_SOURCE" # isn't specified and "BOOST_SOURCE" is specified. # We renamed "BOOST" dependency name to "Boost" in 3.0.0 because From 4efaa415cc1760ec66d669bae23ba1653c0005ff Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Mon, 9 May 2022 22:41:05 +0200 Subject: [PATCH 03/27] add cmake version check for gtest --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index e9235897c8e..5e78bc01b52 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -78,10 +78,6 @@ set(ARROW_THIRDPARTY_DEPENDENCIES ZLIB zstd) -# cmake does not detect the gtest version installed by conda so we havte to use the bundled version -if(ARROW_DEPENDENCY_SOURCE STREQUAL "CONDA" AND MSVC AND "${GTest_SOURCE}" STREQUAL "") - set(GTest_SOURCE "BUNDLED") -endif() # For backward compatibility. We use "BOOST_SOURCE" if "Boost_SOURCE" # isn't specified and "BOOST_SOURCE" is specified. @@ -2054,11 +2050,16 @@ macro(build_gtest) endmacro() if(ARROW_TESTING) + if(CMAKE_VERSION VERSION_LESS 3.23) + set(GTEST_USE_CONFIG TRUE) + else() + set(GTEST_USE_CONFIG FALSE) + endif() resolve_dependency(GTest REQUIRED_VERSION 1.10.0 USE_CONFIG - TRUE) + ${GTEST_USE_CONFIG}) if(NOT GTEST_VENDORED) # TODO(wesm): This logic does not work correctly with the MSVC static libraries From 0763fbe2f6d372c68fc0db1f8697841a58aa1ddf Mon Sep 17 00:00:00 2001 From: Jacob Wujciak-Jens Date: Thu, 12 May 2022 18:52:09 +0200 Subject: [PATCH 04/27] unpin cmake appveyor --- ci/appveyor-cpp-setup.bat | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/appveyor-cpp-setup.bat b/ci/appveyor-cpp-setup.bat index db2bc250633..123bc4a3f66 100644 --- a/ci/appveyor-cpp-setup.bat +++ b/ci/appveyor-cpp-setup.bat @@ -71,7 +71,7 @@ if "%JOB%" NEQ "Build_Debug" ( mamba create -n arrow -q -y -c conda-forge ^ --file=ci\conda_env_python.txt ^ %CONDA_PACKAGES% ^ - "cmake=3.17" ^ + "cmake" ^ "ninja" ^ "nomkl" ^ "pandas" ^ From 507579a096bfae8e537419e801fcc022871db26e Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 13 May 2022 09:24:52 +0900 Subject: [PATCH 05/27] Specify CC/CXX to use cl.exe explicitly --- ci/appveyor-cpp-setup.bat | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ci/appveyor-cpp-setup.bat b/ci/appveyor-cpp-setup.bat index 123bc4a3f66..35e550edb13 100644 --- a/ci/appveyor-cpp-setup.bat +++ b/ci/appveyor-cpp-setup.bat @@ -91,6 +91,8 @@ if defined need_vcvarsall ( exit /B ) call "C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Auxiliary\Build\vcvarsall.bat" amd64 + set CC=cl.exe + set CXX=cl.exe ) @rem @@ -123,4 +125,4 @@ move tzdata %USERPROFILE%\Downloads\tzdata @rem Also need Windows timezone mapping curl https://raw.githubusercontent.com/unicode-org/cldr/master/common/supplemental/windowsZones.xml ^ --output %USERPROFILE%\Downloads\tzdata\windowsZones.xml -@rem (Doc section: Download timezone database) \ No newline at end of file +@rem (Doc section: Download timezone database) From fa0065991b3035534d9afbbce1f08c0a6fa11737 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 13 May 2022 09:28:09 +0900 Subject: [PATCH 06/27] Fix style --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 5e78bc01b52..f99751463ff 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -78,7 +78,6 @@ set(ARROW_THIRDPARTY_DEPENDENCIES ZLIB zstd) - # For backward compatibility. We use "BOOST_SOURCE" if "Boost_SOURCE" # isn't specified and "BOOST_SOURCE" is specified. # We renamed "BOOST" dependency name to "Boost" in 3.0.0 because @@ -2051,10 +2050,10 @@ endmacro() if(ARROW_TESTING) if(CMAKE_VERSION VERSION_LESS 3.23) - set(GTEST_USE_CONFIG TRUE) - else() - set(GTEST_USE_CONFIG FALSE) - endif() + set(GTEST_USE_CONFIG TRUE) + else() + set(GTEST_USE_CONFIG FALSE) + endif() resolve_dependency(GTest REQUIRED_VERSION 1.10.0 From 026ec8a3748648f80643dffad290a5578432e3f0 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 13 May 2022 09:50:27 +0900 Subject: [PATCH 07/27] Specify Release explicitly --- ci/appveyor-cpp-build.bat | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ci/appveyor-cpp-build.bat b/ci/appveyor-cpp-build.bat index 287197458e1..986244e4de4 100644 --- a/ci/appveyor-cpp-build.bat +++ b/ci/appveyor-cpp-build.bat @@ -121,7 +121,7 @@ cmake -G "%GENERATOR%" %CMAKE_ARGS% ^ -DPARQUET_BUILD_EXECUTABLES=ON ^ -DPARQUET_REQUIRE_ENCRYPTION=ON ^ .. || exit /B -cmake --build . --target install --config %CONFIGURATION% || exit /B +cmake --build . --target install --config Release || exit /B @rem Needed so arrow-python-test.exe works set OLD_PYTHONHOME=%PYTHONHOME% From 30af18bcb357a1a9d4b3b5351c191773d4baca7c Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 13 May 2022 11:06:49 +0900 Subject: [PATCH 08/27] Add workaround for GTEST_LINKED_AS_SHARED_LIBRARY for FingGtest.cmake --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 48 +++++++-------------- 1 file changed, 16 insertions(+), 32 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index f99751463ff..4e5b1928ef1 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -1972,8 +1972,6 @@ macro(build_gtest) -DCMAKE_MACOSX_RPATH=OFF) set(GMOCK_INCLUDE_DIR "${GTEST_PREFIX}/include") - add_definitions(-DGTEST_LINKED_AS_SHARED_LIBRARY=1) - if(MSVC AND NOT ARROW_USE_STATIC_CRT) set(GTEST_CMAKE_ARGS ${GTEST_CMAKE_ARGS} -Dgtest_force_shared_crt=ON) endif() @@ -2031,17 +2029,21 @@ macro(build_gtest) add_library(GTest::gtest SHARED IMPORTED) set_target_properties(GTest::gtest PROPERTIES ${_GTEST_IMPORTED_TYPE} "${GTEST_SHARED_LIB}" + INTERFACE_COMPILE_DEFINITIONS "GTEST_LINKED_AS_SHARED_LIBRARY=1" INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIR}") add_library(GTest::gtest_main SHARED IMPORTED) set_target_properties(GTest::gtest_main PROPERTIES ${_GTEST_IMPORTED_TYPE} "${GTEST_MAIN_SHARED_LIB}" - INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIR}") + INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIR}" + INTERFACE_LINK_LIBRARIES Gtest::gtest) add_library(GTest::gmock SHARED IMPORTED) set_target_properties(GTest::gmock PROPERTIES ${_GTEST_IMPORTED_TYPE} "${GMOCK_SHARED_LIB}" - INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIR}") + INTERFACE_COMPILE_DEFINITIONS "GMOCK_LINKED_AS_SHARED_LIBRARY=1" + INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIR}" + INTERFACE_LINK_LIBRARIES Gtest::gtest) add_dependencies(toolchain-tests googletest_ep) add_dependencies(GTest::gtest googletest_ep) add_dependencies(GTest::gtest_main googletest_ep) @@ -2059,34 +2061,16 @@ if(ARROW_TESTING) 1.10.0 USE_CONFIG ${GTEST_USE_CONFIG}) - - if(NOT GTEST_VENDORED) - # TODO(wesm): This logic does not work correctly with the MSVC static libraries - # built for the shared crt - - # set(CMAKE_REQUIRED_LIBRARIES GTest::GTest GTest::Main GTest::GMock) - # CHECK_CXX_SOURCE_COMPILES(" - # #include - # #include - - # class A { - # public: - # int run() const { return 1; } - # }; - - # class B : public A { - # public: - # MOCK_CONST_METHOD0(run, int()); - # }; - - # TEST(Base, Test) { - # B b; - # }" GTEST_COMPILES_WITHOUT_MACRO) - # if (NOT GTEST_COMPILES_WITHOUT_MACRO) - # message(STATUS "Setting GTEST_LINKED_AS_SHARED_LIBRARY=1 on GTest::GTest") - # add_compile_definitions("GTEST_LINKED_AS_SHARED_LIBRARY=1") - # endif() - # set(CMAKE_REQUIRED_LIBRARIES) + if(NOT GTEST_VENDORED AND MSVC) + # CMake's FindGtest.cmake doesn't recognize lib/gtest-md.lib + # installed by Conda as a shared library because it just checks + # .dll suffix. We need GTEST_LINKED_AS_SHARED_LIBRARY=1 to use + # testing::Test as a parent of our arrow::flight::FlightTest in + # arrow_flight_test.dll. + set_target_properties(GTest::gtest + PROPERTIES INTERFACE_COMPILE_DEFINITIONS "GTEST_LINKED_AS_SHARED_LIBRARY=1") + set_target_properties(GTest::gmock + PROPERTIES INTERFACE_COMPILE_DEFINITIONS "GMOCK_LINKED_AS_SHARED_LIBRARY=1") endif() endif() From 182698f1f456b607e7b1de9163099cb16b96156e Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 13 May 2022 11:09:01 +0900 Subject: [PATCH 09/27] Fix format --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 4e5b1928ef1..3a33a255183 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -2029,7 +2029,8 @@ macro(build_gtest) add_library(GTest::gtest SHARED IMPORTED) set_target_properties(GTest::gtest PROPERTIES ${_GTEST_IMPORTED_TYPE} "${GTEST_SHARED_LIB}" - INTERFACE_COMPILE_DEFINITIONS "GTEST_LINKED_AS_SHARED_LIBRARY=1" + INTERFACE_COMPILE_DEFINITIONS + "GTEST_LINKED_AS_SHARED_LIBRARY=1" INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIR}") add_library(GTest::gtest_main SHARED IMPORTED) @@ -2041,7 +2042,8 @@ macro(build_gtest) add_library(GTest::gmock SHARED IMPORTED) set_target_properties(GTest::gmock PROPERTIES ${_GTEST_IMPORTED_TYPE} "${GMOCK_SHARED_LIB}" - INTERFACE_COMPILE_DEFINITIONS "GMOCK_LINKED_AS_SHARED_LIBRARY=1" + INTERFACE_COMPILE_DEFINITIONS + "GMOCK_LINKED_AS_SHARED_LIBRARY=1" INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIR}" INTERFACE_LINK_LIBRARIES Gtest::gtest) add_dependencies(toolchain-tests googletest_ep) @@ -2067,10 +2069,10 @@ if(ARROW_TESTING) # .dll suffix. We need GTEST_LINKED_AS_SHARED_LIBRARY=1 to use # testing::Test as a parent of our arrow::flight::FlightTest in # arrow_flight_test.dll. - set_target_properties(GTest::gtest - PROPERTIES INTERFACE_COMPILE_DEFINITIONS "GTEST_LINKED_AS_SHARED_LIBRARY=1") - set_target_properties(GTest::gmock - PROPERTIES INTERFACE_COMPILE_DEFINITIONS "GMOCK_LINKED_AS_SHARED_LIBRARY=1") + set_target_properties(GTest::gtest PROPERTIES INTERFACE_COMPILE_DEFINITIONS + "GTEST_LINKED_AS_SHARED_LIBRARY=1") + set_target_properties(GTest::gmock PROPERTIES INTERFACE_COMPILE_DEFINITIONS + "GMOCK_LINKED_AS_SHARED_LIBRARY=1") endif() endif() From fbcc332554ba72ef3e2ea6657cd99df9e3edcf4a Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 13 May 2022 11:19:08 +0900 Subject: [PATCH 10/27] Fix a typo --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 3a33a255183..30be8cf1cf5 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -2037,7 +2037,7 @@ macro(build_gtest) set_target_properties(GTest::gtest_main PROPERTIES ${_GTEST_IMPORTED_TYPE} "${GTEST_MAIN_SHARED_LIB}" INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIR}" - INTERFACE_LINK_LIBRARIES Gtest::gtest) + INTERFACE_LINK_LIBRARIES GTest::gtest) add_library(GTest::gmock SHARED IMPORTED) set_target_properties(GTest::gmock @@ -2045,7 +2045,7 @@ macro(build_gtest) INTERFACE_COMPILE_DEFINITIONS "GMOCK_LINKED_AS_SHARED_LIBRARY=1" INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIR}" - INTERFACE_LINK_LIBRARIES Gtest::gtest) + INTERFACE_LINK_LIBRARIES GTest::gtest) add_dependencies(toolchain-tests googletest_ep) add_dependencies(GTest::gtest googletest_ep) add_dependencies(GTest::gtest_main googletest_ep) From 6e085a806c2c61428b82b99d3bda6bd989967dc6 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 13 May 2022 11:53:17 +0900 Subject: [PATCH 11/27] Disable fast_finish --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 8342dbf6cb3..49ec9afb937 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -32,7 +32,7 @@ cache: - C:\Users\Appveyor\clcache1 matrix: - fast_finish: true + fast_finish: false environment: global: From fcdf7706f1bdac891a5ed3b4ef4a20922dedd8c5 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 13 May 2022 12:56:43 +0900 Subject: [PATCH 12/27] Revert GTest::gtest link --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 30be8cf1cf5..a7a213283d7 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -2036,16 +2036,14 @@ macro(build_gtest) add_library(GTest::gtest_main SHARED IMPORTED) set_target_properties(GTest::gtest_main PROPERTIES ${_GTEST_IMPORTED_TYPE} "${GTEST_MAIN_SHARED_LIB}" - INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIR}" - INTERFACE_LINK_LIBRARIES GTest::gtest) + INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIR}") add_library(GTest::gmock SHARED IMPORTED) set_target_properties(GTest::gmock PROPERTIES ${_GTEST_IMPORTED_TYPE} "${GMOCK_SHARED_LIB}" INTERFACE_COMPILE_DEFINITIONS "GMOCK_LINKED_AS_SHARED_LIBRARY=1" - INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIR}" - INTERFACE_LINK_LIBRARIES GTest::gtest) + INTERFACE_INCLUDE_DIRECTORIES "${GTEST_INCLUDE_DIR}") add_dependencies(toolchain-tests googletest_ep) add_dependencies(GTest::gtest googletest_ep) add_dependencies(GTest::gtest_main googletest_ep) From 38859668ab882c79f6325d782bb3d7528d1c092a Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 13 May 2022 14:48:28 +0900 Subject: [PATCH 13/27] Use static link when found GTest is static library on Windows --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 11 -------- cpp/src/arrow/flight/CMakeLists.txt | 29 +++++++++++++++++++++ 2 files changed, 29 insertions(+), 11 deletions(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index a7a213283d7..76c06dcc649 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -2061,17 +2061,6 @@ if(ARROW_TESTING) 1.10.0 USE_CONFIG ${GTEST_USE_CONFIG}) - if(NOT GTEST_VENDORED AND MSVC) - # CMake's FindGtest.cmake doesn't recognize lib/gtest-md.lib - # installed by Conda as a shared library because it just checks - # .dll suffix. We need GTEST_LINKED_AS_SHARED_LIBRARY=1 to use - # testing::Test as a parent of our arrow::flight::FlightTest in - # arrow_flight_test.dll. - set_target_properties(GTest::gtest PROPERTIES INTERFACE_COMPILE_DEFINITIONS - "GTEST_LINKED_AS_SHARED_LIBRARY=1") - set_target_properties(GTest::gmock PROPERTIES INTERFACE_COMPILE_DEFINITIONS - "GMOCK_LINKED_AS_SHARED_LIBRARY=1") - endif() endif() macro(build_benchmark) diff --git a/cpp/src/arrow/flight/CMakeLists.txt b/cpp/src/arrow/flight/CMakeLists.txt index c09159eb151..d4525f1c11b 100644 --- a/cpp/src/arrow/flight/CMakeLists.txt +++ b/cpp/src/arrow/flight/CMakeLists.txt @@ -40,6 +40,31 @@ if(NOT ARROW_GRPC_USE_SHARED) "static" PARENT_SCOPE) endif() +if(ARROW_TESTING) + if(WIN32) + # We need to use the same library type for arrow_flight_testing + # and Google Test because arrow_flight_testing has + # arrow::flight:FlightTest that inherits testing::Test. If + # arrow_flight_testing's library and gtest's library type are + # different such as arrow_flight_testing is a shared library and + # gtest is a static library, link error is occurred. + get_target_property(GTEST_TYPE GTest::gtest TYPE) + if(GTEST_TYPE STREQUAL "SHARED_LIBRARY") + set(ARROW_FLIGHT_TESTING_BUILD_SHARED TRUE) + set(ARROW_FLIGHT_TESTING_BUILD_STATIC FALSE) + else() + set(ARROW_FLIGHT_TESTING_BUILD_SHARED FALSE) + set(ARROW_FLIGHT_TESTING_BUILD_STATIC TRUE) + message(STATUS "Linking Arrow Flight tests statically due to static GTest") + set(ARROW_FLIGHT_TEST_LINKAGE + "static" + PARENT_SCOPE) + endif() + else() + set(ARROW_FLIGHT_TESTING_BUILD_SHARED ${ARROW_BUILD_SHARED}) + set(ARROW_FLIGHT_TESTING_BUILD_STATIC ${ARROW_BUILD_STATIC}) + endif() +endif() set(ARROW_FLIGHT_TEST_INTERFACE_LIBS) if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static") @@ -240,6 +265,10 @@ endforeach() # Define arrow_flight_testing library if(ARROW_TESTING) add_arrow_lib(arrow_flight_testing + BUILD_SHARED + ${ARROW_FLIGHT_TESTING_BUILD_SHARED} + BUILD_STATIC + ${ARROW_FLIGHT_TESTING_BUILD_STATIC} CMAKE_PACKAGE_NAME ArrowFlightTesting PKG_CONFIG_NAME From 6cecac71c984b93b9b257d9a80507dbb5d2d4a3d Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Fri, 13 May 2022 16:45:01 +0900 Subject: [PATCH 14/27] Debug --- ci/appveyor-cpp-build.bat | 2 +- cpp/src/arrow/flight/CMakeLists.txt | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/ci/appveyor-cpp-build.bat b/ci/appveyor-cpp-build.bat index 986244e4de4..ebd6e0dbe3d 100644 --- a/ci/appveyor-cpp-build.bat +++ b/ci/appveyor-cpp-build.bat @@ -117,7 +117,7 @@ cmake -G "%GENERATOR%" %CMAKE_ARGS% ^ -DCMAKE_CXX_FLAGS_RELEASE="/MD /Od /UNDEBUG" ^ -DCMAKE_INSTALL_PREFIX=%CONDA_PREFIX%\Library ^ -DCMAKE_UNITY_BUILD=ON ^ - -DCMAKE_VERBOSE_MAKEFILE=OFF ^ + -DCMAKE_VERBOSE_MAKEFILE=ON ^ -DPARQUET_BUILD_EXECUTABLES=ON ^ -DPARQUET_REQUIRE_ENCRYPTION=ON ^ .. || exit /B diff --git a/cpp/src/arrow/flight/CMakeLists.txt b/cpp/src/arrow/flight/CMakeLists.txt index d4525f1c11b..ea6c5894b1c 100644 --- a/cpp/src/arrow/flight/CMakeLists.txt +++ b/cpp/src/arrow/flight/CMakeLists.txt @@ -67,6 +67,7 @@ if(ARROW_TESTING) endif() set(ARROW_FLIGHT_TEST_INTERFACE_LIBS) +message("ARROW_FLIGHT_TEST_LINKAGE: ${ARROW_FLIGHT_TEST_LINKAGE}") if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static") set(ARROW_FLIGHT_TEST_LINK_LIBS arrow_flight_static arrow_flight_testing_static ${ARROW_FLIGHT_STATIC_LINK_LIBS} @@ -83,6 +84,7 @@ else() list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS arrow_cuda_shared) endif() endif() +message("ARROW_FLIGHT_TEST_LINK_LIBS: ${ARROW_FLIGHT_TEST_LINK_LIBS}") list(APPEND ARROW_FLIGHT_TEST_INTERFACE_LIBS Boost::headers From 28561b3c17124c6c4fce8cde7c57b009d6e8485c Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Sat, 14 May 2022 05:51:43 +0900 Subject: [PATCH 15/27] Remove needless PARENT_SCOPE --- cpp/src/arrow/flight/CMakeLists.txt | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/cpp/src/arrow/flight/CMakeLists.txt b/cpp/src/arrow/flight/CMakeLists.txt index ea6c5894b1c..ce9d93dba1b 100644 --- a/cpp/src/arrow/flight/CMakeLists.txt +++ b/cpp/src/arrow/flight/CMakeLists.txt @@ -25,20 +25,14 @@ if(WIN32) list(APPEND ARROW_FLIGHT_LINK_LIBS ws2_32.lib) endif() -set(ARROW_FLIGHT_TEST_LINKAGE - "${ARROW_TEST_LINKAGE}" - PARENT_SCOPE) +set(ARROW_FLIGHT_TEST_LINKAGE "${ARROW_TEST_LINKAGE}") if(Protobuf_USE_STATIC_LIBS) message(STATUS "Linking Arrow Flight tests statically due to static Protobuf") - set(ARROW_FLIGHT_TEST_LINKAGE - "static" - PARENT_SCOPE) + set(ARROW_FLIGHT_TEST_LINKAGE "static") endif() if(NOT ARROW_GRPC_USE_SHARED) message(STATUS "Linking Arrow Flight tests statically due to static gRPC") - set(ARROW_FLIGHT_TEST_LINKAGE - "static" - PARENT_SCOPE) + set(ARROW_FLIGHT_TEST_LINKAGE "static") endif() if(ARROW_TESTING) if(WIN32) @@ -56,9 +50,7 @@ if(ARROW_TESTING) set(ARROW_FLIGHT_TESTING_BUILD_SHARED FALSE) set(ARROW_FLIGHT_TESTING_BUILD_STATIC TRUE) message(STATUS "Linking Arrow Flight tests statically due to static GTest") - set(ARROW_FLIGHT_TEST_LINKAGE - "static" - PARENT_SCOPE) + set(ARROW_FLIGHT_TEST_LINKAGE "static") endif() else() set(ARROW_FLIGHT_TESTING_BUILD_SHARED ${ARROW_BUILD_SHARED}) @@ -93,11 +85,6 @@ list(APPEND GTest::gtest GTest::gmock) -# Needed for Flight SQL and integration -set(ARROW_FLIGHT_TEST_LINK_LIBS - ${ARROW_FLIGHT_TEST_LINK_LIBS} - PARENT_SCOPE) - # TODO(wesm): Protobuf shared vs static linking set(FLIGHT_PROTO_PATH "${ARROW_SOURCE_DIR}/../format") From e2a27650117d50e07a7ea54e195870c17e7e6f63 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Sat, 14 May 2022 06:07:24 +0900 Subject: [PATCH 16/27] Remove a needless condition --- cpp/cmake_modules/BuildUtils.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/cmake_modules/BuildUtils.cmake b/cpp/cmake_modules/BuildUtils.cmake index f3e3269d4fe..c153f3ab889 100644 --- a/cpp/cmake_modules/BuildUtils.cmake +++ b/cpp/cmake_modules/BuildUtils.cmake @@ -382,7 +382,7 @@ function(ADD_ARROW_LIB LIB_NAME) set(LIB_NAME_STATIC ${LIB_NAME}) endif() - if(ARROW_BUILD_STATIC AND WIN32) + if(WIN32) target_compile_definitions(${LIB_NAME}_static PUBLIC ARROW_STATIC) target_compile_definitions(${LIB_NAME}_static PUBLIC ARROW_FLIGHT_STATIC) endif() From 5f27e8d290606cb064ea2ed8ca24f39d72a97a5b Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Sat, 14 May 2022 06:56:31 +0900 Subject: [PATCH 17/27] Fallback to arrow_flight_shared --- cpp/src/arrow/flight/CMakeLists.txt | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/cpp/src/arrow/flight/CMakeLists.txt b/cpp/src/arrow/flight/CMakeLists.txt index ce9d93dba1b..8b5df0af96b 100644 --- a/cpp/src/arrow/flight/CMakeLists.txt +++ b/cpp/src/arrow/flight/CMakeLists.txt @@ -61,9 +61,16 @@ endif() set(ARROW_FLIGHT_TEST_INTERFACE_LIBS) message("ARROW_FLIGHT_TEST_LINKAGE: ${ARROW_FLIGHT_TEST_LINKAGE}") if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static") - set(ARROW_FLIGHT_TEST_LINK_LIBS - arrow_flight_static arrow_flight_testing_static ${ARROW_FLIGHT_STATIC_LINK_LIBS} - ${ARROW_TEST_LINK_LIBS}) + if(ARROW_BUILD_STATIC) + set(ARROW_FLIGHT_TEST_LINK_LIBS arrow_flight_static) + else() + set(ARROW_FLIGHT_TEST_LINK_LIBS arrow_flight_shared) + endif() + list(APPEND + ARROW_FLIGHT_TEST_LINK_LIBS + arrow_flight_testing_static + ${ARROW_FLIGHT_STATIC_LINK_LIBS} + ${ARROW_TEST_LINK_LIBS}) if(ARROW_CUDA) list(APPEND ARROW_FLIGHT_TEST_INTERFACE_LIBS arrow_cuda_static) list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS arrow_cuda_static) From 931c3f24ee5778a16c93605ca566b1d1db4b0d1e Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Sat, 14 May 2022 06:58:28 +0900 Subject: [PATCH 18/27] Fallback to arrow_flight_testing_shared --- cpp/src/arrow/flight/CMakeLists.txt | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/cpp/src/arrow/flight/CMakeLists.txt b/cpp/src/arrow/flight/CMakeLists.txt index 8b5df0af96b..d94a1d5f58c 100644 --- a/cpp/src/arrow/flight/CMakeLists.txt +++ b/cpp/src/arrow/flight/CMakeLists.txt @@ -66,10 +66,12 @@ if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static") else() set(ARROW_FLIGHT_TEST_LINK_LIBS arrow_flight_shared) endif() - list(APPEND - ARROW_FLIGHT_TEST_LINK_LIBS - arrow_flight_testing_static - ${ARROW_FLIGHT_STATIC_LINK_LIBS} + if(ARROW_FLIGHT_TESTING_BUILD_STATIC) + list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS arrow_flight_testing_static) + else() + list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS arrow_flight_testing_shared) + endif() + list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS ${ARROW_FLIGHT_STATIC_LINK_LIBS} ${ARROW_TEST_LINK_LIBS}) if(ARROW_CUDA) list(APPEND ARROW_FLIGHT_TEST_INTERFACE_LIBS arrow_cuda_static) From bafdbebdd0a01447d52fe4373bb3c16d551b1af9 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Sun, 15 May 2022 06:47:45 +0900 Subject: [PATCH 19/27] Use shared if static isn't built --- cpp/src/arrow/flight/CMakeLists.txt | 32 +++++++++++++------ .../flight/integration_tests/CMakeLists.txt | 12 ++----- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/cpp/src/arrow/flight/CMakeLists.txt b/cpp/src/arrow/flight/CMakeLists.txt index d94a1d5f58c..f62462c4e8c 100644 --- a/cpp/src/arrow/flight/CMakeLists.txt +++ b/cpp/src/arrow/flight/CMakeLists.txt @@ -71,8 +71,7 @@ if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static") else() list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS arrow_flight_testing_shared) endif() - list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS ${ARROW_FLIGHT_STATIC_LINK_LIBS} - ${ARROW_TEST_LINK_LIBS}) + list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS ${ARROW_TEST_LINK_LIBS}) if(ARROW_CUDA) list(APPEND ARROW_FLIGHT_TEST_INTERFACE_LIBS arrow_cuda_static) list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS arrow_cuda_static) @@ -262,6 +261,25 @@ endforeach() # Define arrow_flight_testing library if(ARROW_TESTING) + if(ARROW_BUILD_SHARED AND ARROW_BUILD_STATIC) + set(ARROW_FLIGHT_TESTING_SHARED_LINK_LIBS arrow_shared arrow_flight_shared) + set(ARROW_FLIGHT_TESTING_STATIC_LINK_LIBS arrow_static arrow_flight_static) + elseif(ARROW_BUILD_SHARED) + set(ARROW_FLIGHT_TESTING_SHARED_LINK_LIBS arrow_shared arrow_flight_shared) + set(ARROW_FLIGHT_TESTING_STATIC_LINK_LIBS arrow_shared arrow_flight_shared) + else() + set(ARROW_FLIGHT_TESTING_SHARED_LINK_LIBS arrow_static arrow_flight_static) + set(ARROW_FLIGHT_TESTING_STATIC_LINK_LIBS arrow_static arrow_flight_static) + endif() + if(ARROW_TEST_LINKAGE STREQUAL "shared") + list(APPEND ARROW_FLIGHT_TESTING_SHARED_LINK_LIBS arrow_testing_shared) + list(APPEND ARROW_FLIGHT_TESTING_STATIC_LINK_LIBS arrow_testing_shared) + else() + list(APPEND ARROW_FLIGHT_TESTING_SHARED_LINK_LIBS arrow_testing_static) + list(APPEND ARROW_FLIGHT_TESTING_STATIC_LINK_LIBS arrow_testing_static) + endif() + list(APPEND ARROW_FLIGHT_TESTING_SHARED_LINK_LIBS ${ARROW_FLIGHT_TEST_INTERFACE_LIBS}) + list(APPEND ARROW_FLIGHT_TESTING_STATIC_LINK_LIBS ${ARROW_FLIGHT_TEST_INTERFACE_LIBS}) add_arrow_lib(arrow_flight_testing BUILD_SHARED ${ARROW_FLIGHT_TESTING_BUILD_SHARED} @@ -281,15 +299,9 @@ if(ARROW_TESTING) flight_grpc_gen arrow_dependencies SHARED_LINK_LIBS - arrow_shared - arrow_flight_shared - arrow_testing_shared - ${ARROW_FLIGHT_TEST_INTERFACE_LIBS} + ${ARROW_FLIGHT_TESTING_SHARED_LINK_LIBS} STATIC_LINK_LIBS - arrow_static - arrow_flight_static - arrow_testing_static - ${ARROW_FLIGHT_TEST_INTERFACE_LIBS} + ${ARROW_FLIGHT_TESTING_STATIC_LINK_LIBS} PRIVATE_INCLUDES "${Protobuf_INCLUDE_DIRS}") diff --git a/cpp/src/arrow/flight/integration_tests/CMakeLists.txt b/cpp/src/arrow/flight/integration_tests/CMakeLists.txt index c7a8c4fe459..faff3e74c80 100644 --- a/cpp/src/arrow/flight/integration_tests/CMakeLists.txt +++ b/cpp/src/arrow/flight/integration_tests/CMakeLists.txt @@ -17,16 +17,10 @@ add_custom_target(arrow_flight_integration_tests) -if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static") - set(ARROW_FLIGHT_TEST_LINK_LIBS - arrow_flight_static - arrow_flight_testing_static - arrow_flight_sql_static - ${ARROW_FLIGHT_STATIC_LINK_LIBS} - ${ARROW_FLIGHT_TEST_LINK_LIBS}) +if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static" AND ARROW_BUILD_STATIC) + list(INSERT ARROW_FLIGHT_TEST_LINK_LIBS 0 arrow_flight_sql_static) else() - set(ARROW_FLIGHT_TEST_LINK_LIBS arrow_flight_shared arrow_flight_testing_shared - arrow_flight_sql_shared ${ARROW_FLIGHT_TEST_LINK_LIBS}) + list(INSERT ARROW_FLIGHT_TEST_LINK_LIBS 0 arrow_flight_sql_shared) endif() add_executable(flight-test-integration-server test_integration_server.cc From 74ae6560b447c591f14a4c10a9613282552e8f9c Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Sun, 15 May 2022 07:05:39 +0900 Subject: [PATCH 20/27] Set parallel level by environment variables --- ci/appveyor-cpp-build.bat | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ci/appveyor-cpp-build.bat b/ci/appveyor-cpp-build.bat index ebd6e0dbe3d..e6eb0ce4645 100644 --- a/ci/appveyor-cpp-build.bat +++ b/ci/appveyor-cpp-build.bat @@ -28,6 +28,9 @@ set PARQUET_TEST_DATA=%CD%\cpp\submodules\parquet-testing\data set ARROW_DEBUG_MEMORY_POOL=trap +set CMAKE_BUILD_PARALLEL_LEVEL=%$NUMBER_OF_PROCESSORS% +set CTEST_PARALLEL_LEVEL=%$NUMBER_OF_PROCESSORS% + @rem @rem In the configurations below we disable building the Arrow static library @rem to save some time. Unfortunately this will still build the Parquet static @@ -51,7 +54,7 @@ if "%JOB%" == "Build_Debug" ( .. || exit /B cmake --build . --config Debug || exit /B - ctest --output-on-failure -j2 || exit /B + ctest --output-on-failure || exit /B popd @rem Finish Debug build successfully @@ -127,7 +130,7 @@ cmake --build . --target install --config Release || exit /B set OLD_PYTHONHOME=%PYTHONHOME% set PYTHONHOME=%CONDA_PREFIX% -ctest --output-on-failure -j2 || exit /B +ctest --output-on-failure || exit /B set PYTHONHOME=%OLD_PYTHONHOME% popd From b1258caa0ca380fb146cb6a7b3f4bee15fbc4f4d Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Sun, 15 May 2022 07:28:38 +0900 Subject: [PATCH 21/27] fix link problems --- cpp/src/arrow/CMakeLists.txt | 14 -------------- cpp/src/arrow/flight/CMakeLists.txt | 15 +++++++++++++-- .../flight/integration_tests/CMakeLists.txt | 18 ++++++++++++------ cpp/src/arrow/flight/sql/CMakeLists.txt | 9 ++++----- 4 files changed, 29 insertions(+), 27 deletions(-) diff --git a/cpp/src/arrow/CMakeLists.txt b/cpp/src/arrow/CMakeLists.txt index 3b49f4ca00a..77f9959fdbe 100644 --- a/cpp/src/arrow/CMakeLists.txt +++ b/cpp/src/arrow/CMakeLists.txt @@ -754,20 +754,6 @@ endif() if(ARROW_FLIGHT) add_subdirectory(flight) - - if(ARROW_WITH_UCX) - add_subdirectory(flight/transport/ucx) - endif() -endif() - -if(ARROW_FLIGHT_SQL) - add_subdirectory(flight/sql) -endif() - -if(ARROW_FLIGHT - AND ARROW_FLIGHT_SQL - AND ARROW_BUILD_INTEGRATION) - add_subdirectory(flight/integration_tests) endif() if(ARROW_HIVESERVER2) diff --git a/cpp/src/arrow/flight/CMakeLists.txt b/cpp/src/arrow/flight/CMakeLists.txt index f62462c4e8c..538ffe81497 100644 --- a/cpp/src/arrow/flight/CMakeLists.txt +++ b/cpp/src/arrow/flight/CMakeLists.txt @@ -59,7 +59,6 @@ if(ARROW_TESTING) endif() set(ARROW_FLIGHT_TEST_INTERFACE_LIBS) -message("ARROW_FLIGHT_TEST_LINKAGE: ${ARROW_FLIGHT_TEST_LINKAGE}") if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static") if(ARROW_BUILD_STATIC) set(ARROW_FLIGHT_TEST_LINK_LIBS arrow_flight_static) @@ -84,7 +83,6 @@ else() list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS arrow_cuda_shared) endif() endif() -message("ARROW_FLIGHT_TEST_LINK_LIBS: ${ARROW_FLIGHT_TEST_LINK_LIBS}") list(APPEND ARROW_FLIGHT_TEST_INTERFACE_LIBS Boost::headers @@ -367,3 +365,16 @@ if(ARROW_BUILD_BENCHMARKS) endif() endif() endif(ARROW_BUILD_BENCHMARKS) + +if(ARROW_WITH_UCX) + add_subdirectory(transport/ucx) +endif() + +if(ARROW_FLIGHT_SQL) + add_subdirectory(sql) + + if(ARROW_BUILD_INTEGRATION) + add_subdirectory(integration_tests) + endif() +endif() + diff --git a/cpp/src/arrow/flight/integration_tests/CMakeLists.txt b/cpp/src/arrow/flight/integration_tests/CMakeLists.txt index faff3e74c80..d2e77c28805 100644 --- a/cpp/src/arrow/flight/integration_tests/CMakeLists.txt +++ b/cpp/src/arrow/flight/integration_tests/CMakeLists.txt @@ -17,21 +17,27 @@ add_custom_target(arrow_flight_integration_tests) +set(ARROW_FLIGHT_INTEGRATION_TEST_LINK_LIBS) if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static" AND ARROW_BUILD_STATIC) - list(INSERT ARROW_FLIGHT_TEST_LINK_LIBS 0 arrow_flight_sql_static) + list(APPEND ARROW_FLIGHT_INTEGRATION_TEST_LINK_LIBS arrow_flight_sql_static) else() - list(INSERT ARROW_FLIGHT_TEST_LINK_LIBS 0 arrow_flight_sql_shared) + list(APPEND ARROW_FLIGHT_INTEGRATION_TEST_LINK_LIBS arrow_flight_sql_shared) endif() +list(APPEND + ARROW_FLIGHT_INTEGRATION_TEST_LINK_LIBS + ${ARROW_FLIGHT_TEST_LINK_LIBS} + ${GFLAGS_LIBRARIES} + GTest::gtest) add_executable(flight-test-integration-server test_integration_server.cc test_integration.cc) -target_link_libraries(flight-test-integration-server ${ARROW_FLIGHT_TEST_LINK_LIBS} - ${GFLAGS_LIBRARIES} GTest::gtest) +target_link_libraries(flight-test-integration-server + ${ARROW_FLIGHT_INTEGRATION_TEST_LINK_LIBS}) add_executable(flight-test-integration-client test_integration_client.cc test_integration.cc) -target_link_libraries(flight-test-integration-client ${ARROW_FLIGHT_TEST_LINK_LIBS} - ${GFLAGS_LIBRARIES} GTest::gtest) +target_link_libraries(flight-test-integration-client + ${ARROW_FLIGHT_INTEGRATION_TEST_LINK_LIBS}) add_dependencies(arrow-integration flight-test-integration-client flight-test-integration-server) diff --git a/cpp/src/arrow/flight/sql/CMakeLists.txt b/cpp/src/arrow/flight/sql/CMakeLists.txt index 6a3c9222105..1f5a72b50da 100644 --- a/cpp/src/arrow/flight/sql/CMakeLists.txt +++ b/cpp/src/arrow/flight/sql/CMakeLists.txt @@ -63,13 +63,12 @@ add_arrow_lib(arrow_flight_sql PRIVATE_INCLUDES "${Protobuf_INCLUDE_DIRS}") -if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static") - set(ARROW_FLIGHT_SQL_TEST_LINK_LIBS arrow_flight_sql_static - ${ARROW_FLIGHT_TEST_LINK_LIBS}) +if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static" AND ARROW_BUILD_STATIC) + set(ARROW_FLIGHT_SQL_TEST_LINK_LIBS arrow_flight_sql_static) else() - set(ARROW_FLIGHT_SQL_TEST_LINK_LIBS arrow_flight_sql_shared - ${ARROW_FLIGHT_TEST_LINK_LIBS}) + set(ARROW_FLIGHT_SQL_TEST_LINK_LIBS arrow_flight_sql_shared) endif() +list(APPEND ARROW_FLIGHT_SQL_TEST_LINK_LIBS ${ARROW_FLIGHT_TEST_LINK_LIBS}) # Build test server for unit tests if(ARROW_BUILD_TESTS OR ARROW_BUILD_EXAMPLES) From 90a34cf4bbadd423c9497c91e36b7ec927fe8525 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Sun, 15 May 2022 08:33:05 +0900 Subject: [PATCH 22/27] Clean up --- ci/appveyor-cpp-build.bat | 6 +++--- cpp/src/arrow/flight/CMakeLists.txt | 1 - cpp/src/arrow/flight/integration_tests/CMakeLists.txt | 5 ++--- 3 files changed, 5 insertions(+), 7 deletions(-) diff --git a/ci/appveyor-cpp-build.bat b/ci/appveyor-cpp-build.bat index e6eb0ce4645..e9441c63e20 100644 --- a/ci/appveyor-cpp-build.bat +++ b/ci/appveyor-cpp-build.bat @@ -28,8 +28,8 @@ set PARQUET_TEST_DATA=%CD%\cpp\submodules\parquet-testing\data set ARROW_DEBUG_MEMORY_POOL=trap -set CMAKE_BUILD_PARALLEL_LEVEL=%$NUMBER_OF_PROCESSORS% -set CTEST_PARALLEL_LEVEL=%$NUMBER_OF_PROCESSORS% +set CMAKE_BUILD_PARALLEL_LEVEL=%NUMBER_OF_PROCESSORS% +set CTEST_PARALLEL_LEVEL=%NUMBER_OF_PROCESSORS% @rem @rem In the configurations below we disable building the Arrow static library @@ -120,7 +120,7 @@ cmake -G "%GENERATOR%" %CMAKE_ARGS% ^ -DCMAKE_CXX_FLAGS_RELEASE="/MD /Od /UNDEBUG" ^ -DCMAKE_INSTALL_PREFIX=%CONDA_PREFIX%\Library ^ -DCMAKE_UNITY_BUILD=ON ^ - -DCMAKE_VERBOSE_MAKEFILE=ON ^ + -DCMAKE_VERBOSE_MAKEFILE=OFF ^ -DPARQUET_BUILD_EXECUTABLES=ON ^ -DPARQUET_REQUIRE_ENCRYPTION=ON ^ .. || exit /B diff --git a/cpp/src/arrow/flight/CMakeLists.txt b/cpp/src/arrow/flight/CMakeLists.txt index 538ffe81497..187187dd1b7 100644 --- a/cpp/src/arrow/flight/CMakeLists.txt +++ b/cpp/src/arrow/flight/CMakeLists.txt @@ -377,4 +377,3 @@ if(ARROW_FLIGHT_SQL) add_subdirectory(integration_tests) endif() endif() - diff --git a/cpp/src/arrow/flight/integration_tests/CMakeLists.txt b/cpp/src/arrow/flight/integration_tests/CMakeLists.txt index d2e77c28805..66a021b4b59 100644 --- a/cpp/src/arrow/flight/integration_tests/CMakeLists.txt +++ b/cpp/src/arrow/flight/integration_tests/CMakeLists.txt @@ -17,11 +17,10 @@ add_custom_target(arrow_flight_integration_tests) -set(ARROW_FLIGHT_INTEGRATION_TEST_LINK_LIBS) if(ARROW_FLIGHT_TEST_LINKAGE STREQUAL "static" AND ARROW_BUILD_STATIC) - list(APPEND ARROW_FLIGHT_INTEGRATION_TEST_LINK_LIBS arrow_flight_sql_static) + set(ARROW_FLIGHT_INTEGRATION_TEST_LINK_LIBS arrow_flight_sql_static) else() - list(APPEND ARROW_FLIGHT_INTEGRATION_TEST_LINK_LIBS arrow_flight_sql_shared) + set(ARROW_FLIGHT_INTEGRATION_TEST_LINK_LIBS arrow_flight_sql_shared) endif() list(APPEND ARROW_FLIGHT_INTEGRATION_TEST_LINK_LIBS From b0b05ff434fefef353eea5b394fa4da8fe1f9ad3 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Sun, 15 May 2022 08:35:40 +0900 Subject: [PATCH 23/27] Revert fast_finish --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 49ec9afb937..8342dbf6cb3 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -32,7 +32,7 @@ cache: - C:\Users\Appveyor\clcache1 matrix: - fast_finish: false + fast_finish: true environment: global: From 275a0b265902fb9dba5933cd4b0eadff11c4552a Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Sun, 15 May 2022 08:56:58 +0900 Subject: [PATCH 24/27] Improve comment --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 7 +++++++ cpp/src/arrow/flight/CMakeLists.txt | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 76c06dcc649..637bf9d04cb 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -2056,6 +2056,13 @@ if(ARROW_TESTING) else() set(GTEST_USE_CONFIG FALSE) endif() + # We can't find shred library version of GoogleTest on Windows with + # Conda's gtest package because it doesn't provide GTestConfig.cmake + # provided by GoogleTest and CMake's built-in FindGTtest.cmake + # doesn't support gtest_dll.dll. If we find static library version + # of GoogleTest and ARROW_BUILD_SHARED=ON/ARROW_BUILD_STATIC=OFF are + # specified, we need to build arrow_flight_testing as a static + # library. See cpp/src/arrow/flight/CMakeLists.txt for details. resolve_dependency(GTest REQUIRED_VERSION 1.10.0 diff --git a/cpp/src/arrow/flight/CMakeLists.txt b/cpp/src/arrow/flight/CMakeLists.txt index 187187dd1b7..9e46be301b6 100644 --- a/cpp/src/arrow/flight/CMakeLists.txt +++ b/cpp/src/arrow/flight/CMakeLists.txt @@ -39,7 +39,7 @@ if(ARROW_TESTING) # We need to use the same library type for arrow_flight_testing # and Google Test because arrow_flight_testing has # arrow::flight:FlightTest that inherits testing::Test. If - # arrow_flight_testing's library and gtest's library type are + # arrow_flight_testing's library type and gtest's library type are # different such as arrow_flight_testing is a shared library and # gtest is a static library, link error is occurred. get_target_property(GTEST_TYPE GTest::gtest TYPE) From 8843e47244e30c0fa0d8395c0788ae5a05a4dad0 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Mon, 16 May 2022 10:41:35 +0900 Subject: [PATCH 25/27] Add missing gRPC dependency --- cpp/src/arrow/flight/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/cpp/src/arrow/flight/CMakeLists.txt b/cpp/src/arrow/flight/CMakeLists.txt index 9e46be301b6..ab4a47d39aa 100644 --- a/cpp/src/arrow/flight/CMakeLists.txt +++ b/cpp/src/arrow/flight/CMakeLists.txt @@ -90,6 +90,7 @@ list(APPEND Boost::system GTest::gtest GTest::gmock) +list(APPEND ARROW_FLIGHT_TEST_LINK_LIBS gRPC::grpc++) # TODO(wesm): Protobuf shared vs static linking From 7ec278b069ddd6ef3a8812785f4a990c00f21915 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Mon, 16 May 2022 10:52:39 +0900 Subject: [PATCH 26/27] Set timeout --- dev/release/verify-release-candidate.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dev/release/verify-release-candidate.sh b/dev/release/verify-release-candidate.sh index e9bc6b02b2b..a512449aea5 100755 --- a/dev/release/verify-release-candidate.sh +++ b/dev/release/verify-release-candidate.sh @@ -632,9 +632,10 @@ test_and_install_cpp() { LD_LIBRARY_PATH=$PWD/release:$LD_LIBRARY_PATH PYTHONPATH=$pythonpath ctest \ --exclude-regex "plasma-serialization_tests" \ - -j$NPROC \ + --label-regex unittest \ --output-on-failure \ - -L unittest + --parallel $NPROC \ + --timeout 300 popd } From adc9b76451d19702c9bb68b33f6137e511587304 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Tue, 17 May 2022 05:41:10 +0900 Subject: [PATCH 27/27] Remove a needless empty line --- ci/appveyor-cpp-setup.bat | 1 - 1 file changed, 1 deletion(-) diff --git a/ci/appveyor-cpp-setup.bat b/ci/appveyor-cpp-setup.bat index 35e550edb13..af3456c452d 100644 --- a/ci/appveyor-cpp-setup.bat +++ b/ci/appveyor-cpp-setup.bat @@ -78,7 +78,6 @@ if "%JOB%" NEQ "Build_Debug" ( "fsspec" ^ "python=%PYTHON%" ^ || exit /B - ) @rem