kou commented on code in PR #44989:
URL: https://github.com/apache/arrow/pull/44989#discussion_r1935068380


##########
dev/tasks/linux-packages/apache-arrow/yum/centos-7/Dockerfile:
##########
@@ -75,6 +75,12 @@ RUN \
     zlib-devel && \
   yum clean ${quiet} all
 
+ARG cmake=3.25.0
+RUN curl -L \
+    
"https://github.com/Kitware/CMake/releases/download/v${cmake}/cmake-${cmake}-linux-$(uname
 -m).tar.gz" | \
+    tar -xzf - --directory /usr/local --strip-components=1 && \
+    ln -fs /usr/local/bin/cmake /usr/bin/cmake3

Review Comment:
   This is needed because RPM macros uses `/usr/bin/cmake3` not `cmake3`.



##########
ci/rtools/awssdk_ep.patch:
##########
@@ -0,0 +1,213 @@
+# 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/aws-cpp-sdk-core/include/aws/core/utils/Array.h 
b/aws-cpp-sdk-core/include/aws/core/utils/Array.h
+index 2b5bbc566..7cb93bdf0 100644
+--- a/aws-cpp-sdk-core/include/aws/core/utils/Array.h
++++ b/aws-cpp-sdk-core/include/aws/core/utils/Array.h
+@@ -54,7 +54,7 @@ namespace Aws
+                 {
+                     m_data.reset(Aws::NewArray<T>(m_size, 
ARRAY_ALLOCATION_TAG));
+
+-#ifdef _WIN32
++#ifdef _MSC_VER
+                     std::copy(arrayToCopy, arrayToCopy + arraySize, 
stdext::checked_array_iterator< T * >(m_data.get(), m_size));
+ #else
+                     std::copy(arrayToCopy, arrayToCopy + arraySize, 
m_data.get());
+@@ -82,7 +82,7 @@ namespace Aws
+                     if(arr->m_size > 0 && arr->m_data)
+                     {
+                         size_t arraySize = arr->m_size;
+-#ifdef _WIN32
++#ifdef _MSC_VER
+                         std::copy(arr->m_data.get(), arr->m_data.get() + 
arraySize, stdext::checked_array_iterator< T * >(m_data.get() + location, 
m_size));
+ #else
+                         std::copy(arr->m_data.get(), arr->m_data.get() + 
arraySize, m_data.get() + location);
+@@ -101,7 +101,7 @@ namespace Aws
+                 {
+                     m_data.reset(Aws::NewArray<T>(m_size, 
ARRAY_ALLOCATION_TAG));
+
+-#ifdef _WIN32
++#ifdef _MSC_VER
+                     std::copy(other.m_data.get(), other.m_data.get() + 
other.m_size, stdext::checked_array_iterator< T * >(m_data.get(), m_size));
+ #else
+                     std::copy(other.m_data.get(), other.m_data.get() + 
other.m_size, m_data.get());
+@@ -134,7 +134,7 @@ namespace Aws
+                 {
+                     m_data.reset(Aws::NewArray<T>(m_size, 
ARRAY_ALLOCATION_TAG));
+
+-#ifdef _WIN32
++#ifdef _MSC_VER
+                     std::copy(other.m_data.get(), other.m_data.get() + 
other.m_size, stdext::checked_array_iterator< T * >(m_data.get(), m_size));
+ #else
+                     std::copy(other.m_data.get(), other.m_data.get() + 
other.m_size, m_data.get());
+diff --git 
a/aws-cpp-sdk-core/include/aws/core/utils/crypto/bcrypt/CryptoImpl.h 
b/aws-cpp-sdk-core/include/aws/core/utils/crypto/bcrypt/CryptoImpl.h
+index e26e36b60..3e7189b70 100644
+--- a/aws-cpp-sdk-core/include/aws/core/utils/crypto/bcrypt/CryptoImpl.h
++++ b/aws-cpp-sdk-core/include/aws/core/utils/crypto/bcrypt/CryptoImpl.h
+@@ -29,7 +29,14 @@ namespace Aws
+     {
+         namespace Crypto
+         {
+-            static const char* SecureRandom_BCrypt_Tag = 
"SecureRandom_BCrypt";
++#ifdef __MINGW32__

Review Comment:
   Or can we pass `-Wno-unused-variable` (`-Wno-error`) in 
`ThirdpartyToolchain.cmake`?



##########
ci/rtools/aws_c_common_ep.patch:
##########
@@ -0,0 +1,54 @@
+# 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/cmake/AwsCFlags.cmake b/cmake/AwsCFlags.cmake
+index b717bca..5aa8ac9 100644
+--- a/cmake/AwsCFlags.cmake
++++ b/cmake/AwsCFlags.cmake
+@@ -120,6 +120,10 @@ function(aws_set_common_properties target)
+             list(APPEND AWS_C_FLAGS -Wno-strict-aliasing)
+         endif()
+
++         if (CMAKE_C_IMPLICIT_LINK_LIBRARIES MATCHES "mingw32")
++            list(APPEND AWS_C_FLAGS -D__USE_MINGW_ANSI_STDIO=1 
-Wno-unused-local-typedefs)

Review Comment:
   Can we do this in `ThirdpartyToolchain.cmake` like other flags?
   
   If we aren't harry, we can wait for 
https://github.com/apache/arrow/pull/45306 .
   We don't need those patches for AWS SDK for C++ related projects.



##########
ci/docker/ubuntu-22.04-verify-rc.dockerfile:
##########
@@ -24,3 +24,7 @@ RUN /setup-ubuntu.sh && \
     rm /setup-ubuntu.sh && \
     apt-get clean && \
     rm -rf /var/lib/apt/lists*
+
+ARG cmake=3.25.0

Review Comment:
   ```suggestion
   ARG cmake
   ```



##########
cpp/CMakeLists.txt:
##########
@@ -15,9 +15,14 @@
 # specific language governing permissions and limitations
 # under the License.
 
-cmake_minimum_required(VERSION 3.16)
+cmake_minimum_required(VERSION 3.25)
 message(STATUS "Building using CMake version: ${CMAKE_VERSION}")
 
+# https://cmake.org/cmake/help/latest/policy/CMP0126.html
+#
+# set(CACHE) does not remove a normal variable of the same name.

Review Comment:
   Can we remove a normal `ZLIB_LIBRARY` by `unset` 
https://cmake.org/cmake/help/latest/command/unset.html explicitly instead?
   
   ```diff
   diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake 
b/cpp/cmake_modules/ThirdpartyToolchain.cmake
   index 4ef9fa8f4c..8001f07d8e 100644
   --- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
   +++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
   @@ -4664,6 +4664,7 @@ function(build_orc)
        set(ZLIB_HOME
            ${ZLIB_ROOT}
            CACHE STRING "" FORCE)
   +    set(unset ZLIB_LIBRARY)
        set(ZLIB_LIBRARY
            ZLIB::ZLIB
            CACHE STRING "" FORCE)
   ```



##########
dev/tasks/linux-packages/apache-arrow/yum/amazon-linux-2023/Dockerfile:
##########
@@ -60,3 +60,9 @@ RUN \
     which \
     zlib-devel && \
   dnf clean ${quiet} all
+
+ARG cmake=3.25.0
+RUN curl -L \
+    
"https://github.com/Kitware/CMake/releases/download/v${cmake}/cmake-${cmake}-linux-$(uname
 -m).tar.gz" | \
+    tar -xzf - --directory /usr/local --strip-components=1 && \
+    ln -fs /usr/local/bin/cmake /usr/bin/cmake

Review Comment:
   This is needed because RPM macros uses `/usr/bin/cmake` not `cmake`.



##########
ci/docker/ubuntu-20.04-verify-rc.dockerfile:
##########
@@ -24,3 +24,7 @@ RUN /setup-ubuntu.sh && \
     rm /setup-ubuntu.sh && \
     apt-get clean && \
     rm -rf /var/lib/apt/lists*
+
+ARG cmake=3.25.0

Review Comment:
   We can remove it:
   
   ```suggestion
   ARG cmake
   ```



##########
dev/tasks/linux-packages/apache-arrow/apt/ubuntu-focal/Dockerfile:
##########
@@ -79,3 +80,9 @@ RUN \
   ln -fs /usr/local/bin/gi-docgen /usr/bin && \
   apt clean && \
   rm -rf /var/lib/apt/lists/*
+
+ARG cmake=3.25.0
+RUN curl -L \

Review Comment:
   Let's try this.



##########
dev/tasks/linux-packages/apache-arrow/apt/ubuntu-focal/Dockerfile:
##########
@@ -79,3 +80,9 @@ RUN \
   ln -fs /usr/local/bin/gi-docgen /usr/bin && \
   apt clean && \
   rm -rf /var/lib/apt/lists/*
+
+ARG cmake=3.25.0
+RUN curl -L \
+    
"https://github.com/Kitware/CMake/releases/download/v${cmake}/cmake-${cmake}-linux-$(uname
 -m).tar.gz" | \
+    tar -xzf - --directory /usr/local --strip-components=1 && \
+    ln -fs /usr/local/bin/cmake /usr/bin/cmake

Review Comment:
   Let's try this.



-- 
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]

Reply via email to