Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-44950: [C++] Bump minimum CMake version to 3.25 #44989

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
+512 −66
Draft

GH-44950: [C++] Bump minimum CMake version to 3.25 #44989

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cc4cea4
GH-44950: [C++] Bump minimum CMake version to 3.25
raulcd Dec 10, 2024
4b88388
Fix some review comments
raulcd Jan 21, 2025
908b080
Update some workflows to use Ubuntu 24.04 in order to remove manual C…
raulcd Jan 22, 2025
48a6d0a
Try to use newer CMake from base Windows-2019 image on Windows R buil…
raulcd Jan 22, 2025
0d37aca
Locate CMake
raulcd Jan 22, 2025
fb44d93
Try installing cmake on Linux packages
raulcd Jan 22, 2025
089a981
Locate CMake v2
raulcd Jan 22, 2025
9ae7ca1
Use full path to CMake on Program Files
raulcd Jan 22, 2025
9cdbc70
Try centos7 path (this is just testing at this point)
raulcd Jan 22, 2025
79de380
Try using link for context on Dockerfile
raulcd Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .env
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ UBUNTU=22.04

# Default versions for various dependencies
CLANG_TOOLS=14
CMAKE=3.25.0
raulcd marked this conversation as resolved.
Show resolved Hide resolved
CUDA=11.2.2
DASK=latest
DOTNET=8.0
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/cpp.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ jobs:

build-example:
name: C++ Minimal Build Example
runs-on: ubuntu-latest
runs-on: ubuntu-24.04
if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
timeout-minutes: 45
steps:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/dev.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ jobs:

lint:
name: Lint C++, Python, R, Docker, RAT
runs-on: ubuntu-latest
runs-on: ubuntu-24.04
if: ${{ !contains(github.event.pull_request.title, 'WIP') }}
timeout-minutes: 15
steps:
Expand Down
5 changes: 5 additions & 0 deletions .github/workflows/r.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,11 @@ jobs:
r-version: "4.1"
rtools-version: 40
Ncpus: 2
- name: Locate CMAKE
shell: bash
run: |
cmake --version
which cmake
- name: Build Arrow C++
shell: bash
env:
Expand Down
7 changes: 5 additions & 2 deletions ci/docker/centos-7-cpp.dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ RUN \
-e 's/mirror\.centos\.org/vault.centos.org/' \
/etc/yum.repos.d/CentOS-SCLo-scl*.repo && \
yum install -y \
cmake3 \
curl \
devtoolset-8 \
diffutils \
Expand All @@ -49,9 +48,13 @@ RUN \
wget \
which

ARG cmake
COPY ci/scripts/install_cmake.sh /arrow/ci/scripts/
RUN /arrow/ci/scripts/install_cmake.sh ${cmake} /usr/local/

COPY ci/scripts/install_sccache.sh /arrow/ci/scripts/
RUN bash /arrow/ci/scripts/install_sccache.sh unknown-linux-musl /usr/local/bin

ENV \
ARROW_R_DEV=TRUE \
CMAKE=/usr/bin/cmake3
CMAKE=/usr/local/bin/cmake
4 changes: 4 additions & 0 deletions ci/docker/linux-r.dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ RUN /arrow/ci/scripts/r_docker_configure.sh
COPY ci/scripts/install_sccache.sh /arrow/ci/scripts/
RUN /arrow/ci/scripts/install_sccache.sh unknown-linux-musl /usr/local/bin

ARG cmake
COPY ci/scripts/install_cmake.sh /arrow/ci/scripts/
RUN /arrow/ci/scripts/install_cmake.sh ${cmake} /usr/local/

COPY ci/scripts/r_deps.sh /arrow/ci/scripts/
COPY r/DESCRIPTION /arrow/r/
RUN /arrow/ci/scripts/r_deps.sh /arrow
2 changes: 1 addition & 1 deletion ci/docker/python-wheel-manylinux.dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ ENV PATH=/opt/python/${CPYTHON_VERSION}-${CPYTHON_VERSION}/bin:${PATH}
# Install CMake
ARG cmake=3.29.2
COPY ci/scripts/install_cmake.sh arrow/ci/scripts/
RUN /arrow/ci/scripts/install_cmake.sh ${arch} linux ${cmake} /usr/local
RUN /arrow/ci/scripts/install_cmake.sh ${cmake} /usr/local

# Install Ninja
ARG ninja=1.10.2
Expand Down
5 changes: 4 additions & 1 deletion ci/docker/ubuntu-20.04-cpp-minimal.dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ RUN apt-get update -y -q && \
apt-get install -y -q \
build-essential \
ccache \
cmake \
curl \
gdb \
git \
Expand Down Expand Up @@ -68,6 +67,10 @@ RUN latest_system_llvm=10 && \
apt-get clean && \
rm -rf /var/lib/apt/lists*

ARG cmake
COPY ci/scripts/install_cmake.sh /arrow/ci/scripts/
RUN /arrow/ci/scripts/install_cmake.sh ${cmake} /usr/local/

COPY ci/scripts/install_minio.sh /arrow/ci/scripts/
RUN /arrow/ci/scripts/install_minio.sh latest /usr/local

Expand Down
5 changes: 4 additions & 1 deletion ci/docker/ubuntu-20.04-cpp.dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ RUN apt-get update -y -q && \
autoconf \
ca-certificates \
ccache \
cmake \
curl \
g++ \
gcc \
Expand Down Expand Up @@ -121,6 +120,10 @@ RUN apt-get update -y -q && \
apt-get clean && \
rm -rf /var/lib/apt/lists*

ARG cmake
COPY ci/scripts/install_cmake.sh /arrow/ci/scripts/
RUN /arrow/ci/scripts/install_cmake.sh ${cmake} /usr/local/

COPY ci/scripts/install_minio.sh /arrow/ci/scripts/
RUN /arrow/ci/scripts/install_minio.sh latest /usr/local

Expand Down
4 changes: 4 additions & 0 deletions ci/docker/ubuntu-22.04-cpp-minimal.dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ RUN latest_system_llvm=14 && \
apt-get clean && \
rm -rf /var/lib/apt/lists*

ARG cmake
COPY ci/scripts/install_cmake.sh /arrow/ci/scripts/
RUN /arrow/ci/scripts/install_cmake.sh ${cmake} /usr/local/

COPY ci/scripts/install_minio.sh /arrow/ci/scripts/
RUN /arrow/ci/scripts/install_minio.sh latest /usr/local

Expand Down
5 changes: 4 additions & 1 deletion ci/docker/ubuntu-22.04-cpp.dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ RUN apt-get update -y -q && \
ceph \
ceph-fuse \
ceph-mds \
cmake \
curl \
gdb \
git \
Expand Down Expand Up @@ -168,6 +167,10 @@ RUN if [ "${gcc}" = "" ]; then \
# make sure zlib is cached in the EMSDK folder
RUN source ~/emsdk/emsdk_env.sh && embuilder --pic build zlib

ARG cmake
COPY ci/scripts/install_cmake.sh /arrow/ci/scripts/
RUN /arrow/ci/scripts/install_cmake.sh ${cmake} /usr/local/

COPY ci/scripts/install_minio.sh /arrow/ci/scripts/
RUN /arrow/ci/scripts/install_minio.sh latest /usr/local

Expand Down
54 changes: 54 additions & 0 deletions ci/rtools/aws_c_common_ep.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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)
+ endif()
+
# -moutline-atomics generates code for both older load/store exclusive atomics and also
# Arm's Large System Extensions (LSE) which scale substantially better on large core count systems.
#
diff --git a/include/aws/common/byte_order.inl b/include/aws/common/byte_order.inl
index 1204be0..0abd9cb 100644
--- a/include/aws/common/byte_order.inl
+++ b/include/aws/common/byte_order.inl
@@ -13,7 +13,7 @@
# include <stdlib.h>
#else
# include <netinet/in.h>
-#endif /* _MSC_VER */
+#endif /* _WIN32 */

AWS_EXTERN_C_BEGIN

@@ -39,7 +39,7 @@ AWS_STATIC_IMPL uint64_t aws_hton64(uint64_t x) {
uint64_t v;
__asm__("bswap %q0" : "=r"(v) : "0"(x));
return v;
-#elif defined(_MSC_VER)
+#elif defined(_WIN32)
return _byteswap_uint64(x);
#else
uint32_t low = x & UINT32_MAX;
88 changes: 88 additions & 0 deletions ci/rtools/aws_c_io_ep.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
# 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/source/windows/host_resolver.c b/source/windows/host_resolver.c
index 59fbb85..ad4a99e 100644
--- a/source/windows/host_resolver.c
+++ b/source/windows/host_resolver.c
@@ -4,6 +4,7 @@
*/

/* don't move this below the Windows.h include!!!!*/
+#define _WIN32_WINNT 0x0601
#include <winsock2.h>
#include <ws2tcpip.h>

diff --git a/source/windows/iocp/iocp_event_loop.c b/source/windows/iocp/iocp_event_loop.c
index 9ccce30..5cbbef7 100644
--- a/source/windows/iocp/iocp_event_loop.c
+++ b/source/windows/iocp/iocp_event_loop.c
@@ -12,6 +12,7 @@

#include <aws/io/logging.h>

+#define _WIN32_WINNT 0x0601
#include <Windows.h>

/* The next set of struct definitions are taken directly from the
diff --git a/source/windows/secure_channel_tls_handler.c b/source/windows/secure_channel_tls_handler.c
index 50caf02..29fe850 100644
--- a/source/windows/secure_channel_tls_handler.c
+++ b/source/windows/secure_channel_tls_handler.c
@@ -2,6 +2,7 @@
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
* SPDX-License-Identifier: Apache-2.0.
*/
+#define _WIN32_WINNT 0x0601
#define SECURITY_WIN32

#include <aws/io/tls_channel_handler.h>
@@ -35,6 +36,25 @@
# pragma warning(disable : 4306) /* Identifier is type cast to a larger pointer. */
#endif

+#ifndef SP_PROT_TLS1_0_SERVER
+#define SP_PROT_TLS1_0_SERVER SP_PROT_TLS1_SERVER
+#endif
+#ifndef SP_PROT_TLS1_0_CLIENT
+#define SP_PROT_TLS1_0_CLIENT SP_PROT_TLS1_CLIENT
+#endif
+#ifndef SP_PROT_TLS1_1_SERVER
+#define SP_PROT_TLS1_1_SERVER 0x00000100
+#endif
+#ifndef SP_PROT_TLS1_1_CLIENT
+#define SP_PROT_TLS1_1_CLIENT 0x00000200
+#endif
+#ifndef SCH_USE_STRONG_CRYPTO
+#define SCH_USE_STRONG_CRYPTO 0x00400000
+#endif
+#ifndef SECBUFFER_ALERT
+#define SECBUFFER_ALERT 0x11
+#endif
+
#define KB_1 1024
#define READ_OUT_SIZE (16 * KB_1)
#define READ_IN_SIZE READ_OUT_SIZE
@@ -456,7 +476,7 @@ static int s_fillin_alpn_data(

*extension_length += sizeof(uint32_t) + sizeof(uint16_t);

- *extension_name = SecApplicationProtocolNegotiationExt_ALPN;
+ *extension_name = 2;
Comment on lines +84 to +85
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to reviewers: I tried multiple things to include the header that defines this enum and nothing worked, even things that looked like they should. I think we can get away with hardcoding this because the path only applies to Rtools40 which I think we only keep around for a subset of our users.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we include sspi.h explicitly for it?

It seems that it's defined in sspi.h: https://learn.microsoft.com/en-us/windows/win32/api/sspi/ne-sspi-sec_application_protocol_negotiation_ext

/*now add the protocols*/
for (size_t i = 0; i < protocols_count; ++i) {
struct aws_byte_cursor *protocol_ptr = NULL;
Loading
Loading