Skip to content

Commit 1fc5687

Browse files
committed
[native] Fix build order issues with presto_server
There is an issue where libraries were added after the final presto_server target had already been created. That is, new libraries were added after presto_server_lib was used to build to presto_server. This means presto_server_lib might not have all expected symbols.
1 parent 58df37d commit 1fc5687

File tree

1 file changed

+15
-15
lines changed

1 file changed

+15
-15
lines changed

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

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -96,33 +96,33 @@ target_link_libraries(
9696
target_link_libraries(presto_server_lib presto_thrift-cpp2 presto_thrift_extra
9797
${THRIFT_LIBRARY})
9898

99+
if(PRESTO_ENABLE_REMOTE_FUNCTIONS)
100+
add_library(presto_server_remote_function JsonSignatureParser.cpp
101+
RemoteFunctionRegisterer.cpp)
102+
103+
target_link_libraries(presto_server_remote_function velox_expression
104+
velox_functions_remote ${FOLLY_WITH_DEPENDENCIES})
105+
target_link_libraries(presto_server_lib presto_server_remote_function)
106+
endif()
107+
99108
set_property(TARGET presto_server_lib PROPERTY JOB_POOL_LINK
100109
presto_link_job_pool)
101110

102111
add_executable(presto_server PrestoMain.cpp)
103112

104-
# Moving velox_hive_connector and velox_tpch_connector to presto_server_lib
105-
# results in multiple link errors similar to the one below only on GCC.
106-
# "undefined reference to `vtable for velox::connector::tpch::TpchTableHandle`"
107-
# TODO: Fix these errors.
108-
target_link_libraries(presto_server presto_server_lib velox_hive_connector
109-
velox_tpch_connector)
113+
# velox_tpch_connector is an OBJECT target in Velox and so needs to be linked
114+
# to the executable or use TARGET_OBJECT linkage for the presto_server_lib target.
115+
# However, we also would need to add its dependencies (tpch_gen etc).
116+
# TODO change the target in Velox to a library target then we can move this to the
117+
# presto_server_lib.
118+
target_link_libraries(presto_server presto_server_lib velox_tpch_connector)
110119

111120
# Clang requires explicit linking with libatomic.
112121
if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang"
113122
AND "${CMAKE_CXX_COMPILER_VERSION}" VERSION_GREATER_EQUAL 15)
114123
target_link_libraries(presto_server atomic)
115124
endif()
116125

117-
if(PRESTO_ENABLE_REMOTE_FUNCTIONS)
118-
add_library(presto_server_remote_function JsonSignatureParser.cpp
119-
RemoteFunctionRegisterer.cpp)
120-
121-
target_link_libraries(presto_server_remote_function velox_expression
122-
velox_functions_remote ${FOLLY_WITH_DEPENDENCIES})
123-
target_link_libraries(presto_server_lib presto_server_remote_function)
124-
endif()
125-
126126
set_property(TARGET presto_server PROPERTY JOB_POOL_LINK presto_link_job_pool)
127127

128128
if(PRESTO_ENABLE_TESTING)

0 commit comments

Comments
 (0)