-
Notifications
You must be signed in to change notification settings - Fork 202
build: support a non-standard library prefix for Windows #1143
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
Conversation
In preparation for supporting static library linking on Windows, we are adjusting the library naming convention for static Swift libraries, preferring to use `lib<name>.lib` to differentiate them from the import library for a dynamic variant of the same library. This mechanism is already known to be used by Microsoft for ucrt (`ucrt.lib` vs `libucrt.lib`), and was previously used in Swift as well for the C++ interop libraries.
@swift-ci please test |
@@ -96,7 +96,7 @@ if(NOT BUILD_SHARED_LIBS) | |||
target_compile_options(FoundationEssentials PRIVATE | |||
"SHELL:$<$<COMPILE_LANGUAGE:Swift>:-Xfrontend -public-autolink-library -Xfrontend _FoundationCShims>") | |||
target_compile_options(FoundationEssentials PRIVATE | |||
"SHELL:$<$<COMPILE_LANGUAGE:Swift>:-Xfrontend -public-autolink-library -Xfrontend _FoundationCollections>") | |||
"SHELL:$<$<COMPILE_LANGUAGE:Swift>:-Xfrontend -public-autolink-library -Xfrontend $<$<PLATFORM_ID:Windows>:${CMAKE_STATIC_LIBRARY_PREFIX_Swift}>_FoundationCollections>") |
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.
Two quick questions about these variables:
- Where is
CMAKE_STATIC_LIBRARY_PREFIX_Swift
set? Is that a known variable by CMake, or is that set somewhere else in the build script (in which case it might need a default set here for the standalone CMake build)? - How does
PLATFORM_ID:Windows
compare to the other "windows platform" checks we have elsewhere in CMake viaCMAKE_HOST_SYSTEM_NAME STREQUAL Windows
orCMAKE_SYSTEM_NAME STREQUAL Windows
?
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.
- It is expected to be set by the user. It will eventually be defaulted to
lib
, but the current default is""
. The variable is known by CMake, (CMAKE_*
variables are meant to be CMake known variables). The current default is correct until further compiler changes are merged, but this allows testing to ensure that we cover all the cases with those changes. $<PLATFORM_ID:Windows>
is a generator expression that is basically checkingCMAKE_SYSTEM_NAME STREQUAL Windows
in concept - are you building code meant to run on Windows?CMAKE_HOST_SYSTEM_NAME STREQUAL Windows
is different in that it checks if you are building on Windows rather than building code that will run on Windows.
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.
Thanks for the explanation, makes sense! One more thing that I just realized: Should _FoundationCShims
be treated the same way? It's also a static library that's statically linked into FoundationEssentials (and into FoundationInternationalization for that matter)
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.
No, _FoundationCShims
is a C library and we aren't using a prefix there. We are using it in the Swift case to help allow us to merge the static and dynamic SDKs.
@swift-ci please test macOS platform |
1 similar comment
@swift-ci please test macOS platform |
macOS CI is known to not be working right now and isn't a required check for merging (@shahmishal and team are working on upgrading the OS run by CI to fix that) so no need to test macOS. Also none of the CI on this repo will test the CMake build (yet - @shahmishal and team also working on that), if we want to truly test the CMake build the way to do that for right now is a cross-repo CI run from the swift repo which can build a toolchain / run full toolchain tests |
Ah, okay, in that case, we should be good to go I think. The actual test will come later, right now this is just staging for additional testing and experimentation. |
In preparation for supporting static library linking on Windows, we are adjusting the library naming convention for static Swift libraries, preferring to use
lib<name>.lib
to differentiate them from the import library for a dynamic variant of the same library. This mechanism is already known to be used by Microsoft for ucrt (ucrt.lib
vslibucrt.lib
), and was previously used in Swift as well for the C++ interop libraries.