Open bobergj opened 2 years ago
Hey!
I think the main problem is that Danger is made of two parts, a binary and a library which is linked to the Dangerfile compilation, would that still work out of the box with homebrew?
I think the main problem is that Danger is made of two parts, a binary and a library which is linked to the Dangerfile compilation, would that still work out of the box with homebrew
It would work if we build a .xcframework of the Danger dynamic library, built with BUILD_LIBRARY_FOR_DISTRIBUTION enabled. BUILD_LIBRARY_FOR_DISTRIBUTION not for the purpose of ABI stability, but for the purpose of module stability between Swift compiler versions.
Now there are three pretty big caveats for doing that:
xcodebuild does not support building a .framework (for the .xcframework) directly from a swiftpm package. For example:
xcodebuild archive -scheme Danger -sdk macosx -destination "generic/platform=macOS" -archivePath "archives/Danger.framework" SKIP_INSTALL=NO BUILD_LIBRARY_FOR_DISTRIBUTION=YES
where "Danger" is the library product in the Package.swift file. It does something, namely output some object files, but not the right thing. So the project would have to maintain a .xcodeproj in the repo for building the Danger.framework.
Implementation only dependencies has to be imported with @_implementationOnly
Eg. @_implementationOnly import RequestKit
. This is an underscored attribute, thus it's not guaranteed to be source compatible with future swift compiler versions. It will eventually change name/form, see https://forums.swift.org/t/pre-pitch-import-access-control-a-modest-proposal/50087
In GitHubDSL.swift GitHub
somewhat surprisingly exports the whole of OctoKit as public API:
public internal(set) var api: Octokit!
which means that also Octokit also has to be built with BUILD_LIBRARY_FOR_DISTRIBUTION. That does work today, but it's not in this projects control whether it works for future OctoKit versions.
Btw, building the homebrew tap products with BUILD_LIBRARY_FOR_DISTRIBUTION also when shipping it as source, as today, would solve: https://github.com/danger/swift/issues/449#issuecomment-873446428. In that, if you install the tap, then update Xcode/the swift compiler, the built Dangerfile can still be linked to the installed Danger library. As long as the library and Dangerfile are still API compatible of course.
@bobergj your approach described in the root post, would allow us to include a binary in say our repository, and use that directly? as in, we manage versioning ourselves?
Edit: Turns out it works just fine. I managed to speed up CI!
How is it going? Any updates?
@AvdLee
your approach described in the root post, would allow us to include a binary in say our repository, and use that directly? as in, we manage versioning ourselves? Edit: Turns out it works just fine. I managed to speed up CI!
That's gonna work, but you would have to recompile the binary every time you update Xcode, even minor versions. Since the Danger framework you build won't be binary compatible with a newer Swift compiler version.
I've used the docker image in the past, to avoid the compilation step on every build.
@jesus-mg-ios:
How is it going? Any updates?
Not from my side. As described in my comment above, fixing this would be pretty intrusive so I understand if the maintainers don't want to go for it. Personally I would just recommend using a docker image, although at the moment you'd have to build one yourself to get an up-to-date swiftlint version.
Summary
Include a pre-built executable in each release. Use that pre-built artifact in the homebrew tap.
Why
How
Building a universal binary for Intel and Apple Silicon macs should be a matter of:
Inspecting the resulting executable:
Since the third-party dependencies are statically linked, the executable is standalone: