Skip to content
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

Rewrite the build system with CMake #429

Merged
merged 13 commits into from
Jul 11, 2024
Merged

Conversation

alexcrichton
Copy link
Collaborator

This commit is an attempt to provide a concrete path forward on #425. I personally think it's pretty important to get the ability to have more architectures here but at the same time I also think it's important to to take this as an opportunity to refactor and improve the build system of this repository. To that end this represents my attempt to improve the status quo.

This removes the old Makefile and replaces it with a CMake-based system to build all these projects. Overall this is intended to be a "no functional change" intended sort of refactoring. Changing build systems inevitably causes issues, however, so this change additionally has a very high likelihood of needing follow-up fixes. At a high enough level this commit introduces two major changes to how this repository is built:

  1. The make-based system (the root Makefile) is replaced with CMake. This additionally updates tests to use CMake.
  2. A single "build" is split into either building a toolchain or building a sysroot. This enables builds to only build one or the other as necessary.

The first change, using CMake, is due to the fact that using make on Windows basically is not pleasant coupled with the fact that more advanced logic, such as changing flags, compilers, etc, is much easier with a CMake-based system. The second change is intended to cover the use case of #425 in addition to refactoring the current build.

Throughout this change I have intentionally not tried to keep a 1:1 correspondance with behaviors in the old Makefile because much of this PR is intended to address shortcomings in the old build system. A list of changes, improvements, etc, made here are:

  • CMake provides a much nicer portability story to Windows than make. This is moving towards the direction of not needing bash, for example, to build an SDK. Currently wasi-libc still requires this, but that's now the only "hard" dependency.

  • The set of targets built can now be configured for smaller builds and/or debugging just a single target. All WASI targets are still built by default but it's much easier to add/remove them.

  • Different targets are now able to be built in parallel as opposed to the unconditional serial-nature of the Makefile.

  • Use of ninja is no longer required and separate build systems can be used if desired.

  • The sysroot and the toolchain can now be built with different CMake build profiles. For example the Makefile hardcoded MinSizeRel and RelWithDebInfo and this can now be much more easily customized by the SDK builder.

  • Tarballs are now more consistently produced and named. For a tarball of the name foo.tar.gz it's guaranteed that there's a single folder foo created when unpacking the tarball.

  • The macOS binaries are no longer hybrid x64/arm64 binaries which greatly inflates the size of the SDK. There's now a separate build for each architecture.

  • CI now produces arm64-linux binaries. The sysroot is not built on the arm64-linux builder and the sysroot from the x86_64-linux builder is used instead.

  • Tests are almost ready to execute on Windows, there's just a few minor issues related to exit statuses and probably line endings which need to be worked out. Will require someone with a Windows checkout, however.

  • Tests are now integrated into CMake. This means that the wasm binaries are able to be built in parallel and the tests are additionally executed in parallel with ctest. It is possible to build/run a single test. Tests no longer place all of their output in the source tree.

  • Out-of-tree builds are now possible and the build/installation directories can both be customized.

  • CI configuration of Windows/macOS/Linux is much more uniform by having everything in one build matrix instead of separate matrices.

  • Linux builds are exclusively done in docker containers in CI now. CI no longer produces two Linux builds only for one to be discarded when artifacts are published.

  • Windows 32-bit builds are no longer produced in CI since it's expected that everyone actually wants the 64-bit ones instead.

  • Use of ccache is now automatically enabled if it's detected on the system.

  • Many preexisting shell scripts are now translated to CMake one way or another.

  • There's no longer a separate build script for how to build wasi-sdk in docker and outside of docker which needs to be kept in sync, everything funnels through the same script.

  • The docker/Dockerfile build of wasi-sdk now uses the actual toolchain built from CI and additionally doesn't duplicate various CMake-based configuration files.

Overall one thing I want to additionally point out is that I'm not CMake expert. I suspect there's lots of little stylistic and such improvements that can be made.

@alexcrichton
Copy link
Collaborator Author

A few other points of note:

  • To land this PR the set of required checks in this repository's settings are going to have to change (I don't have access to change that)
  • I haven't updated the instructions for making a release yet. If there's agreement on this I'd like to land this and then I'll work through any changes there necessary.
  • I've tried to leave comments in all the various places, but if anything is surprising and/or missing comments let me know and I'll add more.

wasi-sdk.control Outdated Show resolved Hide resolved
Copy link
Member

@sbc100 sbc100 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not the greatest fan of cmake I have to say. I tend to find shell scripts that are transliterated into cmake even harder to deal with than the originals.

However, you seem to have done all the due diligence here and cmake sounds like it is the best worst option. I wish we had some other/nicer option here but I'm not sure we do.

Thanks for all the efforts here BTW, I know this kind of work can be real pain.

cmake/Platform/WASI.cmake Outdated Show resolved Hide resolved
target_compile_options(${target_name} PRIVATE -D_WASI_EMULATED_MMAN)
target_link_options(${target_name} PRIVATE -lwasi-emulated-mman)
elseif(test MATCHES "(sigabrt|signals).c$")
target_compile_options(${target_name} PRIVATE -D_WASI_EMULATED_SIGNAL)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we not keep the existing .options file thing that reads the options from a file alongside the test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried yeah but I couldn't figure out a way to easily read options from a file at test-compile-time while also rebuilding the test if the file appeared, disappeared, or got modified. This was the least-bad alternative that I could come up with

tests/testcase.sh Show resolved Hide resolved
@@ -1,7 +1,7 @@
# Cmake toolchain description file for the Makefile

# This is arbitrary, AFAIK, for now.
cmake_minimum_required(VERSION 3.4.0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this file (along with the Platform file) now being used as part of the build? Or maybe its being used to build the examples?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct yeah, this was done since CMake was spewing a warning-per-compile (aka lots) that CMake itself is dropping support for 3.4.0 so I bumped this to 3.5.0 to stop CMake from warning.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, once #430 is merged, we'll want to make the same change to wasi-sdk-p2.cmake

@alexcrichton
Copy link
Collaborator Author

I'm not the greatest fan of cmake I have to say. I tend to find shell scripts that are transliterated into cmake even harder to deal with than the originals.

To be clear I completely agree with you. I'm no fan of cmake myself, but AFAIK it's the only build-system-style solution that works across Windows/macOS/Linux. Shell scripts to me chiefly break down on Windows. I also think that cmake has a lot to offer once you get past the "wow that sure is cmake" since it has functions/if-else/loops/etc in addition to being able to define a build system which ends up being pretty flexibly. I definitely agree though that the syntax leaves a bit to be desired.

Copy link
Collaborator

@abrown abrown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like @sbc100, I have been a bit skeptical about switching to CMake in the past. But more recently I am more convinced: it takes on the cross-platform portability burden and it's heavily used out there in the ecosystem (not to mention in LLVM itself). Thanks @alexcrichton for working on this--it's the initial push that was needed to do the switch. I'll let you and @sbc100 figure out any final details but I'm a +1 on this. (I saw up above that some CI settings need to change? Let me know what they are and I'll tweak those.)

@TerrorJack
Copy link
Contributor

It would be very nice to make wasm-component-ld building logic optional, since downstream users may have other ways to provide the rust based tools and want to avoid cargo install during building of wasi-sdk.

@alexcrichton
Copy link
Collaborator Author

I'm not necessarily in any rush to merge this, but I also want to pose a question as to what the next steps for this would be. For example do folks feel this should stay open for awhile to gather more feedback? Or is there anyone in particular I should reach out to and consult on this?

@TerrorJack happy to add other build options, should be pretty easy to do so. That being said I'd like to delve further into the motivations because I think it's best to have a consistent sysroot where possible (e.g. omitting wasm-component-ld would mean that the wasm32-wasip2 target wouldn't work), so mind opening an issue to discuss a bit further?

@TerrorJack
Copy link
Contributor

@alexcrichton Nope, just assuming wasm-component-ld is already built elsewhere and present in PATH. That shouldn't affect p2 sysroots.

@alexcrichton
Copy link
Collaborator Author

Ok I'd still regardless like to discuss further (but probably not here since it's orthogonal). For example I'd like to understand why cargo install is inappropriate because if it's externally available then the versions could be out of sync and that can confuse what it means to have a particular version of wasi-sdk. (I'm trying to explain some questions here rather than simply say "let's discuss in an issue" but I'd prefer to follow-up in an issue)

@glandium
Copy link
Contributor

Does this build system allow not to build clang/llvm before building the sdk?

@alexcrichton
Copy link
Collaborator Author

Yes, it's possible to build just the sysroot since the build is split in two halves. One caveat is that due to how compiler-rt library lookup works the build system tries to install compiler-rt in the in-use toolchain's sysroot, so that will probably need tweaks over time.

@alexcrichton
Copy link
Collaborator Author

I figured I'll solicit once more for thoughts on this, and pending that I'll try to work with @abrown later this week with respect to repo settings to merge.

This commit is an attempt to provide a concrete path forward on
WebAssembly#425. I personally think it's pretty important to
get the ability to have more architectures here but at the same time I
also think it's important to to take this as an opportunity to refactor
and improve the build system of this repository. To that end this
represents my attempt to improve the status quo.

This removes the old `Makefile` and replaces it with a CMake-based
system to build all these projects. Overall this is intended to be a "no
functional change" intended sort of refactoring. Changing build systems
inevitably causes issues, however, so this change additionally has a
very high likelihood of needing follow-up fixes. At a high enough level
this commit introduces two major changes to how this repository is
built:

1. The `make`-based system (the root `Makefile`) is replaced with CMake.
   This additionally updates tests to use CMake.
2. A single "build" is split into either building a toolchain or
   building a sysroot. This enables builds to only build one or the
   other as necessary.

The first change, using CMake, is due to the fact that using `make` on
Windows basically is not pleasant coupled with the fact that more
advanced logic, such as changing flags, compilers, etc, is much easier
with a CMake-based system. The second change is intended to cover the
use case of WebAssembly#425 in addition to refactoring the current build.

Throughout this change I have intentionally not tried to keep a 1:1
correspondance with behaviors in the old `Makefile` because much of this
PR is intended to address shortcomings in the old build system. A list
of changes, improvements, etc, made here are:

* CMake provides a much nicer portability story to Windows than `make`.
  This is moving towards the direction of not needing `bash`, for
  example, to build an SDK. Currently `wasi-libc` still requires this,
  but that's now the only "hard" dependency.

* The set of targets built can now be configured for smaller builds
  and/or debugging just a single target. All WASI targets are still
  built by default but it's much easier to add/remove them.

* Different targets are now able to be built in parallel as opposed to
  the unconditional serial-nature of the `Makefile`.

* Use of `ninja` is no longer required and separate build systems can be
  used if desired.

* The sysroot and the toolchain can now be built with different CMake
  build profiles. For example the `Makefile` hardcoded `MinSizeRel` and
  `RelWithDebInfo` and this can now be much more easily customized by
  the SDK builder.

* Tarballs are now more consistently produced and named. For a tarball
  of the name `foo.tar.gz` it's guaranteed that there's a single folder
  `foo` created when unpacking the tarball.

* The macOS binaries are no longer hybrid x64/arm64 binaries which
  greatly inflates the size of the SDK. There's now a separate build for
  each architecture.

* CI now produces arm64-linux binaries. The sysroot is not built on the
  arm64-linux builder and the sysroot from the x86_64-linux builder is
  used instead.

* Tests are almost ready to execute on Windows, there's just a few minor
  issues related to exit statuses and probably line endings which need
  to be worked out. Will require someone with a Windows checkout, however.

* Tests are now integrated into CMake. This means that the wasm binaries
  are able to be built in parallel and the tests are additionally
  executed in parallel with `ctest`. It is possible to build/run a
  single test. Tests no longer place all of their output in the source
  tree.

* Out-of-tree builds are now possible and the build/installation
  directories can both be customized.

* CI configuration of Windows/macOS/Linux is much more uniform by having
  everything in one build matrix instead of separate matrices.

* Linux builds are exclusively done in docker containers in CI now. CI
  no longer produces two Linux builds only for one to be discarded when
  artifacts are published.

* Windows 32-bit builds are no longer produced in CI since it's expected
  that everyone actually wants the 64-bit ones instead.

* Use of `ccache` is now automatically enabled if it's detected on the
  system.

* Many preexisting shell scripts are now translated to CMake one way or
  another.

* There's no longer a separate build script for how to build wasi-sdk in
  docker and outside of docker which needs to be kept in sync,
  everything funnels through the same script.

* The `docker/Dockerfile` build of wasi-sdk now uses the actual
  toolchain built from CI and additionally doesn't duplicate various
  CMake-based configuration files.

Overall one thing I want to additionally point out is that I'm not CMake
expert. I suspect there's lots of little stylistic and such improvements
that can be made.
@alexcrichton
Copy link
Collaborator Author

@abrown has now removed the gates on specific status checks which enables this PR to merge. I'll wait for green CI and then merge it and then we'll re-enable the gates on the new status checks that this PR introduces.

@abrown abrown merged commit e29a3fe into WebAssembly:main Jul 11, 2024
6 checks passed
@alexcrichton alexcrichton deleted the cmake branch July 12, 2024 14:14
alexcrichton added a commit to alexcrichton/wasi-sdk that referenced this pull request Jul 12, 2024
abrown pushed a commit that referenced this pull request Jul 12, 2024
Updating for the rewrite in #429
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants