apache · arw2019 · Nov 26, 2020 · Dec 1, 2020 · Dec 1, 2020 · Dec 1, 2020
diff --git a/ci/conda_env_cpp.yml b/ci/conda_env_cpp.yml
@@ -24,9 +24,9 @@ c-ares
 cmake
 gflags
 glog
-gmock>=1.8.1
+gmock>=1.10.0
 grpc-cpp>=1.27.3
-gtest=1.8.1
+gtest=1.10.0
 libprotobuf
 libutf8proc
 lz4-c

diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake b/cpp/cmake_modules/SetupCxxFlags.cmake
@@ -159,6 +159,23 @@ if(WIN32)
       set(CXX_COMMON_FLAGS "/W3 /EHsc")
     endif()
 
+    # Disable C5105 (macro expansion producing 'defined' has undefined
+    # behavior) warning because there are codes that produce this
+    # warning in Windows Kits. e.g.:
+    #
+    #   #define _CRT_INTERNAL_NONSTDC_NAMES                                            \
+    #        (                                                                          \
+    #            ( defined _CRT_DECLARE_NONSTDC_NAMES && _CRT_DECLARE_NONSTDC_NAMES) || \
+    #            (!defined _CRT_DECLARE_NONSTDC_NAMES && !__STDC__                 )    \
+    #        )
+    #
+    # See also:
+    # * C5105: https://docs.microsoft.com/en-US/cpp/error-messages/compiler-warnings/c5105
+    # * Related reports:
+    #   * https://developercommunity.visualstudio.com/content/problem/387684/c5105-with-stdioh-and-experimentalpreprocessor.html
+    #   * https://developercommunity.visualstudio.com/content/problem/1249671/stdc17-generates-warning-compiling-windowsh.html
+    set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} /wd5105")
+
     if(ARROW_USE_STATIC_CRT)
       foreach(c_flag
               CMAKE_CXX_FLAGS
@@ -177,6 +194,10 @@ if(WIN32)
 
     # Support large object code
     set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} /bigobj")
+
+    # We may use UTF-8 in source code such as
+    # cpp/src/arrow/compute/kernels/scalar_string_test.cc
+    set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} /utf-8")
   else()
     # MinGW
     check_cxx_compiler_flag(-Wa,-mbig-obj CXX_SUPPORTS_BIG_OBJ)

diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake
@@ -1603,21 +1603,17 @@ macro(build_gtest)
   set(GTEST_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/googletest_ep-prefix")
   set(GTEST_INCLUDE_DIR "${GTEST_PREFIX}/include")
 
-  set(_GTEST_RUNTIME_DIR ${BUILD_OUTPUT_ROOT_DIRECTORY})
+  set(_GTEST_LIBRARY_DIR "${GTEST_PREFIX}/lib")
 
   if(MSVC)
     set(_GTEST_IMPORTED_TYPE IMPORTED_IMPLIB)
     set(_GTEST_LIBRARY_SUFFIX
         "${CMAKE_GTEST_DEBUG_EXTENSION}${CMAKE_IMPORT_LIBRARY_SUFFIX}")
-    # Use the import libraries from the EP
-    set(_GTEST_LIBRARY_DIR "${GTEST_PREFIX}/lib")
   else()
     set(_GTEST_IMPORTED_TYPE IMPORTED_LOCATION)
     set(_GTEST_LIBRARY_SUFFIX
         "${CMAKE_GTEST_DEBUG_EXTENSION}${CMAKE_SHARED_LIBRARY_SUFFIX}")
 
-    # Library and runtime same on non-Windows
-    set(_GTEST_LIBRARY_DIR "${_GTEST_RUNTIME_DIR}")
   endif()
 
   set(GTEST_SHARED_LIB
@@ -1630,38 +1626,16 @@ macro(build_gtest)
     )
   set(GTEST_CMAKE_ARGS
       ${EP_COMMON_TOOLCHAIN}
-      -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
-      "-DCMAKE_INSTALL_PREFIX=${GTEST_PREFIX}"
       -DBUILD_SHARED_LIBS=ON
+      -DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE}
       -DCMAKE_CXX_FLAGS=${GTEST_CMAKE_CXX_FLAGS}
-      -DCMAKE_CXX_FLAGS_${UPPERCASE_BUILD_TYPE}=${GTEST_CMAKE_CXX_FLAGS})
+      -DCMAKE_CXX_FLAGS_${UPPERCASE_BUILD_TYPE}=${GTEST_CMAKE_CXX_FLAGS}
+      -DCMAKE_INSTALL_LIBDIR=lib
+      -DCMAKE_INSTALL_NAME_DIR=$<INSTALL_PREFIX$<ANGLE-R>/lib
+      -DCMAKE_INSTALL_PREFIX=${GTEST_PREFIX}
+      -DCMAKE_MACOSX_RPATH=OFF)
   set(GMOCK_INCLUDE_DIR "${GTEST_PREFIX}/include")
 
-  if(APPLE)
-    set(GTEST_CMAKE_ARGS ${GTEST_CMAKE_ARGS} "-DCMAKE_MACOSX_RPATH:BOOL=ON")
-  endif()
-
-  if(CMAKE_GENERATOR STREQUAL "Xcode")
-    # Xcode projects support multi-configuration builds.  This forces the gtest build
-    # to use the same output directory as a single-configuration Makefile driven build.
-    list(
-      APPEND GTEST_CMAKE_ARGS "-DCMAKE_LIBRARY_OUTPUT_DIRECTORY=${_GTEST_LIBRARY_DIR}"
-             "-DCMAKE_LIBRARY_OUTPUT_DIRECTORY_${CMAKE_BUILD_TYPE}=${_GTEST_RUNTIME_DIR}")
-  endif()
-
-  if(MSVC)
-    if(NOT ("${CMAKE_GENERATOR}" STREQUAL "Ninja"))
-      set(_GTEST_RUNTIME_DIR ${_GTEST_RUNTIME_DIR}/${CMAKE_BUILD_TYPE})
-    endif()
-    set(GTEST_CMAKE_ARGS
-        ${GTEST_CMAKE_ARGS} "-DCMAKE_RUNTIME_OUTPUT_DIRECTORY=${_GTEST_RUNTIME_DIR}"
-        "-DCMAKE_RUNTIME_OUTPUT_DIRECTORY_${CMAKE_BUILD_TYPE}=${_GTEST_RUNTIME_DIR}")
-  else()
-    list(
-      APPEND GTEST_CMAKE_ARGS "-DCMAKE_LIBRARY_OUTPUT_DIRECTORY=${_GTEST_RUNTIME_DIR}"
-             "-DCMAKE_RUNTIME_OUTPUT_DIRECTORY_${CMAKE_BUILD_TYPE}=${_GTEST_RUNTIME_DIR}")
-  endif()
-
   add_definitions(-DGTEST_LINKED_AS_SHARED_LIBRARY=1)
 
   if(MSVC AND NOT ARROW_USE_STATIC_CRT)
@@ -1673,6 +1647,57 @@ macro(build_gtest)
                       BUILD_BYPRODUCTS ${GTEST_SHARED_LIB} ${GTEST_MAIN_SHARED_LIB}
                                        ${GMOCK_SHARED_LIB}
                       CMAKE_ARGS ${GTEST_CMAKE_ARGS} ${EP_LOG_OPTIONS})
+  if(WIN32)
+    # Copy the built shared libraries to the same directory as our
+    # test programs because Windows doesn't provided rpath (run-time
+    # search path) feature. We need to put these shared libraries to
+    # the same directory as our test programs or add
+    # _GTEST_LIBRARY_DIR to PATH when we run our test programs. We
+    # choose the former because the latter may be forgotten.
+    set(_GTEST_RUNTIME_DIR "${GTEST_PREFIX}/bin")
+    set(_GTEST_RUNTIME_SUFFIX
+        "${CMAKE_GTEST_DEBUG_EXTENSION}${CMAKE_SHARED_LIBRARY_SUFFIX}")
+    set(
+      _GTEST_RUNTIME_LIB
+      "${_GTEST_RUNTIME_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest${_GTEST_RUNTIME_SUFFIX}")
+    set(
+      _GMOCK_RUNTIME_LIB
+      "${_GTEST_RUNTIME_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gmock${_GTEST_RUNTIME_SUFFIX}")
+    set(
+      _GTEST_MAIN_RUNTIME_LIB
+      "${_GTEST_RUNTIME_DIR}/${CMAKE_SHARED_LIBRARY_PREFIX}gtest_main${_GTEST_RUNTIME_SUFFIX}"
+      )
+    if(CMAKE_VERSION VERSION_LESS 3.9)
+      message(
+        FATAL_ERROR
+          "Building GoogleTest from source on Windows requires at least CMake 3.9")
+    endif()
+    get_property(_GENERATOR_IS_MULTI_CONFIG GLOBAL PROPERTY GENERATOR_IS_MULTI_CONFIG)
+    if(_GENERATOR_IS_MULTI_CONFIG)
+      set(_GTEST_RUNTIME_OUTPUT_DIR "${BUILD_OUTPUT_ROOT_DIRECTORY}/${CMAKE_BUILD_TYPE}")
+    else()
+      set(_GTEST_RUNTIME_OUTPUT_DIR ${BUILD_OUTPUT_ROOT_DIRECTORY})
+    endif()
+    externalproject_add_step(googletest_ep copy
+                             COMMAND ${CMAKE_COMMAND} -E make_directory
+                                     ${_GTEST_RUNTIME_OUTPUT_DIR}
+                             COMMAND ${CMAKE_COMMAND}
+                                     -E
+                                     copy
+                                     ${_GTEST_RUNTIME_LIB}
+                                     ${_GTEST_RUNTIME_OUTPUT_DIR}
+                             COMMAND ${CMAKE_COMMAND}
+                                     -E
+                                     copy
+                                     ${_GMOCK_RUNTIME_LIB}
+                                     ${_GTEST_RUNTIME_OUTPUT_DIR}
+                             COMMAND ${CMAKE_COMMAND}
+                                     -E
+                                     copy
+                                     ${_GTEST_MAIN_RUNTIME_LIB}
+                                     ${_GTEST_RUNTIME_OUTPUT_DIR}
+                             DEPENDEES install)
+  endif()
 
   # The include directory must exist before it is referenced by a target.
   file(MAKE_DIRECTORY "${GTEST_INCLUDE_DIR}")
@@ -1698,7 +1723,7 @@ macro(build_gtest)
 endmacro()
 
 if(ARROW_TESTING)
-  resolve_dependency(GTest)
+  resolve_dependency(GTest REQUIRED_VERSION 1.10.0)
 
   if(NOT GTEST_VENDORED)
     # TODO(wesm): This logic does not work correctly with the MSVC static libraries

diff --git a/cpp/src/arrow/io/hdfs_test.cc b/cpp/src/arrow/io/hdfs_test.cc
@@ -136,10 +136,9 @@ class TestHadoopFileSystem : public ::testing::Test {
   std::shared_ptr<HadoopFileSystem> client_;
 };
 
-#define SKIP_IF_NO_DRIVER()                                  \
-  if (!this->loaded_driver_) {                               \
-    std::cout << "Driver not loaded, skipping" << std::endl; \
-    return;                                                  \
+#define SKIP_IF_NO_DRIVER()                        \
+  if (!this->loaded_driver_) {                     \
+    GTEST_SKIP() << "Driver not loaded, skipping"; \
   }
 
 TEST_F(TestHadoopFileSystem, ConnectsAgain) {

diff --git a/cpp/src/arrow/util/compression_test.cc b/cpp/src/arrow/util/compression_test.cc
@@ -349,8 +349,7 @@ TEST(TestCodecMisc, GetCompressionType) {
 TEST_P(CodecTest, CodecRoundtrip) {
   const auto compression = GetCompression();
   if (compression == Compression::BZ2) {
-    // SKIP: BZ2 doesn't support one-shot compression
-    return;
+    GTEST_SKIP() << "BZ2 does not support one-shot compression";
   }
 
   int sizes[] = {0, 10000, 100000};
@@ -429,17 +428,14 @@ TEST_P(CodecTest, OutputBufferIsSmall) {
 
 TEST_P(CodecTest, StreamingCompressor) {
   if (GetCompression() == Compression::SNAPPY) {
-    // SKIP: snappy doesn't support streaming compression
-    return;
+    GTEST_SKIP() << "snappy doesn't support streaming compression";
   }
   if (GetCompression() == Compression::BZ2) {
-    // SKIP: BZ2 doesn't support one-shot decompression
-    return;
+    GTEST_SKIP() << "Z2 doesn't support one-shot decompression";
   }
   if (GetCompression() == Compression::LZ4 ||
       GetCompression() == Compression::LZ4_HADOOP) {
-    // SKIP: LZ4 raw format doesn't support streaming compression.
-    return;
+    GTEST_SKIP() << "LZ4 raw format doesn't support streaming compression.";
   }
 
   int sizes[] = {0, 10, 100000};
@@ -456,17 +452,14 @@ TEST_P(CodecTest, StreamingCompressor) {
 
 TEST_P(CodecTest, StreamingDecompressor) {
   if (GetCompression() == Compression::SNAPPY) {
-    // SKIP: snappy doesn't support streaming decompression
-    return;
+    GTEST_SKIP() << "snappy doesn't support streaming decompression.";
   }
   if (GetCompression() == Compression::BZ2) {
-    // SKIP: BZ2 doesn't support one-shot compression
-    return;
+    GTEST_SKIP() << "Z2 doesn't support one-shot compression";
   }
   if (GetCompression() == Compression::LZ4 ||
       GetCompression() == Compression::LZ4_HADOOP) {
-    // SKIP: LZ4 raw format doesn't support streaming decompression.
-    return;
+    GTEST_SKIP() << "LZ4 raw format doesn't support streaming decompression.";
   }
 
   int sizes[] = {0, 10, 100000};
@@ -483,13 +476,11 @@ TEST_P(CodecTest, StreamingDecompressor) {
 
 TEST_P(CodecTest, StreamingRoundtrip) {
   if (GetCompression() == Compression::SNAPPY) {
-    // SKIP: snappy doesn't support streaming decompression
-    return;
+    GTEST_SKIP() << "snappy doesn't support streaming decompression";
   }
   if (GetCompression() == Compression::LZ4 ||
       GetCompression() == Compression::LZ4_HADOOP) {
-    // SKIP: LZ4 raw format doesn't support streaming compression.
-    return;
+    GTEST_SKIP() << "LZ4 raw format doesn't support streaming compression.";
   }
 
   int sizes[] = {0, 10, 100000};
@@ -506,13 +497,11 @@ TEST_P(CodecTest, StreamingRoundtrip) {
 
 TEST_P(CodecTest, StreamingDecompressorReuse) {
   if (GetCompression() == Compression::SNAPPY) {
-    // SKIP: snappy doesn't support streaming decompression
-    return;
+    GTEST_SKIP() << "snappy doesn't support streaming decompression";
   }
   if (GetCompression() == Compression::LZ4 ||
       GetCompression() == Compression::LZ4_HADOOP) {
-    // SKIP: LZ4 raw format doesn't support streaming decompression.
-    return;
+    GTEST_SKIP() << "LZ4 raw format doesn't support streaming decompression.";
   }
 
   auto codec = MakeCodec();

diff --git a/cpp/src/parquet/column_scanner_test.cc b/cpp/src/parquet/column_scanner_test.cc
@@ -48,7 +48,7 @@ void InitDictValues<bool>(int num_values, int dict_per_page, std::vector<bool>&
 template <typename Type>
 class TestFlatScanner : public ::testing::Test {
  public:
-  typedef typename Type::c_type T;
+  using c_type = typename Type::c_type;
 
   void InitScanner(const ColumnDescriptor* d) {
     std::unique_ptr<PageReader> pager(new test::MockPageReader(pages_));
@@ -57,7 +57,7 @@ class TestFlatScanner : public ::testing::Test {
 
   void CheckResults(int batch_size, const ColumnDescriptor* d) {
     TypedScanner<Type>* scanner = reinterpret_cast<TypedScanner<Type>*>(scanner_.get());
-    T val;
+    c_type val;
     bool is_null = false;
     int16_t def_level;
     int16_t rep_level;
@@ -131,7 +131,7 @@ class TestFlatScanner : public ::testing::Test {
   int num_values_;
   std::vector<std::shared_ptr<Page>> pages_;
   std::shared_ptr<Scanner> scanner_;
-  std::vector<T> values_;
+  std::vector<c_type> values_;
   std::vector<int16_t> def_levels_;
   std::vector<int16_t> rep_levels_;
   std::vector<uint8_t> data_buffer_;  // For BA and FLBA

diff --git a/cpp/src/parquet/column_writer_test.cc b/cpp/src/parquet/column_writer_test.cc
@@ -64,8 +64,6 @@ const int64_t DICTIONARY_PAGE_SIZE = 1024 * 1024;
 template <typename TestType>
 class TestPrimitiveWriter : public PrimitiveTypedTest<TestType> {
  public:
-  typedef typename TestType::c_type T;
-
   void SetUp() {
     this->SetupValuesOut(SMALL_SIZE);
     writer_properties_ = default_writer_properties();