Skip to content

Commit 69fd679

Browse files
czentgrczentgr
committed
Prestissimo fixes due to FBOS upgrade
1 parent 39ba8d9 commit 69fd679

File tree

9 files changed

+82
-43
lines changed

9 files changed

+82
-43
lines changed

‎.github/workflows/arrow-flight-tests.yml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ jobs:
122122
-DCMAKE_PREFIX_PATH=/usr/local \
123123
-DThrift_ROOT=/usr/local \
124124
-DCMAKE_CXX_COMPILER_LAUNCHER=ccache \
125-
-DMAX_LINK_JOBS=4
125+
-DMAX_LINK_JOBS=3
126126
ninja -C _build/release -j 4
127127
128128
- name: Ccache after

‎.github/workflows/prestocpp-macos-build.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,8 @@ jobs:
7474
# First selectively install the minimal dependencies for Velox.
7575
install_build_prerequisites
7676
install_velox_deps_from_brew
77+
brew upgrade openssl@3
78+
brew upgrade cmake
7779
install_double_conversion
7880
7981
# Install glog/gflags because they are not installed from homebrew.

‎presto-native-execution/CMakeLists.txt‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,9 @@ find_library(PROXYGEN_HTTP_SERVER proxygenhttpserver)
182182
find_library(FIZZ fizz)
183183
find_library(WANGLE wangle)
184184
find_library(MVFST_EXCEPTION mvfst_exception)
185+
find_library(MVFST_FOLLY_UTILS mvfst_folly_utils)
186+
find_library(MVFST_CODEC_TYPES mvfst_codec_types)
187+
find_library(MVFST_CONTIGUOUS_CURSOR mvfst_contiguous_cursor)
185188

186189
find_library(RE2 re2)
187190

@@ -197,6 +200,9 @@ set(
197200
${WANGLE}
198201
${FIZZ}
199202
${MVFST_EXCEPTION}
203+
${MVFST_FOLLY_UTILS}
204+
${MVFST_CODEC_TYPES}
205+
${MVFST_CONTIGUOUS_CURSOR}
200206
)
201207
find_path(PROXYGEN_DIR NAMES include/proxygen)
202208
set(PROXYGEN_INCLUDE_DIR "${PROXYGEN_DIR}/include/")

‎presto-native-execution/presto_cpp/main/connectors/IcebergPrestoToVeloxConnector.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,11 @@ velox::parquet::ParquetFieldId toParquetField(
159159
children.push_back(toParquetField(child));
160160
}
161161
}
162-
return velox::parquet::ParquetFieldId(column.id, children);
162+
// ParquetFieldId does not declare a constructor that takes fieldId and
163+
// children, so we use aggregate initialization to make it work for compilers
164+
// that don't create the necessary constructors by default (e.g clang-15).
165+
velox::parquet::ParquetFieldId pf{.fieldId = column.id, .children = children};
166+
return pf;
163167
}
164168

165169
} // namespace

‎presto-native-execution/presto_cpp/main/thrift/CMakeLists.txt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,16 @@ find_library(THRIFT_METADATA thriftmetadata)
1818
find_library(THRIFT_TRANSPORT transport)
1919
find_path(THRIFT_INCLUDES thrift/lib/cpp2/gen/module_data_h.h PATH_SUFFIXES include REQUIRED)
2020

21+
# Find libevent on macOS (required by FBThrift)
22+
# Usually /opt/homebrew/include is a CMake isystem path but sometimes it is not (e.g. in the CI).
23+
# It doesn't hurt making sure event.h will be found..
24+
if(APPLE)
25+
find_package(Libevent REQUIRED)
26+
if(Libevent_FOUND)
27+
include_directories(SYSTEM ${LIBEVENT_INCLUDE_DIRS})
28+
endif()
29+
endif()
30+
2131
include(ThriftLibrary.cmake)
2232

2333
thrift_library(

‎presto-native-execution/presto_cpp/main/thrift/ThriftLibrary.cmake‎

Lines changed: 54 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# Copyright (c) Facebook, Inc. and its affiliates.
1+
# Copyright (c) Meta Platforms, Inc. and affiliates.
22
#
33
# Licensed under the Apache License, Version 2.0 (the "License");
44
# you may not use this file except in compliance with the License.
@@ -117,7 +117,7 @@ endmacro()
117117
# #include_prefix
118118
# )
119119
# add_library(somelib ...)
120-
# target_link_libraries(somelibe ${file_name}-${language} ...)
120+
# target_link_libraries(somelib ${file_name}-${language} ...)
121121
#
122122

123123
macro(thrift_library
@@ -167,15 +167,21 @@ endmacro()
167167
#
168168
# thrift_generate
169169
# This is used to codegen thrift files using the thrift compiler
170+
# Supports library names that differ from the file name (to handle two libraries
171+
# with the same filename on disk (in different folders))
170172
# Params:
171-
# @file_name - The name of tge thrift file
173+
# @file_name - Input file name. Will be used for naming the CMake
174+
# target if TARGET_NAME_BASE is not specified.
172175
# @services - A list of services that are declared in the thrift file
173176
# @language - The generator to use (cpp, cpp2 or py3)
174177
# @options - Extra options to pass to the generator
175178
# @output_path - The directory where the thrift file lives
179+
# @include_prefix - Prefix to use for thrift includes in generated sources
180+
# @TARGET_NAME_BASE (optional) - name used for target instead of real filename
181+
# @THRIFT_INCLUDE_DIRECTORIES (optional) path to thrift include directories
176182
#
177183
# Output:
178-
# file-language-target - A custom target to add a dependenct
184+
# file-language-target - A custom target to add a dependency
179185
# ${file-language-HEADERS} - The generated Header Files.
180186
# ${file-language-SOURCES} - The generated Source Files.
181187
#
@@ -186,7 +192,6 @@ endmacro()
186192
# bypass_source_check(${file_language-SOURCES})
187193
# This will prevent cmake from complaining about missing source files
188194
#
189-
190195
macro(thrift_generate
191196
file_name
192197
services
@@ -198,43 +203,59 @@ macro(thrift_generate
198203
)
199204
cmake_parse_arguments(THRIFT_GENERATE # Prefix
200205
"" # Options
201-
"" # One Value args
206+
"TARGET_NAME_BASE" # One Value args
202207
"THRIFT_INCLUDE_DIRECTORIES" # Multi-value args
203208
"${ARGN}")
204209

210+
set(source_file_name ${file_name})
211+
set(target_file_name ${file_name})
205212
set(thrift_include_directories)
206213
foreach(dir ${THRIFT_GENERATE_THRIFT_INCLUDE_DIRECTORIES})
207214
list(APPEND thrift_include_directories "-I" "${dir}")
208215
endforeach()
216+
if(DEFINED THRIFT_GENERATE_TARGET_NAME_BASE
217+
AND NOT THRIFT_GENERATE_TARGET_NAME_BASE STREQUAL "")
218+
set(target_file_name ${THRIFT_GENERATE_TARGET_NAME_BASE})
219+
endif()
209220

210-
set("${file_name}-${language}-HEADERS"
211-
${output_path}/gen-${language}/${file_name}_constants.h
212-
${output_path}/gen-${language}/${file_name}_data.h
213-
${output_path}/gen-${language}/${file_name}_metadata.h
214-
${output_path}/gen-${language}/${file_name}_types.h
215-
${output_path}/gen-${language}/${file_name}_types.tcc
221+
set("${target_file_name}-${language}-HEADERS"
222+
${output_path}/gen-${language}/${source_file_name}_constants.h
223+
${output_path}/gen-${language}/${source_file_name}_data.h
224+
${output_path}/gen-${language}/${source_file_name}_metadata.h
225+
${output_path}/gen-${language}/${source_file_name}_types.h
226+
${output_path}/gen-${language}/${source_file_name}_types.tcch
227+
${output_path}/gen-${language}/${source_file_name}_types_custom_protocol.h
216228
)
217-
set("${file_name}-${language}-SOURCES"
218-
${output_path}/gen-${language}/${file_name}_constants.cpp
219-
${output_path}/gen-${language}/${file_name}_data.cpp
220-
${output_path}/gen-${language}/${file_name}_types.cpp
229+
set("${target_file_name}-${language}-SOURCES"
230+
${output_path}/gen-${language}/${source_file_name}_constants.cpp
231+
${output_path}/gen-${language}/${source_file_name}_data.cpp
232+
${output_path}/gen-${language}/${source_file_name}_types.cpp
233+
${output_path}/gen-${language}/${source_file_name}_types_binary.cpp
234+
${output_path}/gen-${language}/${source_file_name}_types_compact.cpp
235+
${output_path}/gen-${language}/${source_file_name}_types_serialization.cpp
221236
)
237+
if("${options}" MATCHES "layouts")
238+
set("${target_file_name}-${language}-SOURCES"
239+
${${target_file_name}-${language}-SOURCES}
240+
${output_path}/gen-${language}/${source_file_name}_layouts.cpp
241+
)
242+
endif()
222243
if(NOT "${options}" MATCHES "no_metadata")
223-
set("${file_name}-${language}-SOURCES"
224-
${${file_name}-${language}-SOURCES}
225-
${output_path}/gen-${language}/${file_name}_metadata.cpp
244+
set("${target_file_name}-${language}-SOURCES"
245+
${${target_file_name}-${language}-SOURCES}
246+
${output_path}/gen-${language}/${source_file_name}_metadata.cpp
226247
)
227248
endif()
228249
foreach(service ${services})
229-
set("${file_name}-${language}-HEADERS"
230-
${${file_name}-${language}-HEADERS}
250+
set("${target_file_name}-${language}-HEADERS"
251+
${${source_file_name}-${language}-HEADERS}
231252
${output_path}/gen-${language}/${service}.h
232253
${output_path}/gen-${language}/${service}.tcc
233254
${output_path}/gen-${language}/${service}AsyncClient.h
234255
${output_path}/gen-${language}/${service}_custom_protocol.h
235256
)
236-
set("${file_name}-${language}-SOURCES"
237-
${${file_name}-${language}-SOURCES}
257+
set("${target_file_name}-${language}-SOURCES"
258+
${${source_file_name}-${language}-SOURCES}
238259
${output_path}/gen-${language}/${service}.cpp
239260
${output_path}/gen-${language}/${service}AsyncClient.cpp
240261
)
@@ -252,26 +273,27 @@ macro(thrift_generate
252273
set(gen_language "mstch_cpp2")
253274
elseif("${language}" STREQUAL "py3")
254275
set(gen_language "mstch_py3")
255-
file(WRITE "${output_path}/gen-${language}/${file_name}/__init__.py")
276+
file(WRITE "${output_path}/gen-${language}/${source_file_name}/__init__.py")
256277
endif()
278+
message(STATUS "Thrift command:")
279+
message(STATUS "${THRIFT1} -v --gen \"${gen_language}:${options}${include_prefix_text}\" -o ${output_path} ${thrift_include_directories} \"${file_path}/${source_file_name}.thrift\"")
257280
add_custom_command(
258-
OUTPUT ${${file_name}-${language}-HEADERS}
259-
${${file_name}-${language}-SOURCES}
260-
COMMAND mkdir -p ${output_path}
281+
OUTPUT ${${target_file_name}-${language}-HEADERS}
282+
${${target_file_name}-${language}-SOURCES}
261283
COMMAND ${THRIFT1}
262284
--gen "${gen_language}:${options}${include_prefix_text}"
263285
-o ${output_path}
264286
${thrift_include_directories}
265-
"${file_path}/${file_name}.thrift"
287+
"${file_path}/${source_file_name}.thrift"
266288
DEPENDS
267289
${THRIFT1}
268-
"${file_path}/${file_name}.thrift"
269-
COMMENT "Generating ${file_name} files. Output: ${output_path}"
290+
"${file_path}/${source_file_name}.thrift"
291+
COMMENT "Generating ${target_file_name} files. Output: ${output_path}. Command: ${THRIFT1} -v --gen \"${gen_language}:${options}${include_prefix_text}\" -o ${output_path} ${thrift_include_directories} \"${file_path}/${source_file_name}.thrift\""
270292
)
271293
add_custom_target(
272-
${file_name}-${language}-target ALL
294+
${target_file_name}-${language}-target ALL
273295
DEPENDS ${${language}-${language}-HEADERS}
274-
${${file_name}-${language}-SOURCES}
296+
${${target_file_name}-${language}-SOURCES}
275297
)
276298
install(
277299
DIRECTORY gen-${language}

‎presto-native-execution/presto_cpp/main/thrift/tests/CMakeLists.txt‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,6 @@ target_link_libraries(
2424
presto_thrift_extra
2525
presto_thrift-cpp2
2626
presto_native-cpp2
27-
${THRIFTCPP2}
28-
${THRIFT_PROTOCOL}
29-
${THRIFT_METADATA}
30-
${THRIFT_CORE}
31-
${THRIFT_TRANSPORT}
32-
${FOLLY_WITH_DEPENDENCIES}
3327
${RE2}
3428
GTest::gmock
3529
GTest::gtest

‎presto-native-execution/scripts/setup-centos.sh‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,8 @@ fi
3232
export NPROC=${NPROC:-$(getconf _NPROCESSORS_ONLN)}
3333

3434
function install_presto_deps_from_package_managers {
35-
dnf install -y maven java clang-tools-extra jq perl-XML-XPath
35+
# proxygen requires c-ares-devel
36+
dnf install -y maven java clang-tools-extra jq perl-XML-XPath c-ares-devel
3637
# This python version is installed by the Velox setup scripts
3738
pip install regex pyyaml chevron black ptsd-jbroll
3839
}

‎presto-native-execution/scripts/setup-ubuntu.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,9 +22,9 @@ SUDO="${SUDO:-"sudo --preserve-env"}"
2222
DATASKETCHES_VERSION="5.2.0"
2323

2424
function install_proxygen {
25-
# proxygen requires python and gperf
25+
# proxygen requires python, gperf, and libc-ares-dev
2626
${SUDO} apt update
27-
${SUDO} apt install -y gperf python3
27+
${SUDO} apt install -y gperf python3 libc-ares-dev
2828
wget_and_untar https://github.com/facebook/proxygen/archive/refs/tags/${FB_OS_VERSION}.tar.gz proxygen
2929
cmake_install_dir proxygen -DBUILD_TESTS=OFF
3030
}

0 commit comments

Comments
 (0)