Skip to content

[sourcekit] Make it possible to install both sourcekitdInProc and sourcekitd efficiently #34672

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

Merged

Conversation

benlangmuir
Copy link
Contributor

This PR makes it possible to install both sourcekitd.framework and sourcekitdInProc.framework on Darwin (previously it was mutually exclusive). In order to make this configuration practical, when both are being installed, we share the implementation by having the XPC service dynamically link to the InProc framework. When only the XPC service is being installed, it continues to statically link as before. In order to enable dynamic linking, I fixed some library layering violations around notification handling and UID handling so that the "sourcekitdAPI" library does not implicitly depend on the framework that is supposed to contain it. Also fixed the runtime library path when installing sourcekitdInProc.framework, which seems to have bit-rotted.

Note: this PR does not change the default installation components, so it shouldn't change behaviour unless you manually add both "sourcekit-inproc" and "sourcekit-xpc-service" to your install components. My intention is to propose installing sourcekitInProc by default as a follow-up.

This refactors notification handling so that the XPC service and InProc
library pass in their notification handler callback. This fixes a
layering issue where the sourcekitd libraries could not be linked
dynamically due to the missing symbols.
This fixes a layering violation where UID handling was provided by a
dependent library, creating a dependency cycle if you tried to link the
lower level libraries dynamically.
Makes it possible to build and install both sourcekitdInProc and the XPC
service on Darwin.
…e when possible

When installing both InProc and XPC versions of sourcekitd, dynamically
link the XPC service to the InProc version. This lets us install both
frameworks without wasting the disk space from having another copy of
the swift frontend statically linked.
@benlangmuir benlangmuir requested a review from akyrtzi November 10, 2020 23:01
@benlangmuir
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - d4a52b8

@benlangmuir
Copy link
Contributor Author

@swift-ci please test Linux

We only need the C++ symbols on Darwin, so avoid the issue of different
manglings.
@benlangmuir
Copy link
Contributor Author

@swift-ci please test

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 16dd929

@benlangmuir
Copy link
Contributor Author

@swift-ci please clean test Linux

@benlangmuir
Copy link
Contributor Author

@swift-ci please test

@benlangmuir benlangmuir force-pushed the sourcekit-inproc-and-xpc-together branch from 4eeb381 to 16dd929 Compare November 11, 2020 19:17
@benlangmuir
Copy link
Contributor Author

@swift-ci please test

@benlangmuir benlangmuir merged commit f58c143 into swiftlang:main Nov 11, 2020
@benlangmuir benlangmuir deleted the sourcekit-inproc-and-xpc-together branch November 11, 2020 19:18
@swift-ci
Copy link
Contributor

Build failed
Swift Test OS X Platform
Git Sha - 16dd929

@swift-ci
Copy link
Contributor

Build failed
Swift Test Linux Platform
Git Sha - 16dd929

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants