-
Notifications
You must be signed in to change notification settings - Fork 225
Migrate the Makefile to CMake
#685
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
base: main
Are you sure you want to change the base?
Conversation
This commit migrates wasi-libc's build system from a `make`-based system to CMake. This is a complete rewrite of the build system which culminates in the deletion of the current `Makefile` and a few supporting scripts and files. The rationale/reasons for this are similar to WebAssembly/wasi-sdk#429, namely: * Building a correct and robust build system in `make` is not easy. There are many times I've found myself in a situation where I need to blow away the entire build directory between builds. Much of the this this bottoms out in subtle behavior like "this file was renamed, but didn't get deleted in the archive" or subtle things like that. CMake is responsible for handling these by default and, in general, is probably going to be more correct than what we write. * Out-of-tree builds are now supported. * Customizing CFLAGS is now supported via standard mechanisms. Previously `EXTRA_CFLAGS` was required since using `CFLAGS` could break the build. * It's easier to move more logic into CMake, such as downloading compiler-rt, than it is to codify it all in makefiles. * Platform portability is generally easier in CMake than make. Building on Windows shouldn't require a full GNU-like environment, for example. * Tests now properly rebuild themselves when wasi-libc changes. * It's easier to customize high-level options, like "enable SIMD", in CMake than it is in Makefiles. This can be documented as a single option to pass where that option affects the build, flags, etc. Personally I'm not a fan of CMake, but I'm more of a fan of it than Makefiles, hence my desire to switch. I want to make this repository easier to build, configure, and change over time. This will also make it easier to integrate this all into wasi-sdk where everything is CMake-based over there as well. I am not a CMake expert, nor am I necessarily an expert in the previous Makefiles. I've done my best here, but I'm happy to change things if someone who knows more about CMake than I (which is a lot of folks) recommends doing so. I'm also happy to adjust the libc build as desired too. Closes WebAssembly#46 Closes WebAssembly#156 Closes WebAssembly#295 Closes WebAssembly#322 Closes WebAssembly#330 Closes WebAssembly#514 Closes WebAssembly#605
|
To verify this change I've built the sysroot with the old makefile and this PR. The order of files in This is expected in that I removed some wasip2 bits from the wasip1 archives/sysroot (e.g. the header and wasip2-only objects). The Basically I've double-checked that, with the right compile flags, the set of objects produced is the same and they all have the exact same contents as well. Artifacts will differ slightly as items are reordered, however. I'll also note that CMake's release build uses |
sbc100
left a comment
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.
A very impressive feat!
I can't say I reviewed every line but if the tests pass the lgtm
| # | ||
| include(GNUInstallDirs) | ||
| install(DIRECTORY ${SYSROOT}/include/ DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}) | ||
| install(DIRECTORY ${SYSROOT}/lib/ DESTINATION ${CMAKE_INSTALL_LIBDIR}) |
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.
Does cmake always make sure it builds all the libs in ${SYSROOT}/lib/ before running this install step? i.e. does it always to a make all before running any install steps?
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.
I believe so yeah, the install target in the generated build system has a dependency on the all target which ensures that the sysroot is filled in
| #define __wasilibc___typedef_time_t_h | ||
| #define __wasilibc___typedef_uid_t_h | ||
| #define __wasilibc_use_wasip2 1 | ||
| #define __wasilibc_use_wasip2 |
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.
Do you know why these expected files change?
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.
This was due to this change, notably replacing an empty file with a CMake-generated *.h.in file (which is configured during the cmake configuration process)
| @@ -0,0 +1,3 @@ | |||
| add_internal_library_pair(wasi-emulated-getpid getpid.c) | |||
| sysroot_lib(wasi-emulated-getpid-static libwasi-emulated-getpid.a) | |||
| sysroot_lib(wasi-emulated-getpid libwasi-emulated-getpid.so) | |||
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.
Is it worth having all these small CMakeLists.txt? Does cmake dictate that each libray have its own file?
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.
I went back-and-forth on this myself having originally written this as one monolithic CMakeLists.txt and then refactoring to this. My impression was that idiomatic CMake has a CMakeLists.txt roughly per-directory containing the build logic for that directory itself which is what I tried to mirror here.
This folder in particular is a bit extreme since it's just a single C file going into a library, but the libc-bottom-half/signal/CMakeLists.txt file was more substantial for example. Overall I personally felt that the per-directory build felt better than a single monolithic configuration file if only to cut down the size of all the file paths mentioned
This commit migrates wasi-libc's build system from a
make-based system to CMake. This is a complete rewrite of the build system which culminates in the deletion of the currentMakefileand a few supporting scripts and files.The rationale/reasons for this are similar to WebAssembly/wasi-sdk#429, namely:
Building a correct and robust build system in
makeis not easy. There are many times I've found myself in a situation where I need to blow away the entire build directory between builds. Much of the this this bottoms out in subtle behavior like "this file was renamed, but didn't get deleted in the archive" or subtle things like that. CMake is responsible for handling these by default and, in general, is probably going to be more correct than what we write.Out-of-tree builds are now supported.
Customizing CFLAGS is now supported via standard mechanisms. Previously
EXTRA_CFLAGSwas required since usingCFLAGScould break the build.It's easier to move more logic into CMake, such as downloading compiler-rt, than it is to codify it all in makefiles.
Platform portability is generally easier in CMake than make. Building on Windows shouldn't require a full GNU-like environment, for example.
Tests now properly rebuild themselves when wasi-libc changes.
It's easier to customize high-level options, like "enable SIMD", in CMake than it is in Makefiles. This can be documented as a single option to pass where that option affects the build, flags, etc.
Personally I'm not a fan of CMake, but I'm more of a fan of it than Makefiles, hence my desire to switch. I want to make this repository easier to build, configure, and change over time. This will also make it easier to integrate this all into wasi-sdk where everything is CMake-based over there as well.
I am not a CMake expert, nor am I necessarily an expert in the previous Makefiles. I've done my best here, but I'm happy to change things if someone who knows more about CMake than I (which is a lot of folks) recommends doing so. I'm also happy to adjust the libc build as desired too.
Closes #46
Closes #156
Closes #259
Closes #322
Closes #330
Closes #514
Closes #551
Closes #605