-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade CMake to 3.24 and refactor FetchContent usage #7794
Conversation
Looks like the first step is getting 3.24 installed on all the bots -- I'll attend to that this morning.
If you can provide guidance on this I can see about making this happen. |
add_executable(flatbuffers::flatc ALIAS flatc) | ||
|
||
add_library(flatbuffers::flatbuffers ALIAS flatbuffers) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Flatbuffers ought to provide these aliases itself. These are the names provided by the installed CMake package.
set(BUILD_TESTS OFF) | ||
set(BUILD_TOOLS OFF) | ||
set(BUILD_LIBWASM OFF) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wabt should disable building tests when it is not top-level. Ideally its settings would be prefixed with wabt_
, too, so it would be safer to set those variables directly here, rather than via project injection.
You should probably sync to main (again) to eliminate false build failures on OSX |
I realize this is still WIP, just wanted to check in and see where the plans are |
I'm going to pick this back up soon now that I'm at Adobe. I took one step in #8265 |
Should target CMake 3.28 now that it's the version on Ubuntu 24.04 LTS |
I assume this PR is defunct and superseded by other work? |
Yes. Closed and deleted. |
WIP -- still to-do is to audit our packaging rules (and maybe add some GHA tests, hmm) and update the READMEs.
This PR replaces all usage of
FetchContent
inside our build withfind_package
. When Halide is the top-level project, i.e. not being included into another project viaFetchContent
(oradd_subdirectory
), it will load the newcmake/dependencies.cmake
. This file contains a series ofFetchContent
calls that hookfind_package
. Some dependencies, likewabt
andflatbuffers
do not play nicely withFetchContent
, and so some patches must be injected. We should consider opening issues/PRs upstream to enable us to remove these hacks down the line. (Unfortunately, this is difficult for me to do).FetchContent can be disabled wholesale by setting
-DCMAKE_PROJECT_TOP_LEVEL_INCLUDES=""
on the CMake command line. This should avoid issues like #5092 resurfacing.This also makes progress toward #7611 by deferring to the including project's dependency management strategy.
Fixes #7757