From 453ade9b6ebaa2f51829875f5c7221db79e0c578 Mon Sep 17 00:00:00 2001 From: Neal Richardson Date: Mon, 17 Oct 2022 16:12:16 -0400 Subject: [PATCH 1/3] Remove a bunch of checks for old gcc --- ci/conan/all/conanfile.py | 7 +--- cpp/cmake_modules/SetupCxxFlags.cmake | 39 +++++++-------------- cpp/cmake_modules/ThirdpartyToolchain.cmake | 19 ++-------- cpp/src/arrow/dataset/file_parquet.cc | 9 ----- cpp/thirdparty/versions.txt | 3 -- 5 files changed, 16 insertions(+), 61 deletions(-) diff --git a/ci/conan/all/conanfile.py b/ci/conan/all/conanfile.py index 26abcc028b3..5d927bd05fb 100644 --- a/ci/conan/all/conanfile.py +++ b/ci/conan/all/conanfile.py @@ -266,10 +266,7 @@ def _with_boost(self, required=False): if self.options.gandiva: return True version = tools.Version(self.version) - if version.major == "1": - if self._parquet() and self.settings.compiler == "gcc" and self.settings.compiler.version < tools.Version("4.9"): - return True - elif version.major >= "2": + if version.major >= "2": if self.settings.compiler == "Visual Studio": return True return False @@ -614,8 +611,6 @@ def package_info(self): if self.options.gandiva: # FIXME: only filesystem component is used self.cpp_info.components["libgandiva"].requires.append("boost::boost") - if self._parquet() and self.settings.compiler == "gcc" and self.settings.compiler.version < tools.Version("4.9"): - self.cpp_info.components["libparquet"].requires.append("boost::boost") if tools.Version(self.version) >= "2.0": # FIXME: only headers components is used self.cpp_info.components["libarrow"].requires.append("boost::boost") diff --git a/cpp/cmake_modules/SetupCxxFlags.cmake b/cpp/cmake_modules/SetupCxxFlags.cmake index cef4eb0b161..1abc6a5fe46 100644 --- a/cpp/cmake_modules/SetupCxxFlags.cmake +++ b/cpp/cmake_modules/SetupCxxFlags.cmake @@ -400,22 +400,13 @@ elseif(CMAKE_CXX_COMPILER_ID STREQUAL "GNU") set(CXX_ONLY_FLAGS "${CXX_ONLY_FLAGS} -Wno-noexcept-type") endif() - if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER "5.2") - # Disabling semantic interposition allows faster calling conventions - # when calling global functions internally, and can also help inlining. - # See https://stackoverflow.com/questions/35745543/new-option-in-gcc-5-3-fno-semantic-interposition - set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -fno-semantic-interposition") - endif() - - if(CMAKE_CXX_COMPILER_VERSION VERSION_GREATER "4.9") - # Add colors when paired with ninja - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fdiagnostics-color=always") - endif() + # Disabling semantic interposition allows faster calling conventions + # when calling global functions internally, and can also help inlining. + # See https://stackoverflow.com/questions/35745543/new-option-in-gcc-5-3-fno-semantic-interposition + set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -fno-semantic-interposition") - if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS "6.0") - # Work around https://gcc.gnu.org/bugzilla/show_bug.cgi?id=43407 - set(CXX_COMMON_FLAGS "${CXX_COMMON_FLAGS} -Wno-attributes") - endif() + # Add colors when paired with ninja + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fdiagnostics-color=always") if(CMAKE_UNITY_BUILD) # Work around issue similar to https://bugs.webkit.org/show_bug.cgi?id=176869 @@ -507,18 +498,12 @@ if(ARROW_CPU_FLAG STREQUAL "armv8") add_definitions(-DARROW_HAVE_NEON) - if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS - "5.4") - message(WARNING "Disable Armv8 CRC and Crypto as compiler doesn't support them well." - ) - else() - if(ARROW_ARMV8_ARCH_FLAG MATCHES "\\+crypto") - add_definitions(-DARROW_HAVE_ARMV8_CRYPTO) - endif() - # armv8.1+ implies crc support - if(ARROW_ARMV8_ARCH_FLAG MATCHES "armv8\\.[1-9]|\\+crc") - add_definitions(-DARROW_HAVE_ARMV8_CRC) - endif() + if(ARROW_ARMV8_ARCH_FLAG MATCHES "\\+crypto") + add_definitions(-DARROW_HAVE_ARMV8_CRYPTO) + endif() + # armv8.1+ implies crc support + if(ARROW_ARMV8_ARCH_FLAG MATCHES "armv8\\.[1-9]|\\+crc") + add_definitions(-DARROW_HAVE_ARMV8_CRC) endif() elseif(NOT ARROW_SIMD_LEVEL STREQUAL "NONE") message(WARNING "ARROW_SIMD_LEVEL=${ARROW_SIMD_LEVEL} not supported by Arm.") diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index b7cd31f3d74..0ec7d13669e 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -652,18 +652,9 @@ endif() if(DEFINED ENV{ARROW_SNAPPY_URL}) set(SNAPPY_SOURCE_URL "$ENV{ARROW_SNAPPY_URL}") else() - if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS - "4.9") - # There is a bug in GCC < 4.9 with Snappy 1.1.9, so revert to 1.1.8 "SNAPPY_OLD" for those (ARROW-14661) - set_urls(SNAPPY_SOURCE_URL - "https://github.com/google/snappy/archive/${ARROW_SNAPPY_OLD_BUILD_VERSION}.tar.gz" - "${THIRDPARTY_MIRROR_URL}/snappy-${ARROW_SNAPPY_OLD_BUILD_VERSION}.tar.gz") - set(ARROW_SNAPPY_BUILD_SHA256_CHECKSUM ${ARROW_SNAPPY_OLD_BUILD_SHA256_CHECKSUM}) - else() - set_urls(SNAPPY_SOURCE_URL - "https://github.com/google/snappy/archive/${ARROW_SNAPPY_BUILD_VERSION}.tar.gz" - "${THIRDPARTY_MIRROR_URL}/snappy-${ARROW_SNAPPY_BUILD_VERSION}.tar.gz") - endif() + set_urls(SNAPPY_SOURCE_URL + "https://github.com/google/snappy/archive/${ARROW_SNAPPY_BUILD_VERSION}.tar.gz" + "${THIRDPARTY_MIRROR_URL}/snappy-${ARROW_SNAPPY_BUILD_VERSION}.tar.gz") endif() if(DEFINED ENV{ARROW_SUBSTRAIT_URL}) @@ -4584,10 +4575,6 @@ endif() macro(build_awssdk) message(STATUS "Building AWS C++ SDK from source") - if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND CMAKE_CXX_COMPILER_VERSION VERSION_LESS - "4.9") - message(FATAL_ERROR "AWS C++ SDK requires gcc >= 4.9") - endif() set(AWSSDK_PREFIX "${CMAKE_CURRENT_BINARY_DIR}/awssdk_ep-install") set(AWSSDK_INCLUDE_DIR "${AWSSDK_PREFIX}/include") set(AWSSDK_LIB_DIR "lib") diff --git a/cpp/src/arrow/dataset/file_parquet.cc b/cpp/src/arrow/dataset/file_parquet.cc index f07254e1115..da9748de96e 100644 --- a/cpp/src/arrow/dataset/file_parquet.cc +++ b/cpp/src/arrow/dataset/file_parquet.cc @@ -742,16 +742,7 @@ Result> ParquetFileFragment::TryCountRows( compute::Expression predicate) { DCHECK_NE(metadata_, nullptr); if (ExpressionHasFieldRefs(predicate)) { -#if defined(__GNUC__) && (__GNUC__ < 5) - // ARROW-12694: with GCC 4.9 (RTools 35) we sometimes segfault here if we move(result) - auto result = TestRowGroups(std::move(predicate)); - if (!result.ok()) { - return result.status(); - } - auto expressions = result.ValueUnsafe(); -#else ARROW_ASSIGN_OR_RAISE(auto expressions, TestRowGroups(std::move(predicate))); -#endif int64_t rows = 0; for (size_t i = 0; i < row_groups_->size(); i++) { // If the row group is entirely excluded, exclude it from the row count diff --git a/cpp/thirdparty/versions.txt b/cpp/thirdparty/versions.txt index 7409b66c86e..736fb6f76f3 100644 --- a/cpp/thirdparty/versions.txt +++ b/cpp/thirdparty/versions.txt @@ -81,9 +81,6 @@ ARROW_RE2_BUILD_SHA256_CHECKSUM=f89c61410a072e5cbcf8c27e3a778da7d6fd2f2b5b1445cd # 1.1.9 is patched to implement https://github.com/google/snappy/pull/148 if this is bumped, remove the patch ARROW_SNAPPY_BUILD_VERSION=1.1.9 ARROW_SNAPPY_BUILD_SHA256_CHECKSUM=75c1fbb3d618dd3a0483bff0e26d0a92b495bbe5059c8b4f1c962b478b6e06e7 -# There is a bug in GCC < 4.9 with Snappy 1.1.9, so revert to 1.1.8 for those (ARROW-14661) -ARROW_SNAPPY_OLD_BUILD_VERSION=1.1.8 -ARROW_SNAPPY_OLD_BUILD_SHA256_CHECKSUM=16b677f07832a612b0836178db7f374e414f94657c138e6993cbfc5dcc58651f ARROW_SUBSTRAIT_BUILD_VERSION=v0.6.0 ARROW_SUBSTRAIT_BUILD_SHA256_CHECKSUM=7b8583b9684477e9027f417bbfb4febb8acfeb01923dcaa7cf0fd3f921d69c88 ARROW_THRIFT_BUILD_VERSION=0.16.0 From 408a1e771b21f47930e917e779a899c3b4aa86af Mon Sep 17 00:00:00 2001 From: Neal Richardson Date: Tue, 18 Oct 2022 16:08:47 -0400 Subject: [PATCH 2/3] Revert conanfile changes --- ci/conan/all/conanfile.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/ci/conan/all/conanfile.py b/ci/conan/all/conanfile.py index 5d927bd05fb..26abcc028b3 100644 --- a/ci/conan/all/conanfile.py +++ b/ci/conan/all/conanfile.py @@ -266,7 +266,10 @@ def _with_boost(self, required=False): if self.options.gandiva: return True version = tools.Version(self.version) - if version.major >= "2": + if version.major == "1": + if self._parquet() and self.settings.compiler == "gcc" and self.settings.compiler.version < tools.Version("4.9"): + return True + elif version.major >= "2": if self.settings.compiler == "Visual Studio": return True return False @@ -611,6 +614,8 @@ def package_info(self): if self.options.gandiva: # FIXME: only filesystem component is used self.cpp_info.components["libgandiva"].requires.append("boost::boost") + if self._parquet() and self.settings.compiler == "gcc" and self.settings.compiler.version < tools.Version("4.9"): + self.cpp_info.components["libparquet"].requires.append("boost::boost") if tools.Version(self.version) >= "2.0": # FIXME: only headers components is used self.cpp_info.components["libarrow"].requires.append("boost::boost") From cfc33dd7d2d24328a350cf654a34b4d6394a8e72 Mon Sep 17 00:00:00 2001 From: Neal Richardson Date: Tue, 18 Oct 2022 16:10:31 -0400 Subject: [PATCH 3/3] Fix cmake formatting --- 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 0ec7d13669e..adc620368eb 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -653,8 +653,8 @@ if(DEFINED ENV{ARROW_SNAPPY_URL}) set(SNAPPY_SOURCE_URL "$ENV{ARROW_SNAPPY_URL}") else() set_urls(SNAPPY_SOURCE_URL - "https://github.com/google/snappy/archive/${ARROW_SNAPPY_BUILD_VERSION}.tar.gz" - "${THIRDPARTY_MIRROR_URL}/snappy-${ARROW_SNAPPY_BUILD_VERSION}.tar.gz") + "https://github.com/google/snappy/archive/${ARROW_SNAPPY_BUILD_VERSION}.tar.gz" + "${THIRDPARTY_MIRROR_URL}/snappy-${ARROW_SNAPPY_BUILD_VERSION}.tar.gz") endif() if(DEFINED ENV{ARROW_SUBSTRAIT_URL})