-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[CMake] Merge Clang's features file with Swift's #41547
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
@@ -6,16 +6,18 @@ add_dependencies(swiftOption | |||
target_link_libraries(swiftOption PRIVATE | |||
swiftBasic) | |||
|
|||
set(features_file_src "${CMAKE_CURRENT_SOURCE_DIR}/features.json") | |||
set(features_file_swift_src "${CMAKE_CURRENT_SOURCE_DIR}/features.json") | |||
set(features_file_clang_src "${LLVM_MAIN_SRC_DIR}/../clang/tools/driver/features.json") |
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.
This feels very wrong to me, if there's a better way to do this please let me know 🙇
@swift-ci please test |
9f8fb78
to
79e8116
Compare
@swift-ci please test |
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.
Just one change, otherwise LGTM!
set(features_file_dest "${CMAKE_BINARY_DIR}/share/swift/features.json") | ||
|
||
add_custom_command( | ||
OUTPUT | ||
${features_file_dest} | ||
COMMAND | ||
${CMAKE_COMMAND} -E copy ${features_file_src} ${features_file_dest} | ||
${SWIFT_SOURCE_DIR}/utils/merge-features.py -f ${features_file_swift_src} -p \"\" -f ${features_file_clang_src} -p clang- ${features_file_dest} | ||
DEPENDS |
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.
You should also add a dependency on the merge-features.py
script itself.
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.
Oh nice catch, thanks!
Clang has a new `redirecting-with` property in the VFS overlay files. While this could be added to Swift's features as well, it is generally the case that features provided by Clang can also be useful to know for clients of Swift. Merge the features from Clang into Swift's features file with the "clang-" prefix to differentiate them.
79e8116
to
914b984
Compare
@swift-ci please test |
This seems to cause problems on some build configurations. The installation may attempt to install the target multiple files and using the feature merger rather than the previous |
Clang has a new
redirecting-with
property in the VFS overlay files.While this could be added to Swift's features as well, it is generally
the case that features provided by Clang can also be useful to know for
clients of Swift. Merge the features from Clang into Swift's features
file with the "clang-" prefix to differentiate them.