Skip to content

shared: add CMake based build rules for ICU #21

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

Merged
merged 2 commits into from
May 21, 2022
Merged

Conversation

compnerd
Copy link
Member

@compnerd compnerd commented Aug 20, 2021

This adds a CMake based build for ICU. This is motivated by Windows
where we can avoid having to install a large number of tools in order to
build ICU by rewriting the build system in CMake.

This is a stopgap solution to the fact that ICU does not a CMake based
build. The project has been trying to switch to a CMake based for
nearly 4 years now [1].

This allows building ICU on Windows without having to setup and
configure cygwin to get access to autotools and without having to do
stage and coordinate the data build and the MSVC build. This also
allows the use of ninja to build which speeds up the build of ICU, as
well as enables easy cross-compilation. Additionally, this allows for a
more homogeneous build of Swift as nearly every other project builds
with CMake. There are a number of other parallel attempts to add an
out-of-tree CMake based system (e.g. [2]). This is a fairly common
problem but this solution is far more simple as it only provides enough
support to deal with the immediate needs of the Swift project. It only
supports building the bits of ICU that are needed for the Swift project
and tries to hardcode as much of the configuration as possible to reduce
the size of the ICU build while retaining all functionality required for
the Swift use cases.

[1] https://unicode-org.atlassian.net/browse/ICU-7747
[2] https://github.com/LibCMaker/ICU_CMake_Files

@compnerd
Copy link
Member Author

CC: @shahmishal

@shahmishal
Copy link
Member

Is there a reason we don't want to add this part of swift repository?

@compnerd
Copy link
Member Author

The swift repository is already overly complicated and a collection of disjoint things. We shouldn't encourage that. This is part of the build infrastructure, and I think that it makes sense to have it be part of the packaging/build scripts.

@shahmishal
Copy link
Member

cc: @gottesmm

@gottesmm
Copy link

@shahmishal I talked with compnerd about this. Part of the issue here is that currently ICU has been trying for ~4 years to get a CMake build. Given that we are technically taking on more maintenance when we update ICU if we take this, I think that is important that we make the pragmatic considerations here obvious to future maintainers. With that in mind:

  1. I think it is important to directly link to the ICU project ticket in the commit message where this work is being tracked. https://unicode-org.atlassian.net/browse/ICU-7747. As one can see, the ticket still has multiple things to work on there.
  2. I think it is important to bring up the specific benefits that this provides in the commit message. Something along the lines of will improve our build by making cross compilation easier and will allow us to use ninja to build ICU improving incremental build times.
  3. From what compnerd has told me, other projects are already doing something like this. I would like to see some links to that in the commit message as well that shows we are doing something that is standard.

So big picture, I think this is a reasonable trade-off on balance but we need to add a bunch of documentation into the commit message and probably the file as well that says /why/ we are doing this so that it is clear in the future why we did this (in case the pragmatics change [e.x.: ICU gets a real cmake build]).

@compnerd
Copy link
Member Author

@gottesmm updated the commit message with additional context

@compnerd
Copy link
Member Author

@gottesmm, @shahmishal I have now this and swiftlang/swift#58546 ... can we please get this resolved and merged?

This adds a CMake based build for ICU. This is motivated by Windows
where we can avoid having to install a large number of tools in order to
build ICU by rewriting the build system in CMake.

This is a stopgap solution to the fact that ICU does not a CMake based
build. The project has been trying to switch to a CMake based for
nearly 4 years now [1].

This allows building ICU on Windows without having to setup and
configure cygwin to get access to autotools and without having to do
stage and coordinate the data build and the MSVC build. This also
allows the use of ninja to build which speeds up the build of ICU, as
well as enables easy cross-compilation. Additionally, this allows for a
more homogeneous build of Swift as nearly every other project builds
with CMake. There are a number of other parallel attempts to add an
out-of-tree CMake based system (e.g. [2]). This is a fairly common
problem but this solution is far more simple as it only provides enough
support to deal with the immediate needs of the Swift project. It only
supports building the bits of ICU that are needed for the Swift project
and tries to hardcode as much of the configuration as possible to reduce
the size of the ICU build while retaining all functionality required for
the Swift use cases.

[1] https://unicode-org.atlassian.net/browse/ICU-7747
[2] https://github.com/LibCMaker/ICU_CMake_Files
Copy link
Member

@shahmishal shahmishal left a comment

Choose a reason for hiding this comment

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

small changes required

@shahmishal shahmishal merged commit e3950df into swiftlang:main May 21, 2022
@compnerd compnerd deleted the icu branch May 21, 2022 19:17
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.

3 participants