Skip to content

Commit

Permalink
GH-45096: [C++] Apply a cstdint patch to bundled Thrift for GCC 15
Browse files Browse the repository at this point in the history
  • Loading branch information
kou committed Dec 23, 2024
1 parent 164e18a commit f3ecaa8
Show file tree
Hide file tree
Showing 2 changed files with 84 additions and 1 deletion.
17 changes: 16 additions & 1 deletion cpp/cmake_modules/ThirdpartyToolchain.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -1777,13 +1777,28 @@ macro(build_thrift)
set(THRIFT_DEPENDENCIES ${THRIFT_DEPENDENCIES} boost_ep)
endif()

set(THRIFT_PATCH_COMMAND)
if(CMAKE_COMPILER_IS_GNUCC AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 15.0)
# Thrift 0.21.0 doesn't support GCC 15.
# https://github.com/apache/arrow/issues/45096
# https://github.com/apache/thrift/pull/3078
find_program(PATCH patch REQUIRED)
list(APPEND
THRIFT_PATCH_COMMAND
${PATCH}
-p1
-i
${CMAKE_CURRENT_LIST_DIR}/thrift-cstdint.patch)
endif()

externalproject_add(thrift_ep
${EP_COMMON_OPTIONS}
URL ${THRIFT_SOURCE_URL}
URL_HASH "SHA256=${ARROW_THRIFT_BUILD_SHA256_CHECKSUM}"
BUILD_BYPRODUCTS "${THRIFT_LIB}"
CMAKE_ARGS ${THRIFT_CMAKE_ARGS}
DEPENDS ${THRIFT_DEPENDENCIES})
DEPENDS ${THRIFT_DEPENDENCIES}
PATCH_COMMAND ${THRIFT_PATCH_COMMAND})

add_library(thrift::thrift STATIC IMPORTED)
# The include directory must exist before it is referenced by a target.
Expand Down
68 changes: 68 additions & 0 deletions cpp/cmake_modules/thrift-cstdint.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# 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.

# https://github.com/apache/thrift/pull/3078

From 1920f04398ca32e320f6cf942534ba9d8b3231fd Mon Sep 17 00:00:00 2001
From: Sutou Kouhei <[email protected]>
Date: Mon, 23 Dec 2024 12:33:22 +0900
Subject: [PATCH] THRIFT-5842: Add missing cstdint include for int64_t in
Mutex.h

Client: cpp

GCC 15 (not released yet) requires `#include <cstdint>` for `int64_t`
but `lib/cpp/src/thrift/concurrency/Mutex.h` doesn't have it. So we
can't build Thrift with GCC 15:

[80/359] Building CXX object lib/cpp/CMakeFiles/thrift.dir/src/thrift/transport/TSSLServerSocket.cpp.o
FAILED: lib/cpp/CMakeFiles/thrift.dir/src/thrift/transport/TSSLServerSocket.cpp.o
/bin/g++-15 -DBOOST_ALL_DYN_LINK -DBOOST_TEST_DYN_LINK -DTHRIFT_STATIC_DEFINE -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -I/home/kou/work/cpp/thrift.kou.build/lib/cpp -I/home/kou/work/cpp/thrift.kou/lib/cpp -I/home/kou/work/cpp/thrift.kou.build -I/home/kou/work/cpp/thrift.kou/lib/cpp/src -g -std=c++11 -MD -MT lib/cpp/CMakeFiles/thrift.dir/src/thrift/transport/TSSLServerSocket.cpp.o -MF lib/cpp/CMakeFiles/thrift.dir/src/thrift/transport/TSSLServerSocket.cpp.o.d -o lib/cpp/CMakeFiles/thrift.dir/src/thrift/transport/TSSLServerSocket.cpp.o -c /home/kou/work/cpp/thrift.kou/lib/cpp/src/thrift/transport/TSSLServerSocket.cpp
In file included from /home/kou/work/cpp/thrift.kou/lib/cpp/src/thrift/transport/TServerSocket.h:25,
from /home/kou/work/cpp/thrift.kou/lib/cpp/src/thrift/transport/TSSLServerSocket.h:23,
from /home/kou/work/cpp/thrift.kou/lib/cpp/src/thrift/transport/TSSLServerSocket.cpp:21:
/home/kou/work/cpp/thrift.kou/lib/cpp/src/thrift/concurrency/Mutex.h:47:26: error: 'int64_t' has not been declared
47 | virtual bool timedlock(int64_t milliseconds) const;
| ^~~~~~~
/home/kou/work/cpp/thrift.kou/lib/cpp/src/thrift/concurrency/Mutex.h:25:1: note: 'int64_t' is defined in header '<cstdint>'; this is probably fixable by adding '#include <cstdint>'
24 | #include <thrift/TNonCopyable.h>
+++ |+#include <cstdint>
25 |
/home/kou/work/cpp/thrift.kou/lib/cpp/src/thrift/concurrency/Mutex.h:60:29: error: 'int64_t' has not been declared
60 | Guard(const Mutex& value, int64_t timeout = 0) : mutex_(&value) {
| ^~~~~~~
/home/kou/work/cpp/thrift.kou/lib/cpp/src/thrift/concurrency/Mutex.h:60:29: note: 'int64_t' is defined in header '<cstdint>'; this is probably fixable by adding '#include <cstdint>'

See also: https://github.com/apache/arrow/issues/45096
---
lib/cpp/src/thrift/concurrency/Mutex.h | 1 +
1 file changed, 1 insertion(+)

diff --git a/lib/cpp/src/thrift/concurrency/Mutex.h b/lib/cpp/src/thrift/concurrency/Mutex.h
index 1e5c3fba3..12f1729d6 100644
--- a/lib/cpp/src/thrift/concurrency/Mutex.h
+++ b/lib/cpp/src/thrift/concurrency/Mutex.h
@@ -20,6 +20,7 @@
#ifndef _THRIFT_CONCURRENCY_MUTEX_H_
#define _THRIFT_CONCURRENCY_MUTEX_H_ 1

+#include <cstdint>
#include <memory>
#include <thrift/TNonCopyable.h>

--
2.45.2

0 comments on commit f3ecaa8

Please sign in to comment.