Skip to content

Commit 40212a2

Browse files
Move ANTLR4 dependency inclusion to top-level CMakeLists.txt (#127)
MSVC has a hard 260 character limit which (unfortunately) isn't immediately fixable (note: this is independent of the "infamous" Windows path length limit). `substrait-cpp` can become an issue when it itself is being used as a dependency of other projects ([see e.g. this discussion](substrait-io/substrait-mlir-contrib#92 (comment)). Here, it's the `antlr4` build that's generating the longest paths, e.g.: > `substrait-cpp/src/substrait/textplan/parser/grammar/antlr4_runtime/src/antlr4_runtime/runtime/Cpp/CMakeFiles/CMakeScratch/TryCompile-s3o9if/CMakeFiles/cmTC_945a8.dir/./` To give ourselves a bit of breathing room here, `src/substrait/textplan/parser/grammar` seems fairly unnecessary, and is simply a side-effect of `include(ExternalAntlr4Cpp)` being called at that nested location. This PR moves that inclusion up to the top-level, giving MSVC builds an extra 38 characters to work with 🙂. Another possible work around could be to rewrite the ANTLR4 dependency management s.t. `FetchContent` is used to manage the build. This would mean that the dependency would be built in e.g. the root `build/_deps/antlr4` location, irrespective of any git submodule nesting. * Also allows setting `ANTLR_BUILD_CPP_TESTS` for the ANTLR build (avoids ANTLR pulling in gtest which may conflict with other things when `substrait-cpp` is a dependency of other projects). * Removes the assumption that antlr libraries are located in the `dist` folder within the ANTLR checked-out source hierarchy (we're doing an in-source build). AFAICT, [this behavior places a hard assumption on `ANTLR_BUILD_CPP_TESTS` being set](https://github.com/antlr/antlr4/blob/dev/runtime/Cpp/runtime/CMakeLists.txt#L159-L177) which is what triggers the behaviour for copying binaries to the `dist` folder. --------- Co-authored-by: David Sisson <[email protected]>
1 parent 5d05a2b commit 40212a2

File tree

4 files changed

+27
-27
lines changed

4 files changed

+27
-27
lines changed

CMakeLists.txt

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,26 @@ option(
3131

3232
set(SUBSTRAIT_CPP_INCLUDE_DIR ${CMAKE_CURRENT_SOURCE_DIR}/include)
3333

34+
# Setup ANTLR4 dependency. It's important that this is done at the
35+
# top-level CMakeLists.txt, given that the ANTLR4 build may generate deeply nested
36+
# paths - which may be a problem for MSVC, with its 260 character path limit.
37+
list(APPEND CMAKE_MODULE_PATH
38+
"${CMAKE_CURRENT_SOURCE_DIR}/third_party/antlr4/cmake")
39+
40+
set(CMAKE_CXX_STANDARD 17)
41+
add_definitions(-DANTLR4CPP_STATIC)
42+
# using /MD flag for antlr4_runtime (for Visual C++ compilers only)
43+
set(ANTLR4_WITH_STATIC_CRT OFF)
44+
set(ANTLR4_TAG 4.13.2)
45+
set(ANTLR4_ZIP_REPOSITORY
46+
https://github.com/antlr/antlr4/archive/refs/tags/${ANTLR4_TAG}.zip)
47+
include(ExternalAntlr4Cpp)
48+
include_directories(${ANTLR4_INCLUDE_DIRS})
49+
set(ANTLR_EXECUTABLE_DIR ${CMAKE_CURRENT_BINARY_DIR})
50+
file(DOWNLOAD https://www.antlr.org/download/antlr-4.13.2-complete.jar
51+
"${ANTLR_EXECUTABLE_DIR}/antlr.jar")
52+
set(ANTLR_EXECUTABLE "${ANTLR_EXECUTABLE_DIR}/antlr.jar")
53+
3454
# Local files come first.
3555
include_directories(include)
3656
include_directories(src)

scripts/run-clang-tidy.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ rm -rf tmp && mkdir tmp && cmake -Btmp -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DSUBS
88
# Build substrait protobuf
99
pushd tmp/src/substrait/proto && make -j 2 && popd || exit
1010
# Build textplan grammar
11-
pushd tmp/src/substrait/textplan/parser/grammar && make -j antlr4_runtime textplan_grammar_headers && popd || exit
11+
pushd tmp/ && make -j antlr4_runtime textplan_grammar_headers && popd || exit
1212
# Run clang-tidy
1313
if [ "$1" == "fix" ]; then
1414
python3 scripts/run-clang-tidy.py "$WORKDIR" "tmp" "third_party" "h,hpp,cc,cpp" "--quiet --fix"

src/substrait/textplan/parser/grammar/CMakeLists.txt

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -1,28 +1,5 @@
11
# SPDX-License-Identifier: Apache-2.0
22

3-
list(APPEND CMAKE_MODULE_PATH
4-
"${CMAKE_CURRENT_SOURCE_DIR}/../../../../../third_party/antlr4/cmake")
5-
6-
set(CMAKE_CXX_STANDARD 17)
7-
8-
add_definitions(-DANTLR4CPP_STATIC)
9-
10-
set(GRAMMAR_DIR ${CMAKE_CURRENT_BINARY_DIR})
11-
12-
# using /MD flag for antlr4_runtime (for Visual C++ compilers only)
13-
set(ANTLR4_WITH_STATIC_CRT OFF)
14-
15-
set(ANTLR4_TAG 4.13.2)
16-
set(ANTLR4_ZIP_REPOSITORY
17-
https://github.com/antlr/antlr4/archive/refs/tags/${ANTLR4_TAG}.zip)
18-
19-
include(ExternalAntlr4Cpp)
20-
21-
include_directories(${ANTLR4_INCLUDE_DIRS})
22-
23-
file(DOWNLOAD https://www.antlr.org/download/antlr-4.13.2-complete.jar
24-
"${GRAMMAR_DIR}/antlr.jar")
25-
set(ANTLR_EXECUTABLE "${GRAMMAR_DIR}/antlr.jar")
263
find_package(ANTLR REQUIRED)
274

285
antlr_target(SubstraitPlanLexer SubstraitPlanLexer.g4 LEXER PACKAGE
@@ -53,6 +30,7 @@ message(
5330
add_library(textplan_grammar ${ANTLR_SubstraitPlanLexer_CXX_OUTPUTS}
5431
${ANTLR_SubstraitPlanParser_CXX_OUTPUTS})
5532

33+
set(GRAMMAR_DIR ${CMAKE_CURRENT_BINARY_DIR})
5634
message(STATUS "generated dir: ${GRAMMAR_DIR}/antlr4cpp_generated_src")
5735

5836
target_include_directories(

third_party/antlr4/cmake/ExternalAntlr4Cpp.cmake

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,11 @@ endif()
2020
file(MAKE_DIRECTORY "${ANTLR4_INCLUDE_DIRS}")
2121

2222
if(${CMAKE_GENERATOR} MATCHES "Visual Studio.*")
23-
set(ANTLR4_OUTPUT_DIR ${ANTLR4_ROOT}/runtime/Cpp/dist/$(Configuration))
23+
set(ANTLR4_OUTPUT_DIR ${ANTLR4_ROOT}/runtime/Cpp/runtime/$(Configuration))
2424
elseif(${CMAKE_GENERATOR} MATCHES "Xcode.*")
25-
set(ANTLR4_OUTPUT_DIR ${ANTLR4_ROOT}/runtime/Cpp/dist/$(CONFIGURATION))
25+
set(ANTLR4_OUTPUT_DIR ${ANTLR4_ROOT}/runtime/Cpp/runtime/$(CONFIGURATION))
2626
else()
27-
set(ANTLR4_OUTPUT_DIR ${ANTLR4_ROOT}/runtime/Cpp/dist)
27+
set(ANTLR4_OUTPUT_DIR ${ANTLR4_ROOT}/runtime/Cpp/runtime)
2828
endif()
2929

3030
if(MSVC)
@@ -96,6 +96,7 @@ if(ANTLR4_ZIP_REPOSITORY)
9696
CMAKE_CACHE_ARGS
9797
-DCMAKE_BUILD_TYPE:STRING=${CMAKE_BUILD_TYPE}
9898
-DWITH_STATIC_CRT:BOOL=${ANTLR4_WITH_STATIC_CRT}
99+
-DANTLR_BUILD_CPP_TESTS:BOOL=${ANTLR4_BUILD_CPP_TESTS}
99100
-DDISABLE_WARNINGS:BOOL=ON
100101
-DCMAKE_POSITION_INDEPENDENT_CODE:BOOL=ON
101102
-DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}
@@ -117,6 +118,7 @@ else()
117118
CMAKE_CACHE_ARGS
118119
-DCMAKE_BUILD_TYPE:STRING=${CMAKE_BUILD_TYPE}
119120
-DWITH_STATIC_CRT:BOOL=${ANTLR4_WITH_STATIC_CRT}
121+
-DANTLR_BUILD_CPP_TESTS:BOOL=${ANTLR4_BUILD_CPP_TESTS}
120122
-DDISABLE_WARNINGS:BOOL=ON
121123
-DCMAKE_POSITION_INDEPENDENT_CODE:BOOL=ON
122124
-DCMAKE_CXX_COMPILER=${CMAKE_CXX_COMPILER}

0 commit comments

Comments
 (0)