[CMake] Fix status message when not building stdlib/runtime #12205
Conversation
|
@mnvr Rather than just not printing anything, can you instead print out a message saying that we are not building them? |
Fixes: SR-5817 Swift CMake prints "Building Swift standard library and SDK overlays" and "Building Swift runtime", even when those are *not* being built https://bugs.swift.org/browse/SR-5817 Test Cases: - Reproduction of Issue ("Should not Print"): export SWFT=~/Desktop/Swift mkdir /tmp/swift-build cd /tmp/swift-build cmake \ -G 'Unix Makefiles' \ -DCMAKE_BUILD_TYPE=Release \ -DSWIFT_PATH_TO_LLVM_SOURCE=$SWFT/llvm \ -DSWIFT_PATH_TO_LLVM_BUILD=$SWFT/build/llvm \ -DSWIFT_PATH_TO_CMARK_SOURCE=$SWFT/cmark \ -DSWIFT_PATH_TO_CMARK_BUILD=$SWFT/build/cmark \ -DSWIFT_BUILD_REMOTE_MIRROR=FALSE \ -DSWIFT_BUILD_DYNAMIC_STDLIB=FALSE \ -DSWIFT_BUILD_STATIC_STDLIB=FALSE \ -DSWIFT_BUILD_DYNAMIC_SDK_OVERLAY=FALSE \ -DSWIFT_BUILD_STATIC_SDK_OVERLAY=FALSE \ $SWFT/swift - Verification of Fix ("Should Print"): cmake \ -G 'Unix Makefiles' \ -DCMAKE_BUILD_TYPE=Release \ -DSWIFT_PATH_TO_LLVM_SOURCE=$SWFT/llvm \ -DSWIFT_PATH_TO_LLVM_BUILD=$SWFT/build/llvm \ -DSWIFT_PATH_TO_CMARK_SOURCE=$SWFT/cmark \ -DSWIFT_PATH_TO_CMARK_BUILD=$SWFT/build/cmark \ -DSWIFT_BUILD_REMOTE_MIRROR=FALSE \ $SWFT/swift Reported-by: Brian Ivan Gesiak
Updated. |
CMakeLists.txt
Outdated
| message(STATUS "Building Swift runtime with:") | ||
| message(STATUS " Leak Detection Checker Entrypoints: ${SWIFT_RUNTIME_ENABLE_LEAK_CHECKER}") | ||
| message(STATUS "") | ||
| message(STATUS "Building Swift standard library and SDK overlays for SDKs: ${SWIFT_SDKS}") |
There was a problem hiding this comment.
Drive-by comment: while we're here, can we fix this redundant redundancy? Either "overlays for SDKs" or "SDK overlays"; no need for "SDK overlays for SDKs".
CMakeLists.txt
Outdated
|
|
||
| else() | ||
|
|
||
| message(STATUS "Not building Swift standard library and runtime") |
There was a problem hiding this comment.
Not building Swift standard library, SDK overlays, or runtime, right?
There was a problem hiding this comment.
I'd thought about saying that when I was first writing it out, but lazed out at the last minute, thinking it would be a "duh". Apparently not.
Also, I was wondering if you folks preferred an Oxford comma or not. Luckily, your comment itself made it easy for me to figure that one out :)
There was a problem hiding this comment.
It wasn't a purposeful Oxford comma; I'm not sure that there's an answer to that :) Core team will weigh in here if they feel strongly.
|
LGTM. I think we could wordsmith some more, but I think we at least get clarity here. |
|
@swift-ci smoke test and merge |
1 similar comment
|
@swift-ci smoke test and merge |
I'm curious - which parts do you think can be improved. If I know of them, I could perhaps make those changes in some subsequent PR which touches CMakeLists.txt. |
Fixes: SR-5817
Test Cases:
These depend on the path on the particular machine, so including these
inline instead of adding them somewhere to the source tree.
Reported-by: Brian Ivan Gesiak (@modocache)