Skip to content

Commit cc4e0f3

Browse files
koujonkeane
authored andcommitted
GH-45096: [C++] Apply a cstdint patch to bundled Thrift for GCC 15 (#45097)
### Rationale for this change Apache Thrift misses `#include <cstdint>` for `int64_t`: apache/thrift#3078 GCC 15 requires it. ### What changes are included in this PR? Apply apache/thrift#3078 only with GCC 15. ### Are these changes tested? Yes. ### Are there any user-facing changes? No. * GitHub Issue: #45096 Authored-by: Sutou Kouhei <kou@clear-code.com> Signed-off-by: Jonathan Keane <jkeane@gmail.com>
1 parent 151becb commit cc4e0f3

File tree

2 files changed

+84
-1
lines changed

2 files changed

+84
-1
lines changed

cpp/cmake_modules/ThirdpartyToolchain.cmake

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1777,13 +1777,28 @@ macro(build_thrift)
17771777
set(THRIFT_DEPENDENCIES ${THRIFT_DEPENDENCIES} boost_ep)
17781778
endif()
17791779

1780+
set(THRIFT_PATCH_COMMAND)
1781+
if(CMAKE_COMPILER_IS_GNUCC AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 15.0)
1782+
# Thrift 0.21.0 doesn't support GCC 15.
1783+
# https://github.com/apache/arrow/issues/45096
1784+
# https://github.com/apache/thrift/pull/3078
1785+
find_program(PATCH patch REQUIRED)
1786+
list(APPEND
1787+
THRIFT_PATCH_COMMAND
1788+
${PATCH}
1789+
-p1
1790+
-i
1791+
${CMAKE_CURRENT_LIST_DIR}/thrift-cstdint.patch)
1792+
endif()
1793+
17801794
externalproject_add(thrift_ep
17811795
${EP_COMMON_OPTIONS}
17821796
URL ${THRIFT_SOURCE_URL}
17831797
URL_HASH "SHA256=${ARROW_THRIFT_BUILD_SHA256_CHECKSUM}"
17841798
BUILD_BYPRODUCTS "${THRIFT_LIB}"
17851799
CMAKE_ARGS ${THRIFT_CMAKE_ARGS}
1786-
DEPENDS ${THRIFT_DEPENDENCIES})
1800+
DEPENDS ${THRIFT_DEPENDENCIES}
1801+
PATCH_COMMAND ${THRIFT_PATCH_COMMAND})
17871802

17881803
add_library(thrift::thrift STATIC IMPORTED)
17891804
# The include directory must exist before it is referenced by a target.
Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
# Licensed to the Apache Software Foundation (ASF) under one
2+
# or more contributor license agreements. See the NOTICE file
3+
# distributed with this work for additional information
4+
# regarding copyright ownership. The ASF licenses this file
5+
# to you under the Apache License, Version 2.0 (the
6+
# "License"); you may not use this file except in compliance
7+
# with the License. You may obtain a copy of the License at
8+
#
9+
# http://www.apache.org/licenses/LICENSE-2.0
10+
#
11+
# Unless required by applicable law or agreed to in writing,
12+
# software distributed under the License is distributed on an
13+
# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
14+
# KIND, either express or implied. See the License for the
15+
# specific language governing permissions and limitations
16+
# under the License.
17+
18+
# https://github.com/apache/thrift/pull/3078
19+
20+
From 1920f04398ca32e320f6cf942534ba9d8b3231fd Mon Sep 17 00:00:00 2001
21+
From: Sutou Kouhei <kou@clear-code.com>
22+
Date: Mon, 23 Dec 2024 12:33:22 +0900
23+
Subject: [PATCH] THRIFT-5842: Add missing cstdint include for int64_t in
24+
Mutex.h
25+
26+
Client: cpp
27+
28+
GCC 15 (not released yet) requires `#include <cstdint>` for `int64_t`
29+
but `lib/cpp/src/thrift/concurrency/Mutex.h` doesn't have it. So we
30+
can't build Thrift with GCC 15:
31+
32+
[80/359] Building CXX object lib/cpp/CMakeFiles/thrift.dir/src/thrift/transport/TSSLServerSocket.cpp.o
33+
FAILED: lib/cpp/CMakeFiles/thrift.dir/src/thrift/transport/TSSLServerSocket.cpp.o
34+
/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
35+
In file included from /home/kou/work/cpp/thrift.kou/lib/cpp/src/thrift/transport/TServerSocket.h:25,
36+
from /home/kou/work/cpp/thrift.kou/lib/cpp/src/thrift/transport/TSSLServerSocket.h:23,
37+
from /home/kou/work/cpp/thrift.kou/lib/cpp/src/thrift/transport/TSSLServerSocket.cpp:21:
38+
/home/kou/work/cpp/thrift.kou/lib/cpp/src/thrift/concurrency/Mutex.h:47:26: error: 'int64_t' has not been declared
39+
47 | virtual bool timedlock(int64_t milliseconds) const;
40+
| ^~~~~~~
41+
/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>'
42+
24 | #include <thrift/TNonCopyable.h>
43+
+++ |+#include <cstdint>
44+
25 |
45+
/home/kou/work/cpp/thrift.kou/lib/cpp/src/thrift/concurrency/Mutex.h:60:29: error: 'int64_t' has not been declared
46+
60 | Guard(const Mutex& value, int64_t timeout = 0) : mutex_(&value) {
47+
| ^~~~~~~
48+
/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>'
49+
50+
See also: https://github.com/apache/arrow/issues/45096
51+
---
52+
lib/cpp/src/thrift/concurrency/Mutex.h | 1 +
53+
1 file changed, 1 insertion(+)
54+
55+
diff --git a/lib/cpp/src/thrift/concurrency/Mutex.h b/lib/cpp/src/thrift/concurrency/Mutex.h
56+
index 1e5c3fba3..12f1729d6 100644
57+
--- a/lib/cpp/src/thrift/concurrency/Mutex.h
58+
+++ b/lib/cpp/src/thrift/concurrency/Mutex.h
59+
@@ -20,6 +20,7 @@
60+
#ifndef _THRIFT_CONCURRENCY_MUTEX_H_
61+
#define _THRIFT_CONCURRENCY_MUTEX_H_ 1
62+
63+
+#include <cstdint>
64+
#include <memory>
65+
#include <thrift/TNonCopyable.h>
66+
67+
--
68+
2.45.2

0 commit comments

Comments
 (0)