-
Notifications
You must be signed in to change notification settings - Fork 10.4k
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
[SourceKit] Link with libdispatch on Linux #2862
[SourceKit] Link with libdispatch on Linux #2862
Conversation
@swift-ci please test |
23cda11
to
fd913d7
Compare
@swift-ci please test |
The CI failures appear unrelated. |
@gribozavr @akyrtzi @modocache Any feedback here would be appreciated! |
@swift-ci please test |
@@ -1,4 +1,4 @@ | |||
//===--- Concurrency-Mac.cpp ----------------------------------------------===// | |||
//===--- Concurrency-libdispatch.cpp --------------------------------------===// |
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.
Excellent rename, thanks!
Looks good to me!! Thanks for the continued work on this. |
fd913d7
to
a01b11b
Compare
I've made some minor adjustments to this to now also address dispatch-related linker failures that were occurring while building the SourceKit unit tests on Linux. |
@swift-ci please test |
a01b11b
to
f00a817
Compare
sourcekitdInProc, sourcekitd-test, and complete-test need to be explicitly linked with libdispatch on Linux to build properly.
This file builds fine as long as libdispatch is available, even on non-Darwin platforms.
f00a817
to
d88bfb6
Compare
@gribozavr How about closing it? Because it has been superseded by #3594 and it has been already merged. |
👍 big thanks to @jpsim for seeing this through! |
[pull] swiftwasm from main
What's in this pull request?
Some pains have been taken within SourceKit to make
libdispatch
an optional dependency (e.g. theWorkQueue
abstraction inSourceKitSupport
, however it is neither used everywhere, nor are any implementations available besides the one that useslibdispatch
. Sincelibdispatch
is intended to be available anywhere Swift is anyway, it seems reasonable to go ahead and use it here, at least for now.This PR adds an include path to the
libdispatch
headers as well an explicit linking dependency onlibdispatch
for Linux. This has NFC in the default build configuration, since SourceKit doesn't build on Linux by default. Note that further work in the build script will be required to makelibdispatch
build before SourceKit during a standard build.This supersedes #2704
cc @akyrtzi @modocache
Resolved bug number: (SR-1677)
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.