-
Notifications
You must be signed in to change notification settings - Fork 724
Description
🐛 Describe the bug
The include paths in executorch look like #include <executorch/runtime/executor/method.h>. This means that the .h file needs to live under a subpath named exactly .../executorch/runtime/executor/.
The headers in the repo live in locations like <repo-root>/runtime/executor/method.h. To make the executorch/... include paths work, the build system currently assumes/requires that the repo lives in a directory named exactly executorch, and adds the include path <repo-root>/..:
Lines 326 to 327 in cb25809
| # Let files say "include <executorch/path/to/header.h>". | |
| set(_common_include_directories ${CMAKE_CURRENT_SOURCE_DIR}/..) |
But if the repo does not live in a directory named exactly executorch, this causes the includes to fail. Or, worse, leads to subtle breakages like #6385 when there is a ../executorch directory, but it's not the directory we're building.
Versions
This issue exists in v0.4.0
Solution
The traditional solution for this is to have includes directories under the repo root that become part of the include path when depending on that library/component. E.g., <repo-root>/runtime/executor/includes/executorch/runtime/executor/method.h.
Another solution would be to build a symlink farm, constructing a tree under cmake-out that points to the underlying headers. But this doesn't work on systems without symlinks, like Windows; in those cases, we'd need extra build logic to copy the headers into a tree, and to re-copy them when the contents of the headers change.
Mitigation
Since we depend on the name of the repo, and can't get far if the name is wrong, the build system should assert that the directory is named exactly executorch. This would help save time for users, and could point to this task to track the real fix.