[SR-1613][SourceKit] Include libdispatch #2704
Conversation
| @@ -496,6 +496,7 @@ if("${CMAKE_SYSTEM_NAME}" STREQUAL "Darwin") | |||
| endif() | |||
|
|
|||
| include_directories(BEFORE | |||
| ${SWIFT_SOURCE_DIR}/../swift-corelibs-libdispatch | |||
There was a problem hiding this comment.
We should not be doing this on Darwin.
Several parts of SourceKit make use of libdispatch. Include its headers in order to allow those parts to build.
modocache
force-pushed
the
sr-710-sourcekit-linux-libdispatch
branch
from
028d7db to
f01ee69
Compare
|
@gribozavr Thanks! I addressed your comment by only including libdispatch on Linux. @swift-ci please test |
|
@gribozavr @akyrtzi What do you think of this one? Like #2703, this fixes one of the initial compilation errors when building SourceKit on Linux. Any objections to me merging it? |
| @@ -500,5 +500,10 @@ include_directories(BEFORE | |||
| ${SOURCEKIT_SOURCE_DIR}/include | |||
| ) | |||
|
|
|||
| if("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux") | |||
There was a problem hiding this comment.
Pardon my CMake ignorance, but is there a way to phrase this such that it specifies non-Darwin instead? I presume that future Unix-y target platforms will also be in need of these headers.
There was a problem hiding this comment.
Absolutely. I'm worried about SR-710 getting done in time for Swift 3.0, so I'm reducing scope a bit here. Ideally, I'd like SourceKit to run everywhere. Were it able to do so, we'd probably want to change this to handle more than just Linux.
|
Even with the headers resolved, aren't we going to fail to link because |
|
Corelibs libdispatch does include a Swift overlay which is currently enabled in the build script. |
|
@briancroom Sorry, I deleted my comment before noticing your reply. Yeah, to build libdispatch first, we have to configure it without |
|
All good points! Yeah, this pull request won't get SourceKit building properly -- I just sent it out to resolve the headers. Traditionally, I've noticed the core team is not thrilled with the idea of requiring multiple invocations of the build script... we should create a separate task on JIRA to discuss potential approaches here. |
|
I have opened #2862 which I believe supersedes this PR. |
|
Sounds good, @briancroom! I'll close this in favor of your pull request. What about #2769? Is that also taken care of in #2862? |
|
No, the changes in #2769 are addressing a different set of linker failures. |
What's in this pull request?
Several parts of SourceKit make use of libdispatch. Include its headers in order to allow those parts to build.
Of course, SourceKit isn't actually built on Linux yet. You may attempt to build it by applying this patch.
ResolvedRelated bug number: (SR-710)Before merging this pull request to apple/swift repository:
Triggering Swift CI
The swift-ci is triggered by writing a comment on this PR addressed to the GitHub user @swift-ci. Different tests will run depending on the specific comment that you use. The currently available comments are:
Smoke Testing
Validation Testing
Lint Testing
Note: Only members of the Apple organization can trigger swift-ci.