kou commented on code in PR #48964: URL: https://github.com/apache/arrow/pull/48964#discussion_r3014153819
########## cpp/cmake_modules/google-cloud-cpp-bcrypt-mingw.patch: ########## @@ -0,0 +1,65 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +diff --git a/google/cloud/internal/win32/sign_using_sha256.cc b/google/cloud/internal/win32/sign_using_sha256.cc +index 15ecc6e6f9..9595ed9eb3 100644 +--- a/google/cloud/internal/win32/sign_using_sha256.cc ++++ b/google/cloud/internal/win32/sign_using_sha256.cc +@@ -99,6 +99,11 @@ StatusOr<std::unique_ptr<std::remove_pointer_t<BCRYPT_KEY_HANDLE>, + decltype(&BCryptDestroyKey)>> + CreateRsaBCryptKey(std::vector<BYTE> buffer) { + BCRYPT_KEY_HANDLE key_handle; ++ // Workaround missing macros in MinGW-w64: ++ // https://github.com/mingw-w64/mingw-w64/issues/49 ++ #ifndef BCRYPT_RSA_ALG_HANDLE ++ #define BCRYPT_RSA_ALG_HANDLE ((BCRYPT_ALG_HANDLE)0x000000e1) ++ #endif + if (BCryptImportKeyPair(BCRYPT_RSA_ALG_HANDLE, nullptr, + BCRYPT_RSAPRIVATE_BLOB, &key_handle, buffer.data(), + static_cast<DWORD>(buffer.size()), +diff --git a/google/cloud/storage/internal/md5hash.cc b/google/cloud/storage/internal/md5hash.cc +index 00c72e0e5d..7fbd04507f 100644 +--- a/google/cloud/storage/internal/md5hash.cc ++++ b/google/cloud/storage/internal/md5hash.cc +@@ -31,6 +31,11 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN + std::vector<std::uint8_t> MD5Hash(absl::string_view payload) { + #ifdef _WIN32 + std::vector<unsigned char> digest(16); ++ // Workaround missing macros in MinGW-w64: ++ // https://github.com/mingw-w64/mingw-w64/issues/49 ++ #ifndef BCRYPT_MD5_ALG_HANDLE ++ #define BCRYPT_MD5_ALG_HANDLE ((BCRYPT_ALG_HANDLE)0x00000021) ++ #endif + BCryptHash(BCRYPT_MD5_ALG_HANDLE, nullptr, 0, + reinterpret_cast<PUCHAR>(const_cast<char*>(payload.data())), + static_cast<ULONG>(payload.size()), digest.data(), +diff --git a/google/cloud/storage/internal/win32/hash_function_impl.cc b/google/cloud/storage/internal/win32/hash_function_impl.cc +index 655c9f0991..07b2937c94 100644 +--- a/google/cloud/storage/internal/win32/hash_function_impl.cc ++++ b/google/cloud/storage/internal/win32/hash_function_impl.cc +@@ -33,6 +33,11 @@ using ContextPtr = + + ContextPtr CreateMD5HashCtx() { + BCRYPT_HASH_HANDLE hHash = nullptr; ++ // Workaround missing macros in MinGW-w64: ++ // https://github.com/mingw-w64/mingw-w64/issues/49 ++ #ifndef BCRYPT_MD5_ALG_HANDLE ++ #define BCRYPT_MD5_ALG_HANDLE ((BCRYPT_ALG_HANDLE)0x00000021) Review Comment: ```suggestion + #define BCRYPT_MD5_ALG_HANDLE (static_cast<BCRYPT_ALG_HANDLE>(0x00000021)) ``` ########## cpp/cmake_modules/ThirdpartyToolchain.cmake: ########## @@ -1890,12 +1882,66 @@ if(ARROW_WITH_THRIFT) list(GET Thrift_VERSION_LIST 2 Thrift_VERSION_PATCH) endif() +# ---------------------------------------------------------------------- +# Abseil defined here so it can be called from build_protobuf() + +function(build_absl) + list(APPEND CMAKE_MESSAGE_INDENT "ABSL: ") + message(STATUS "Building Abseil from source using FetchContent") + set(ABSL_VENDORED + TRUE + PARENT_SCOPE) + + if(CMAKE_COMPILER_IS_GNUCC AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 13.0) + string(APPEND CMAKE_CXX_FLAGS " -include stdint.h") + endif() + + fetchcontent_declare(absl + ${FC_DECLARE_COMMON_OPTIONS} OVERRIDE_FIND_PACKAGE + URL ${ABSL_SOURCE_URL} + URL_HASH "SHA256=${ARROW_ABSL_BUILD_SHA256_CHECKSUM}") + + prepare_fetchcontent() + + # Unity build causes symbol redefinition errors (e.g. kDigits in + # time_zone_fixed.cc and time_zone_posix.cc anonymous namespaces). + set(CMAKE_UNITY_BUILD OFF) + # We have to enable Abseil install to add Abseil targets to an export set. + # But we don't install Abseil by EXCLUDE_FROM_ALL. + set(ABSL_ENABLE_INSTALL ON) Review Comment: Can we use `OFF` here? ```suggestion set(ABSL_ENABLE_INSTALL OFF) ``` ########## cpp/cmake_modules/google-cloud-cpp-bcrypt-mingw.patch: ########## @@ -0,0 +1,65 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +diff --git a/google/cloud/internal/win32/sign_using_sha256.cc b/google/cloud/internal/win32/sign_using_sha256.cc +index 15ecc6e6f9..9595ed9eb3 100644 +--- a/google/cloud/internal/win32/sign_using_sha256.cc ++++ b/google/cloud/internal/win32/sign_using_sha256.cc +@@ -99,6 +99,11 @@ StatusOr<std::unique_ptr<std::remove_pointer_t<BCRYPT_KEY_HANDLE>, + decltype(&BCryptDestroyKey)>> + CreateRsaBCryptKey(std::vector<BYTE> buffer) { + BCRYPT_KEY_HANDLE key_handle; ++ // Workaround missing macros in MinGW-w64: ++ // https://github.com/mingw-w64/mingw-w64/issues/49 ++ #ifndef BCRYPT_RSA_ALG_HANDLE ++ #define BCRYPT_RSA_ALG_HANDLE ((BCRYPT_ALG_HANDLE)0x000000e1) Review Comment: ```suggestion + #define BCRYPT_RSA_ALG_HANDLE (static_cast<BCRYPT_ALG_HANDLE>(0x000000e1)) ``` Or we may be able to add `-DBCRYPT_RSA_ALG_HANDLE=0x000000e1` only for old MinGW in CMake. ########## cpp/cmake_modules/google-cloud-cpp-bcrypt-mingw.patch: ########## @@ -0,0 +1,65 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +diff --git a/google/cloud/internal/win32/sign_using_sha256.cc b/google/cloud/internal/win32/sign_using_sha256.cc +index 15ecc6e6f9..9595ed9eb3 100644 +--- a/google/cloud/internal/win32/sign_using_sha256.cc ++++ b/google/cloud/internal/win32/sign_using_sha256.cc +@@ -99,6 +99,11 @@ StatusOr<std::unique_ptr<std::remove_pointer_t<BCRYPT_KEY_HANDLE>, + decltype(&BCryptDestroyKey)>> + CreateRsaBCryptKey(std::vector<BYTE> buffer) { + BCRYPT_KEY_HANDLE key_handle; ++ // Workaround missing macros in MinGW-w64: ++ // https://github.com/mingw-w64/mingw-w64/issues/49 ++ #ifndef BCRYPT_RSA_ALG_HANDLE ++ #define BCRYPT_RSA_ALG_HANDLE ((BCRYPT_ALG_HANDLE)0x000000e1) ++ #endif + if (BCryptImportKeyPair(BCRYPT_RSA_ALG_HANDLE, nullptr, + BCRYPT_RSAPRIVATE_BLOB, &key_handle, buffer.data(), + static_cast<DWORD>(buffer.size()), +diff --git a/google/cloud/storage/internal/md5hash.cc b/google/cloud/storage/internal/md5hash.cc +index 00c72e0e5d..7fbd04507f 100644 +--- a/google/cloud/storage/internal/md5hash.cc ++++ b/google/cloud/storage/internal/md5hash.cc +@@ -31,6 +31,11 @@ GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN + std::vector<std::uint8_t> MD5Hash(absl::string_view payload) { + #ifdef _WIN32 + std::vector<unsigned char> digest(16); ++ // Workaround missing macros in MinGW-w64: ++ // https://github.com/mingw-w64/mingw-w64/issues/49 ++ #ifndef BCRYPT_MD5_ALG_HANDLE ++ #define BCRYPT_MD5_ALG_HANDLE ((BCRYPT_ALG_HANDLE)0x00000021) Review Comment: ```suggestion + #define BCRYPT_MD5_ALG_HANDLE (static_cast<BCRYPT_ALG_HANDLE>(0x00000021)) ``` ########## cpp/cmake_modules/ThirdpartyToolchain.cmake: ########## @@ -1890,12 +1882,66 @@ if(ARROW_WITH_THRIFT) list(GET Thrift_VERSION_LIST 2 Thrift_VERSION_PATCH) endif() +# ---------------------------------------------------------------------- +# Abseil defined here so it can be called from build_protobuf() + +function(build_absl) + list(APPEND CMAKE_MESSAGE_INDENT "ABSL: ") + message(STATUS "Building Abseil from source using FetchContent") + set(ABSL_VENDORED + TRUE + PARENT_SCOPE) + + if(CMAKE_COMPILER_IS_GNUCC AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 13.0) + string(APPEND CMAKE_CXX_FLAGS " -include stdint.h") + endif() + + fetchcontent_declare(absl + ${FC_DECLARE_COMMON_OPTIONS} OVERRIDE_FIND_PACKAGE + URL ${ABSL_SOURCE_URL} + URL_HASH "SHA256=${ARROW_ABSL_BUILD_SHA256_CHECKSUM}") + + prepare_fetchcontent() + + # Unity build causes symbol redefinition errors (e.g. kDigits in + # time_zone_fixed.cc and time_zone_posix.cc anonymous namespaces). + set(CMAKE_UNITY_BUILD OFF) + # We have to enable Abseil install to add Abseil targets to an export set. + # But we don't install Abseil by EXCLUDE_FROM_ALL. + set(ABSL_ENABLE_INSTALL ON) + fetchcontent_makeavailable(absl) + + if(CMAKE_VERSION VERSION_LESS 3.28) + set_property(DIRECTORY ${absl_SOURCE_DIR} PROPERTY EXCLUDE_FROM_ALL TRUE) + endif() + + if(APPLE) + # This is due to upstream absl::cctz issue + # https://github.com/abseil/abseil-cpp/issues/283 + find_library(CoreFoundation CoreFoundation) + # When ABSL_ENABLE_INSTALL is ON, the real target is "time" not "absl_time" + # Cannot use set_property on alias targets (absl::time is an alias) + set_property(TARGET time + APPEND + PROPERTY INTERFACE_LINK_LIBRARIES ${CoreFoundation}) Review Comment: Can we use `target_link_libraries(time INTERFACE ${CoreFoundation})` here? ```suggestion target_link_libraries(time INTERFACE ${CoreFoundation}) ``` ########## cpp/cmake_modules/ThirdpartyToolchain.cmake: ########## @@ -3380,25 +3370,157 @@ if(ARROW_WITH_NLOHMANN_JSON) message(STATUS "Found nlohmann_json headers: ${nlohmann_json_INCLUDE_DIR}") endif() +function(build_opentelemetry) + list(APPEND CMAKE_MESSAGE_INDENT "OpenTelemetry: ") + message(STATUS "Building OpenTelemetry from source using FetchContent") + + if(Protobuf_VERSION VERSION_GREATER_EQUAL 3.22) + message(FATAL_ERROR "GH-36013: Can't use bundled OpenTelemetry with Protobuf 3.22 or later. " + "Protobuf is version ${Protobuf_VERSION}") + endif() + + set(OPENTELEMETRY_VENDORED + TRUE + PARENT_SCOPE) + + fetchcontent_declare(opentelemetry_proto + ${FC_DECLARE_COMMON_OPTIONS} + URL ${OPENTELEMETRY_PROTO_SOURCE_URL} + URL_HASH "SHA256=${ARROW_OPENTELEMETRY_PROTO_BUILD_SHA256_CHECKSUM}" + ) + + # Use FetchContent_Populate instead of MakeAvailable because opentelemetry-proto + # has no CMakeLists.txt. + cmake_policy(PUSH) + if(POLICY CMP0169) + cmake_policy(SET CMP0169 OLD) + endif() + fetchcontent_populate(opentelemetry_proto) + cmake_policy(POP) Review Comment: It seems that we can use `fetchcontent_makeavailable()` without `CMakeLists.txt`: https://cmake.org/cmake/help/latest/module/FetchContent.html#command:fetchcontent_makeavailable > If the top directory of the populated content contains a CMakeLists.txt file, call [add_subdirectory()](https://cmake.org/cmake/help/latest/command/add_subdirectory.html#command:add_subdirectory) to add it to the main build. It is not an error for there to be no CMakeLists.txt file, which allows the command to be used for dependencies that make downloaded content available at a known location, but which do not need or support being added directly to the build. -- 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]
