build: add CMake based build system for XCTest #214
Conversation
|
CC: @modocache |
|
@aciidb0mb3r - I realized too late that you had taken a stab at this earlier. This attempt is inspired by the work that I did for apple/swift, apple/swift-corelibs-libdispatch, and apple/swift-corelibs-foundation. It is very similar to those builds but is lacking support for building and running the test suite still. |
|
Ah, I had done this for llbuild but yeah no support for tests: https://github.com/apple/swift-llbuild/blob/master/cmake/modules/Utility.cmake#L120 I am going to work on makefile support in SwiftPM to allow building and testing all related project with swiftpm but I don't know when that'll be done. Here is some more info on this: |
|
Okay, the newest version has been tested against s-c-f and passes tests as well. It has testing support too via the |
There was a problem hiding this comment.
Looks good to me! I only had questions for my own edification, no requests for any changes. I'll leave this to @briancroom to accept, just to make sure he has a chance to comment.
|
|
||
| find_package(LLVM CONFIG) | ||
| if(NOT LLVM_FOUND) | ||
| message(SEND_ERROR "Could not find LLVM; configure with -DCMAKE_PREFIX_PATH=/path/to/llvm/install") |
There was a problem hiding this comment.
Question for you: my understanding is that there's also a method of configuration in which the llvm-config executable is invoked, is that correct? But I believe I heard that means of configuration is deprecated -- I'm not sure. Do you know the current situation is?
To be clear, I'm not asking you to change anything here, this looks right to me, I'm just wondering about llvm-config.
There was a problem hiding this comment.
This is the more modern, CMake way to handle this. LLVM will install a CMake module that can be used. If you use the llvm-config invocation approach, that requires the user to specify the path to the installation. If you install LLVM to the system normally, this should JustWork(TM) so it is nicer.
| @@ -0,0 +1,122 @@ | |||
|
|
|||
There was a problem hiding this comment.
Tiny nit-pick/question: is there a reason you're leaving an empty line at the top of the CMake files? Same goes for the cmake/modules/SwiftSupport.cmake file below.
There was a problem hiding this comment.
Yeah, ideally, someone would do the hard work of figuring out how to add the authorship information to the CMake content. I've been lazy and this is something that should be done at some point - it is not for any functional reason, only so that there is uniformity in all the files across the repositories.
| Sources/XCTest/Public/Asynchronous/XCWaitCompletionHandler.swift | ||
| Sources/XCTest/Public/Asynchronous/XCNotificationExpectationHandler.swift | ||
| Sources/XCTest/Public/Asynchronous/XCTestCase+Asynchronous.swift | ||
| Sources/XCTest/Public/Asynchronous/XCTestExpectation.swift |
There was a problem hiding this comment.
(I vaguely remembered that globbing source files in CMake was discouraged, so for any other reviewers out there, I looked up the exact reason: https://stackoverflow.com/a/32412044/679254)
|
|
||
| include(CMakeParseArguments) | ||
|
|
||
| function(add_swift_target target) |
There was a problem hiding this comment.
This seems like a handy, generic function. Are you anticipating moving it somewhere where all the swift-corelibs-* projects could use it, maybe someday?
There was a problem hiding this comment.
swift-tools-support-core repository is not ready yet but it can be used to hold the shared infrastructure once its open for business!
There was a problem hiding this comment.
@modocache - you are right, this is a handy, generic function. I use the same basic functionality in swift-corelibs-libdispatch and am using it in swift-corelibs-foundation too. Once we have the shared repository, moving this would be great.
|
@briancroom - ping! |
|
@compnerd hey, thanks for the ping! I'm sorry I haven't had a chance to take a proper look here. I'm pretty swamped with WWDC responsibilities right now still but will really try to make some time in the next day or two for this. Thank you for your patience, and for the PR! |
|
@briancroom - ping! |
There was a problem hiding this comment.
Alright, sorry for the delay.
I've looked through this and it looks fine to me, though take that with a grain of salt because my CMake knowledge is very rudimentary! With @modocache and @aciidb0mb3r having looked as well though, I'm not concerned.
For reference, could you provide some context on what you're trying to accomplish with this, now and in the future? Would you like to see the Swift build-script switch over to calling into this, and deprecate the existing Python build script that we have here?
|
Sure. Yes, the desire is to deprecate the python build script infrastructure in favor of this. CMake would also be able to generate an Xcode project if you so desired. This will create a single unified build system that can target all the platforms. It also makes it easier to cross-compile XCTest (for say Windows). I think that switching over the build-script to CMake would be the ideal thing, but that would be a secondary change as we need to ensure that XCTest is ready to be built with CMake first. It also aids in reducing some of the cognitive overload with different build systems for the different pieces of swift. |
|
Sounds good, thanks! |
No description provided.