SPM dependency support - Githubissues

stephencelis commented 8 years ago

A colleague and I tried to install SourceKitten as a Swift Package Manager dependency earlier this week...

giphy (Historical reenactment.)

Unfortunately, the current workarounds in SourceKitten's Makefile don't propagate to dependencies. While we were able to see some success by copying over some of the -Xcc and -Xlinker flags, heavily editing and paring down a forked version of the SourceKitten repo, and specifying sourcekitd and clang-c dependencies explicitly in the Package.swift, in the end we couldn't get it to work.

We ended up running out of time, so we built the tool into an OS X app instead (where SourceKitten—and other dependencies—installed flawlessly). I did want to flag the lack of SPM support as a current issue, though.

There's one other thing I'd love to have discussion around after diving into the SourceKitten code base: have you thought about dividing SourceKitten's responsibilities into multiple libraries? The sourcekitd bits don't seem to require the clang-c bits at all—the latter seems to be used for documentation support/analyis and the tool we were working on really only needed the former. We may have been able to get things up and running on SPM quickly if it weren't for the Clang_C module.

jpsim commented 8 years ago

Historical reenactment.

:sob:

I'm sorry about this, it's still early days for SourceKitten's SPM support.

have you thought about dividing SourceKitten's responsibilities into multiple libraries

Over time, SourceKitten has expanded from a mere "Swift wrapper around sourcekitd" to "backend to SwiftLint and jazzy", which is unfortunate. Splitting that out into different libraries is logical, but would be a significant undertaking. I'm not opposed to it, but neither am I really motivated to do that just yet, to be blunt.

Although I don't think avoiding Clang_C would have helped for your specific case, since it's packaged with sourcekitd as part of the Swift distribution.

You can see the (admittedly very hacky) way SwiftLint integrates SourceKitten here: realm/SwiftLint#462

You could mirror that approach with your project.

Ultimately, I can think of a few ways we could improve our support for SPM in SourceKitten:

avoid the use of custom swift build flags. This was initially the case when I took a first crack at SPM support in #141, using other packages/modules with external installation rather than swift build flags.
better comply with SPM's recommended source layout. By splitting out the sourcekitten target into a separate repo, moving Source/SourceKittenFramework to Sources/SourceKittenFramework and Source/SourceKittenFrameworkTests to Tests.

These changes are pretty destructive to how things are set up now, but may be necessary to properly support SPM moving forward.

stephencelis commented 8 years ago

I'm sorry about this, it's still early days for SourceKitten's SPM support.

No apologies necessary! We were perhaps being too ambitious, diving into an SPM project with dependencies. Most of my libraries have yet to support SPM, too!

I had fun diving into the code and learning more about SourceKit, and using SourceKitten in the end. Thanks for the library! :+1:

avoid the use of custom swift build flags

I'm not sure this is possible at the moment. Even in my vanilla attempts to load sourcekitd into an SPM package, because sourcekitd ships as a framework, I think you need a build flag to add it to swift build's framework search path. This may be functionality that needs to be aded to SPM directly.

jpsim commented 8 years ago

I'm not sure this is possible at the moment. Even in my vanilla attempts to load sourcekitd in an SPM package, because sourcekitd ships as a framework, I think you need a build flag to add it to swift build's framework search path. This may be functionality that needs to be aded to SPM directly.

This is definitely possible, it's what we did originally in #141.

stephencelis commented 8 years ago

This is definitely possible, it's what we did originally in #141.

I skimmed this but couldn't find the commit that allows for a simple swift build, import sourcekitd, and sourcekitd_initialize() without error.

jpsim commented 8 years ago

import sourcekitd was actually import SourceKit, with SourceKit defined as an SPM package here: https://github.com/jpsim/SourceKit

and added as a dependency in the Package.swift file as you can see here: https://github.com/jpsim/SourceKitten/pull/141/files#diff-37ca2dd15ca0f6b1b49e78db084ef5b9R11

stephencelis commented 8 years ago

Ah, I guess older versions of the Swift toolchain shipped with a static library instead of a framework. This no longer works in the latest toolchain (where sourcekitdInProc.dylib has been replaced with sourcekitd.framework).

jpsim commented 8 years ago

Ah, I guess older versions of the Swift toolchain shipped with a static library instead of a framework. This no longer works in the latest toolchain (where sourcekitdInProc.dylib has been replaced with sourcekitd.framework).

No, sourcekitdInProc.dylib never came with the Swift distribution, I built it from source with utils/build-script -R and then uploaded it, which is what I meant earlier by "external installation". #141 did this "external installation" by downloading this binary.

stephencelis commented 8 years ago

Gotcha! I may have been unclear earlier. I actually originally linked against a dylib I built from Swift source, too, but came across the sourcekitd.framework bundled in the toolchain and found that adding its parent directory to the framework includes paths was easier in the end. Unless I'm missing something from your comments here, there's definitely extra (unavoidable) work required on the part of libraries that depend on SourceKitten (or SourceKit directly). A simple line in Package.swift dependencies won't do, but if the manifest allowed packages to specify compiler/linker flags, that could resolve things.

jpsim commented 8 years ago

You're right on all counts on that last comment! Sorry if I seemed defensive earlier, I was just trying to explain what was being done before.

stephencelis commented 8 years ago

Not at all. Thanks again for the clarifications! I just wanted to make sure I had the full story if I have time to make any of the enhancements we discussed. I just didn't want to reinvent the wheel or work on my own leaner SourceKit wrapper if you had interest in splitting them up!

jpsim commented 8 years ago

I filed SR-688 to track part of this on the SPM side.

jpsim commented 8 years ago

We made some changes in #170 that should allow slightly better SPM dependency integration.

You'll first need to run this:

curl https://raw.githubusercontent.com/jpsim/SourceKitten/master/script/spm_bootstrap | bash -s $(SWIFT_SNAPSHOT)

and then if your Package.swift file contains a SourceKitten dependency, swift build should "just work". See how we're adapting SwiftLint for these changes: realm/SwiftLint#516

stephencelis commented 8 years ago

It's beautiful! :joy:

jpsim commented 8 years ago

Yeah yeah, I know, curl ... | sh makes me cringe too, but unfortunately that's the best way to install the libclang and libsourcekitdInProc dependencies for now. It's not like they're available via Homebrew!

I've asked someone at Apple if there's a way we can get libsourcekitdInProc included in the swift.org releases, and if that happens this will be a bit nicer.

I think this resolves your initial issue. Please open a new issue if you can think of other improvements we can make in this space.

stephencelis commented 8 years ago

I was being sincere! It's a great solution given the problem! Hope Apple provides a better solution for us in the future.

jpsim commented 8 years ago

Hope Apple provides a better solution for us in the future.

Cheers to that!

jpsim / SourceKitten

SPM dependency support #160