[SourceKit] don't install libdispatch and libBlocksRuntime twice, outside of Mac/Windows

[finagolfin]

Added in #19800, but unused on linux and Android, as their rpath uses the libdispatch.so in lib/swift/os. I don't know what happens on a mac, as I don't know what @rpath does, so I left it in for macOS.

I've had to hack around this install before, but now I just patch it out for the Android package of Swift. Better to just not install it twice for platforms where it's not used, which may include macOS for all I know.

[finagolfin]

@benlangmuir, I think you added the first libdispatch install in #19921, let me know what you think.

[benlangmuir]

I did not add the install step, I fixed a missing dependency on it that was causing build issues. https://bugs.swift.org/browse/SR-8927 Maybe @compnerd knows better the intent.

[finagolfin]

Btw @benlangmuir, I recently noticed that libsourcekitdInProc.so exports a ton of LLVM symbols on linux, after it caused a problem for me on Android, intentional or bug? Here is what I mean, from the official 5.1.4 release for linux:

> readelf -sW swift-5.1.4-RELEASE-ubuntu18.04/usr/lib/libsourcekitdInProc.so | ag "GLOBAL DEFAULT.*_ZN4llvm" | tail -10                                                                96737: 00000000009b9b90   569 FUNC    GLOBAL DEFAULT   11 _ZN4llvm11raw_ostream5writeEPKcm
 96738: 00000000009b9280   204 FUNC    GLOBAL DEFAULT   11 _ZN4llvm11raw_ostream16SetBufferAndModeEPcmNS0_10BufferKindE
 96739: 00000000009b9aa0    58 FUNC    GLOBAL DEFAULT   11 _ZN4llvm11raw_ostream14flush_nonemptyEv
 96957: 00000000009bbd30    57 FUNC    GLOBAL DEFAULT   11 _ZN4llvm3sys9MutexImplD1Ev
 96958: 00000000009bbc30   254 FUNC    GLOBAL DEFAULT   11 _ZN4llvm3sys9MutexImplC1Eb
 96959: 00000000009bbdb0    52 FUNC    GLOBAL DEFAULT   11 _ZN4llvm3sys9MutexImpl7releaseEv
 96960: 00000000009bbd70    52 FUNC    GLOBAL DEFAULT   11 _ZN4llvm3sys9MutexImpl7acquireEv
 96961: 00000000009bd400    47 FUNC    GLOBAL DEFAULT   11 _ZN4llvm3sys4path11parent_pathENS_9StringRefENS1_5StyleE
 96962: 00000000009a1890   518 FUNC    GLOBAL DEFAULT   11 _ZN4llvm25llvm_unreachable_internalEPKcS1_j
 96963: 0000000005135c1c     4 OBJECT  GLOBAL DEFAULT   25 _ZN4llvm23EnableABIBreakingChecksE
> readelf -sW swift-5.1.4-RELEASE-ubuntu18.04/usr/lib/libsourcekitdInProc.so | ag "GLOBAL DEFAULT.*_ZN4llvm" | wc -l
15710

[finagolfin]

@tachoknight, I just checked and saw that you copy both these and the libraries in lib/swift/linux over, any reason why?

@drodriguez, any feedback on this pull or the symbols I mentioned in the previous comment would be appreciated.

[tachoknight]

@buttaface Is this in reference to requiring libBlocksRuntime for the build? A lot of what drives what I do is based on the user being able to install swift-lang alongside an existing clang/llvm installation, as well as a totally separate libblocksruntime that can be used independently of the Swift version. And because of that, at any time the user can decide to remove the existing clang/llvm and/or libblocksruntime installation and that should not break Swift.

Hopefully that answers the question regarding libBlocksRuntime; I don't package libDispatch separately so that does need to be included in the swift-lang package. I'd ask that, if you don't want libDispatch in the Android version, you make it more explicit because I think this PR will break Ubuntu and Fedora.

[benlangmuir]

I recently noticed that libsourcekitdInProc.so exports a ton of LLVM symbols on linux, after it caused a problem for me on Android, intentional or bug?

@buttaface we have an explicit exports list that only includes the sourcekitd-specific symbols, but it's only being applied on Darwin: https://github.com/apple/swift/blob/b58ea4bb377cd9978f9b056494c266db70faa80b/tools/SourceKit/cmake/modules/AddSwiftSourceKit.cmake#L2-L5

I don't know what the equivalent of -exported_symbols_list linker option is for Windows or Linux

[finagolfin]

@tachoknight,

Is this in reference to requiring libBlocksRuntime for the build?

No, I didn't even look at those.

Hopefully that answers the question regarding libBlocksRuntime

No, I don't think you looked at the links in my original post: let me state the problem briefly. It always struck me as strange that the official Swift tarfile release for linux installs libdispatch.so and libBlocksRuntime.so in two places, both in usr/lib and usr/lib/swift/linux. Like you, that caused a problem for me because of a potential clash with a libdispatch package, and I had to remove the ones in usr/lib from the swift Android package.

I haven't unpacked your final Fedora package to check, but based on your build script, my understanding is that you place one copy of these two libraries in usr/lib/swift-lldb and another in usr/lib/swift/linux. My reading of the rpaths is that the former is unused and all this pull does is remove them, so that only the latter two are used.

Let me know if you actually need the former.

@benlangmuir, thanks, I figured it was an oversight but wasn't sure.

[tachoknight]

@buttaface Ah, sorry, yeah, I did look at the links and verified it against my installed copy. You're right in that's what I do; I'll have to check whether both are required. I did notice for the first time that the libdispatch.so files are actually different sizes so that came as a surprise. I was gonna suggest symlinking but now I'm curious why they are, in fact, two different files.

[drodriguez]

@compnerd: I think you will explain the reason of this double copy better than anyone. It would be good to have your comment here.

@buttaface: please don't take my word as gospel, and let's wait for Saleem answers. I think there might be a confusion here. There might have been also changes over changes that might have modified the original intention of the change (specially with respect to rpaths and what libraries are linked against which libraries). The copy you don't want should only be installed if SWIFT_INSTALL_COMPONENTS includes the sourcekit-inproc component. If your distribution doesn't make use of sourcekit-inproc, the easiest way of avoiding that copy should be removing it from your CMake configuration.

[finagolfin]

If your distribution doesn't make use of sourcekit-inproc, the easiest way of avoiding that copy should be removing it from your CMake configuration.

I've already removed these two libdispatch libraries from the Swift 5.1.4 for Android package script (see last link in my OP): I'm just submitting this back upstream so other packagers like Ron don't have to deal with this either and I can remove it from my local patches. As for not installing sourcekit-inproc instead, I think that would work too for now, but I plan to ship sourcekit-lsp with the Android package of the upcoming 5.2 release, swiftlang/sourcekit-lsp#210.

Btw, I just submitted a pull to get cross-compilation with the Android NDK working again, #30170, when using the updated commands from docs/Android.md in master, along with other fixes. The Android CI didn't detect that regression because it builds libicu for linux from source, unlike what the doc commands do.

[finagolfin]

I don't think an answer is forthcoming, that linux rpath was already set before this install dependency was added.

@benlangmuir, do you mind running the tests for this pull? I can also disable it for macOS if you think it's not needed there, just let me know.

[benlangmuir]

@swift-ci please test

[finagolfin]

Alright, looks like this small cleanup doesn't break anything on linux/Android, and it changes nothing on mac/Windows (the Windows CI broke last week, long before this pull was tested, so a rerun of the Windows tests should work now), should be good to go.

[drodriguez]

@swift-ci please test Windows platform