Skip to content

Commit 926d70e

Browse files
dbortdbort
committed
Add EXECUTORCH_SEPARATE_FLATCC_HOST_PROJECT cmake option (#3356)
Summary: Currently, we always build two copies of the flatcc targets, just in case we happen to be cross-compiling. But because the flatcc project puts its binaries in the source directory, those two copies can interfere with each other. We don't need to build two copies when not cross-compiling, so add a new option to avoid the second "host" build. Eventually we should only enable this when cross-compiling, but for now disable it when building the pip package (which is never cross-compiled). Pull Request resolved: #3356 Test Plan: `rm -rf pip-out && ./install_requirements.sh` succeeded. Looking in the `pip-out/temp.*/cmake-out` directory, there is no `_host_build` directory, but the etdump headers were successfully generated under `pip-out/temp.*/cmake-out/sdk/include/executorch/sdk/etdump/`. Reviewed By: malfet, larryliu0820 Differential Revision: D56582507 Pulled By: dbort fbshipit-source-id: 4ce6c680657bc57cfcf016826364a3f46c4c953e
1 parent 835cdaa commit 926d70e

File tree

2 files changed

+59
-27
lines changed

2 files changed

+59
-27
lines changed

‎sdk/CMakeLists.txt

Lines changed: 54 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -69,18 +69,33 @@ include(ExternalProject)
6969
set(_program_schema__include_dir "${CMAKE_BINARY_DIR}/sdk/include")
7070
set(_bundled_schema__include_dir "${CMAKE_BINARY_DIR}/sdk/bundled_program")
7171

72-
# Add the host project
73-
# lint_cmake: -readability/wonkycase
74-
ExternalProject_Add(
75-
flatcc_project
76-
PREFIX ${CMAKE_BINARY_DIR}/_host_build
77-
SOURCE_DIR ${_flatcc_source_dir}
78-
BINARY_DIR ${CMAKE_BINARY_DIR}/_host_build
79-
CMAKE_CACHE_ARGS -DFLATCC_TEST:BOOL=OFF -DFLATCC_REFLECTION:BOOL=OFF
72+
# TODO(dbort): Only enable this when cross-compiling. It can cause build race
73+
# conditions (libflatcc.a errors) when enabled.
74+
option(EXECUTORCH_SEPARATE_FLATCC_HOST_PROJECT
75+
"Whether to build the flatcc commandline tool as a separate project" ON)
76+
77+
if(EXECUTORCH_SEPARATE_FLATCC_HOST_PROJECT)
78+
# Add the host project. We build this separately so that we can generate
79+
# headers on the host during the build, even if we're cross-compiling the
80+
# flatcc runtime to a different architecture.
81+
82+
# lint_cmake: -readability/wonkycase
83+
ExternalProject_Add(
84+
flatcc_project
85+
PREFIX ${CMAKE_BINARY_DIR}/_host_build
86+
SOURCE_DIR ${_flatcc_source_dir}
87+
BINARY_DIR ${CMAKE_BINARY_DIR}/_host_build
88+
CMAKE_CACHE_ARGS
89+
-DFLATCC_TEST:BOOL=OFF -DFLATCC_REFLECTION:BOOL=OFF
8090
# See above comment about POSITION_INDEPENDENT_CODE.
8191
-DCMAKE_POSITION_INDEPENDENT_CODE:BOOL=ON
82-
INSTALL_COMMAND "" # Prevent the install step, modify as needed
83-
)
92+
INSTALL_COMMAND "" # Prevent the install step
93+
)
94+
set(_etdump_schema_gen_dep flatcc_project)
95+
else()
96+
# If we're not cross-compiling, we can just use the plain commandline target.
97+
set(_etdump_schema_gen_dep flatcc_cli)
98+
endif()
8499

85100
set(_etdump_schema__outputs)
86101
foreach(fbs_file ${_etdump_schema_names})
@@ -114,33 +129,45 @@ file(MAKE_DIRECTORY ${_program_schema__include_dir}/executorch/sdk/etdump)
114129
file(MAKE_DIRECTORY
115130
${_program_schema__include_dir}/executorch/sdk/bundled_program)
116131

117-
add_custom_command(
118-
OUTPUT ${_etdump_schema__outputs}
119-
COMMAND
120-
# Note that the flatcc project actually writes its outputs into the source
121-
# tree instead of under the binary directory, and there's no way to change
122-
# that behavior.
123-
${_flatcc_source_dir}/bin/flatcc -cwr -o
124-
${_program_schema__include_dir}/executorch/sdk/etdump
125-
${_etdump_schema__srcs}
132+
if(EXECUTORCH_SEPARATE_FLATCC_HOST_PROJECT)
133+
# If we cross-compiling, we need to use the version of the commandline tool
134+
# built for the host.
135+
set(_etdump_schema_gen_dep flatcc_project)
136+
126137
# TODO(dbort): flatcc installs its files directly in its source directory
127-
# instead of under CMAKE_BINARY_DIR, and it has no options to avoid
128-
# doing this. We build flatcc twice in the executorch build: once to get
129-
# the `flatcc` host commandline tool, and once to get the (potentially
138+
# instead of under CMAKE_BINARY_DIR, and it has no options to avoid doing
139+
# this. We build flatcc twice in the executorch build: once to get the
140+
# `flatcc` host commandline tool, and once to get the (potentially
130141
# cross-compiled) target runtime library. The host build will put its outputs
131-
# in the source tree, making the cross-compiling target build think that
132-
# the outputs have already been built. It will then try to link against the
142+
# in the source tree, making the cross-compiling target build think that the
143+
# outputs have already been built. It will then try to link against the
133144
# host-architecture libraries, failing when cross-compiling. To work around
134145
# this, delete the host outputs after running this command (which only runs
135146
# when setting up the cmake files, not when actually building). This leaves
136147
# room for the target build to put its own files in the source tree. We should
137148
# try to remove this hack, ideally by submitting an upstream PR that adds an
138149
# option to change the installation location.
150+
set(_etdump_schema_cleanup_paths ${_flatcc_source_dir}/bin/*
151+
${_flatcc_source_dir}/lib/*)
152+
else()
153+
# If we're not cross-compiling we can use the plain commandline target, and we
154+
# don't need to delete any files.
155+
set(_etdump_schema_gen_dep flatcc_cli)
156+
set(_etdump_schema_cleanup_paths "")
157+
endif()
158+
159+
add_custom_command(
160+
OUTPUT ${_etdump_schema__outputs}
139161
COMMAND
140-
rm -f ${_flatcc_source_dir}/bin/*
141-
${_flatcc_source_dir}/lib/*
162+
# Note that the flatcc project actually writes its outputs into the source
163+
# tree instead of under the binary directory, and there's no way to change
164+
# that behavior.
165+
${_flatcc_source_dir}/bin/flatcc -cwr -o
166+
${_program_schema__include_dir}/executorch/sdk/etdump
167+
${_etdump_schema__srcs}
168+
COMMAND rm -f ${_etdump_schema_cleanup_paths}
142169
WORKING_DIRECTORY ${CMAKE_BINARY_DIR}/sdk
143-
DEPENDS flatcc_project
170+
DEPENDS ${_etdump_schema_gen_dep}
144171
COMMENT "Generating etdump headers"
145172
VERBATIM)
146173

‎setup.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,11 @@ def run(self):
430430
"-DEXECUTORCH_ENABLE_LOGGING=ON",
431431
"-DEXECUTORCH_LOG_LEVEL=Info",
432432
"-DCMAKE_OSX_DEPLOYMENT_TARGET=10.15",
433+
# The separate host project is only required when cross-compiling,
434+
# and it can cause build race conditions (libflatcc.a errors) when
435+
# enabled. TODO(dbort): Remove this override once this option is
436+
# managed by cmake itself.
437+
"-DEXECUTORCH_SEPARATE_FLATCC_HOST_PROJECT=OFF",
433438
]
434439

435440
build_args = [f"-j{self.parallel}"]

0 commit comments

Comments
 (0)