[CMake] Update defines to indicate combined, dynamic module build #28
Conversation
| $<$<COMPILE_LANGUAGE:C,CXX>:U_SHOW_CPLUSPLUS_API=1> | ||
| $<$<COMPILE_LANGUAGE:C,CXX>:U_SHOW_INTERNAL_API=1> | ||
| $<$<COMPILE_LANGUAGE:C,CXX>:U_STATIC_IMPLEMENTATION> |
There was a problem hiding this comment.
This has broader implications on Windows. If this module is now being built dynamically, it needs to be explicitly packaged (see https://github.com/apple/swift-installer-scripts).
Is there a reason to build this dynamically now rather than statically? The only consumer of this library is Foundation right?
There was a problem hiding this comment.
Both CoreFoundation (which is a static library that is statically linked into the Foundation dynamic library) and FoundationInternationalization (a dynamic library) link this _FoundationICU module, so rather than statically linking ICU into both we took the approach of having them both link the same dynamic library. Would FoundationEssentials and FoundationInternationalization (which are new dynamic libraries that we are installing) also be in the same boat here?
There was a problem hiding this comment.
I took a look at that repo, looks like we'll need to update the installer scripts to account for the new _FoundationICU.dll, FoundationEssentials.dll, and FoundationInternationalization.dll libraries along with the new FoundationEssentials.swiftmodule and FoundationInternationalization.swiftmodule. That should be doable, I can post a PR to update it once we get far enough along in our toolchain builds to test it. Since we have to update it for FoundationEssentials and FoundationInternationalization anyways I don't think adding _FoundationICU to that list adds any additional burden - are there any other implications that we need to make sure we address here? (looks like we need to add the repos to the lists in the linux files too)
|
Confirmed that SwiftPM builds of swift-foundation with this change build/pass tests on both Darwin and Windows. Linux and Windows toolchain builds are ongoing for validating the CMake build |
|
Linux toolchain build has passed and Windows toolchain build did not encounter any errors not seen previously |
|
I'm going to merge this change as is to keep the ball rolling here, will followup with the installer script concerns separately |
While this didn't cause issues on Linux/Darwin, the Windows builds fails due to incorrect defines specified. Since we are building all pieces as one module, we need to set
U_COMBINED_IMPLEMENTATIONalong with the otherU_X_IMPLEMENTATIONdefines for all source files. Doing this also revealed that settingU_ATTRIBUTE_DEPRECATEDto an empty string causes build failures so I've removed that define.