From f3a5c73d588bd696bddf7ec1db7d1aa61c834f93 Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Thu, 19 May 2022 14:33:31 +0900 Subject: [PATCH 1/2] ARROW-16614: [C++] Use lz4::lz4 for lz4's CMake target name Because upstream uses lz4::lz4 not LZ4::lz4. This also adds support for upstream lz4Config.cmake. --- ci/conan/all/conanfile.py | 5 +++- ci/scripts/conan_build.sh | 7 ++++- ci/scripts/cpp_build.sh | 2 +- cpp/CMakeLists.txt | 6 ++-- .../{FindLz4.cmake => Findlz4Alt.cmake} | 28 ++++++++++++++----- cpp/cmake_modules/ThirdpartyToolchain.cmake | 26 +++++++++++------ cpp/src/arrow/adapters/orc/CMakeLists.txt | 2 +- .../apache-arrow/debian/libarrow-dev.install | 1 - .../apache-arrow/yum/arrow.spec.in | 2 +- dev/tasks/tasks.yml | 8 ++++++ 10 files changed, 62 insertions(+), 25 deletions(-) rename cpp/cmake_modules/{FindLz4.cmake => Findlz4Alt.cmake} (80%) diff --git a/ci/conan/all/conanfile.py b/ci/conan/all/conanfile.py index 0edbb947815..e232a8cc838 100644 --- a/ci/conan/all/conanfile.py +++ b/ci/conan/all/conanfile.py @@ -382,7 +382,10 @@ def _configure_cmake(self): if self.options.with_bz2: self._cmake.definitions["ARROW_BZ2_USE_SHARED"] = self.options["bzip2"].shared self._cmake.definitions["ARROW_WITH_LZ4"] = self.options.with_lz4 - self._cmake.definitions["Lz4_SOURCE"] = "SYSTEM" + if tools.Version(self.version) >= "9.0.0": + self._cmake.definitions["lz4_SOURCE"] = "SYSTEM" + else: + self._cmake.definitions["Lz4_SOURCE"] = "SYSTEM" if self.options.with_lz4: self._cmake.definitions["ARROW_LZ4_USE_SHARED"] = self.options["lz4"].shared self._cmake.definitions["ARROW_WITH_SNAPPY"] = self.options.with_snappy diff --git a/ci/scripts/conan_build.sh b/ci/scripts/conan_build.sh index 2f38b0ded56..260f8e63bb4 100755 --- a/ci/scripts/conan_build.sh +++ b/ci/scripts/conan_build.sh @@ -27,6 +27,11 @@ shift export ARROW_HOME=${source_dir} export CONAN_HOOK_ERROR_LEVEL=40 +conan_args=() +if [ -n "${ARROW_CONAN_WITH_LZ4:-}" ]; then + conan_args+=(--options arrow:with_lz4=${ARROW_CONAN_WITH_LZ4}) +fi + version=$(grep '^set(ARROW_VERSION ' ${ARROW_HOME}/cpp/CMakeLists.txt | \ grep -E -o '([0-9.]*)') @@ -39,4 +44,4 @@ else sudo chown -R $(id -u):$(id -g) ${build_dir}/conan/ fi cd ${build_dir}/conan/all -conan create . arrow/${version}@ "$@" +conan create . arrow/${version}@ "${conan_args[@]}" "$@" diff --git a/ci/scripts/cpp_build.sh b/ci/scripts/cpp_build.sh index 44acb23295c..d88d95b6774 100755 --- a/ci/scripts/cpp_build.sh +++ b/ci/scripts/cpp_build.sh @@ -149,7 +149,7 @@ cmake \ -Dgoogle_cloud_cpp_storage_SOURCE=${google_cloud_cpp_storage_SOURCE:-} \ -DgRPC_SOURCE=${gRPC_SOURCE:-} \ -DGTest_SOURCE=${GTest_SOURCE:-} \ - -DLz4_SOURCE=${Lz4_SOURCE:-} \ + -Dlz4_SOURCE=${lz4_SOURCE:-} \ -DORC_SOURCE=${ORC_SOURCE:-} \ -DPARQUET_BUILD_EXAMPLES=${PARQUET_BUILD_EXAMPLES:-OFF} \ -DPARQUET_BUILD_EXECUTABLES=${PARQUET_BUILD_EXECUTABLES:-OFF} \ diff --git a/cpp/CMakeLists.txt b/cpp/CMakeLists.txt index ffd906afdf0..5d198ec8f53 100644 --- a/cpp/CMakeLists.txt +++ b/cpp/CMakeLists.txt @@ -745,9 +745,9 @@ if(ARROW_WITH_BZ2) endif() if(ARROW_WITH_LZ4) - list(APPEND ARROW_STATIC_LINK_LIBS LZ4::lz4) - if(Lz4_SOURCE STREQUAL "SYSTEM") - list(APPEND ARROW_STATIC_INSTALL_INTERFACE_LIBS LZ4::lz4) + list(APPEND ARROW_STATIC_LINK_LIBS lz4::lz4) + if(lz4_SOURCE STREQUAL "SYSTEM") + list(APPEND ARROW_STATIC_INSTALL_INTERFACE_LIBS lz4::lz4) endif() endif() diff --git a/cpp/cmake_modules/FindLz4.cmake b/cpp/cmake_modules/Findlz4Alt.cmake similarity index 80% rename from cpp/cmake_modules/FindLz4.cmake rename to cpp/cmake_modules/Findlz4Alt.cmake index bc8051fe9c5..186fec7e40a 100644 --- a/cpp/cmake_modules/FindLz4.cmake +++ b/cpp/cmake_modules/Findlz4Alt.cmake @@ -15,6 +15,19 @@ # specific language governing permissions and limitations # under the License. +set(find_package_args) +if(lz4Alt_FIND_VERSION) + list(APPEND find_package_args ${lz4Alt_FIND_VERSION}) +endif() +if(lz4Alt_FIND_QUIETLY) + list(APPEND find_package_args QUIET) +endif() +find_package(lz4 ${find_package_args}) +if(lz4_FOUND) + set(lz4Alt_FOUND TRUE) + return() +endif() + if(MSVC_TOOLCHAIN AND NOT DEFINED LZ4_MSVC_LIB_PREFIX) set(LZ4_MSVC_LIB_PREFIX "lib") endif() @@ -73,12 +86,13 @@ else() endif() endif() -find_package_handle_standard_args(Lz4 REQUIRED_VARS LZ4_LIB LZ4_INCLUDE_DIR) +find_package_handle_standard_args(lz4Alt REQUIRED_VARS LZ4_LIB LZ4_INCLUDE_DIR) -if(Lz4_FOUND) - set(Lz4_FOUND TRUE) - add_library(LZ4::lz4 UNKNOWN IMPORTED) - set_target_properties(LZ4::lz4 - PROPERTIES IMPORTED_LOCATION "${LZ4_LIB}" - INTERFACE_INCLUDE_DIRECTORIES "${LZ4_INCLUDE_DIR}") +if(lz4Alt_FOUND) + if(NOT TARGET lz4::lz4) + add_library(lz4::lz4 UNKNOWN IMPORTED) + set_target_properties(lz4::lz4 + PROPERTIES IMPORTED_LOCATION "${LZ4_LIB}" + INTERFACE_INCLUDE_DIRECTORIES "${LZ4_INCLUDE_DIR}") + endif() endif() diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index 549211531ab..cf2d470374e 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -62,7 +62,7 @@ set(ARROW_THIRDPARTY_DEPENDENCIES gRPC GTest LLVM - Lz4 + lz4 nlohmann_json opentelemetry-cpp ORC @@ -94,6 +94,14 @@ if("${re2_SOURCE}" STREQUAL "" AND NOT "${RE2_SOURCE}" STREQUAL "") set(re2_SOURCE ${RE2_SOURCE}) endif() +# For backward compatibility. We use "Lz4_SOURCE" if "lz4_SOURCE" +# isn't specified and "lz4_SOURCE" is specified. +# We renamed "Lz4" dependency name to "lz4" in 9.0.0 because +# upstream uses "lz4" not "Lz4" as package name. +if("${lz4_SOURCE}" STREQUAL "" AND NOT "${Lz4_SOURCE}" STREQUAL "") + set(lz4_SOURCE ${Lz4_SOURCE}) +endif() + message(STATUS "Using ${ARROW_DEPENDENCY_SOURCE} approach to find dependencies") if(ARROW_DEPENDENCY_SOURCE STREQUAL "CONDA") @@ -154,7 +162,7 @@ macro(build_dependency DEPENDENCY_NAME) build_grpc() elseif("${DEPENDENCY_NAME}" STREQUAL "GTest") build_gtest() - elseif("${DEPENDENCY_NAME}" STREQUAL "Lz4") + elseif("${DEPENDENCY_NAME}" STREQUAL "lz4") build_lz4() elseif("${DEPENDENCY_NAME}" STREQUAL "nlohmann_json") build_nlohmann_json() @@ -2317,18 +2325,18 @@ macro(build_lz4) BUILD_BYPRODUCTS ${LZ4_STATIC_LIB} ${LZ4_BUILD_COMMAND}) file(MAKE_DIRECTORY "${LZ4_PREFIX}/include") - add_library(LZ4::lz4 STATIC IMPORTED) - set_target_properties(LZ4::lz4 + add_library(lz4::lz4 STATIC IMPORTED) + set_target_properties(lz4::lz4 PROPERTIES IMPORTED_LOCATION "${LZ4_STATIC_LIB}" INTERFACE_INCLUDE_DIRECTORIES "${LZ4_PREFIX}/include") add_dependencies(toolchain lz4_ep) - add_dependencies(LZ4::lz4 lz4_ep) + add_dependencies(lz4::lz4 lz4_ep) - list(APPEND ARROW_BUNDLED_STATIC_LIBS LZ4::lz4) + list(APPEND ARROW_BUNDLED_STATIC_LIBS lz4::lz4) endmacro() if(ARROW_WITH_LZ4) - resolve_dependency(Lz4 PC_PACKAGE_NAMES liblz4) + resolve_dependency(lz4 HAVE_ALT TRUE PC_PACKAGE_NAMES liblz4) endif() macro(build_zstd) @@ -4025,7 +4033,7 @@ macro(build_orc) get_target_property(ORC_SNAPPY_INCLUDE_DIR Snappy::snappy INTERFACE_INCLUDE_DIRECTORIES) get_filename_component(ORC_SNAPPY_ROOT "${ORC_SNAPPY_INCLUDE_DIR}" DIRECTORY) - get_target_property(ORC_LZ4_ROOT LZ4::lz4 INTERFACE_INCLUDE_DIRECTORIES) + get_target_property(ORC_LZ4_ROOT lz4::lz4 INTERFACE_INCLUDE_DIRECTORIES) get_filename_component(ORC_LZ4_ROOT "${ORC_LZ4_ROOT}" DIRECTORY) # Weirdly passing in PROTOBUF_LIBRARY for PROTOC_LIBRARY still results in ORC finding @@ -4069,7 +4077,7 @@ macro(build_orc) set(ORC_VENDORED 1) add_dependencies(orc_ep ZLIB::ZLIB) - add_dependencies(orc_ep LZ4::lz4) + add_dependencies(orc_ep lz4::lz4) add_dependencies(orc_ep Snappy::snappy) add_dependencies(orc_ep ${ARROW_PROTOBUF_LIBPROTOBUF}) diff --git a/cpp/src/arrow/adapters/orc/CMakeLists.txt b/cpp/src/arrow/adapters/orc/CMakeLists.txt index b1b6847cfc5..444a45e4ebe 100644 --- a/cpp/src/arrow/adapters/orc/CMakeLists.txt +++ b/cpp/src/arrow/adapters/orc/CMakeLists.txt @@ -30,7 +30,7 @@ set(ORC_MIN_TEST_LIBS GTest::gtest_main GTest::gtest Snappy::snappy - LZ4::lz4 + lz4::lz4 ZLIB::ZLIB) if(ARROW_BUILD_STATIC) diff --git a/dev/tasks/linux-packages/apache-arrow/debian/libarrow-dev.install b/dev/tasks/linux-packages/apache-arrow/debian/libarrow-dev.install index ccd0c4e5b06..81789b9d569 100644 --- a/dev/tasks/linux-packages/apache-arrow/debian/libarrow-dev.install +++ b/dev/tasks/linux-packages/apache-arrow/debian/libarrow-dev.install @@ -5,7 +5,6 @@ usr/lib/*/cmake/arrow/ArrowTargets*.cmake usr/lib/*/cmake/arrow/Find*Alt.cmake usr/lib/*/cmake/arrow/FindArrow.cmake usr/lib/*/cmake/arrow/FindBrotli.cmake -usr/lib/*/cmake/arrow/FindLz4.cmake usr/lib/*/cmake/arrow/Find[Suz]*.cmake usr/lib/*/cmake/arrow/arrow-config.cmake usr/lib/*/libarrow.a diff --git a/dev/tasks/linux-packages/apache-arrow/yum/arrow.spec.in b/dev/tasks/linux-packages/apache-arrow/yum/arrow.spec.in index ad6cd6de69b..a8c8ccf2c49 100644 --- a/dev/tasks/linux-packages/apache-arrow/yum/arrow.spec.in +++ b/dev/tasks/linux-packages/apache-arrow/yum/arrow.spec.in @@ -275,11 +275,11 @@ Libraries and header files for Apache Arrow C++. %{_libdir}/cmake/arrow/ArrowTargets*.cmake %{_libdir}/cmake/arrow/FindArrow.cmake %{_libdir}/cmake/arrow/FindBrotli.cmake -%{_libdir}/cmake/arrow/FindLz4.cmake %{_libdir}/cmake/arrow/FindSnappy.cmake %if %{use_flight} %{_libdir}/cmake/arrow/Findc-aresAlt.cmake %endif +%{_libdir}/cmake/arrow/Findlz4Alt.cmake %if %{have_re2} %{_libdir}/cmake/arrow/Findre2Alt.cmake %endif diff --git a/dev/tasks/tasks.yml b/dev/tasks/tasks.yml index d9e36ab4a86..93dd5229848 100644 --- a/dev/tasks/tasks.yml +++ b/dev/tasks/tasks.yml @@ -195,6 +195,14 @@ tasks: params: image: conan + conan-maximum: + ci: github + template: docker-tests/github.linux.yml + params: + flags: >- + -e ARROW_CONAN_WITH_LZ4=True + image: conan + ############################## Conda Linux ############################ conda-clean: From 6047f894ceed7ada0e6c599b3b9284c17eae54aa Mon Sep 17 00:00:00 2001 From: Sutou Kouhei Date: Thu, 19 May 2022 15:05:39 +0900 Subject: [PATCH 2/2] Fix style --- cpp/cmake_modules/ThirdpartyToolchain.cmake | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake b/cpp/cmake_modules/ThirdpartyToolchain.cmake index cf2d470374e..aa01b7528ca 100644 --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake @@ -2336,7 +2336,11 @@ macro(build_lz4) endmacro() if(ARROW_WITH_LZ4) - resolve_dependency(lz4 HAVE_ALT TRUE PC_PACKAGE_NAMES liblz4) + resolve_dependency(lz4 + HAVE_ALT + TRUE + PC_PACKAGE_NAMES + liblz4) endif() macro(build_zstd)