swift icon indicating copy to clipboard operation
swift copied to clipboard

[Feature Request] Include a pre-built executable in each release and use in the homebrew tap

Open bobergj opened this issue 4 years ago • 5 comments

Summary

Include a pre-built executable in each release. Use that pre-built artifact in the homebrew tap.

Why

  • Fetching the dependencies and building danger-swift can take a long time, even with the swiftpm approach, especially on CI such as Xcode Cloud where cache is per workflow/branch
  • The homebrew tap builds from source https://github.com/danger/swift/blob/master/Scripts/create_homebrew_tap.sh
  • The danger-js homebrew tap already serves a pre-built binary https://github.com/danger/homebrew-tap/blob/master/danger-js.rb

How

Building a universal binary for Intel and Apple Silicon macs should be a matter of:

swift build --configuration release --arch arm64 --arch x86_64

Inspecting the resulting executable:

file .build/apple/Products/Release/danger-swift
.build/apple/Products/Release/danger-swift: Mach-O universal binary with 2 architectures: [x86_64:Mach-O 64-bit executable x86_64] [arm64:Mach-O 64-bit executable arm64]
.build/apple/Products/Release/danger-swift (for architecture x86_64):	Mach-O 64-bit executable x86_64
.build/apple/Products/Release/danger-swift (for architecture arm64):	Mach-O 64-bit executable arm64
du -h .build/apple/Products/Release/danger-swift
1.4M	.build/apple/Products/Release/danger-swift

Since the third-party dependencies are statically linked, the executable is standalone:

otool -L .build/apple/Products/Release/danger-swift
.build/apple/Products/Release/danger-swift:
	/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 1775.118.101)
	/usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1292.100.5)
	/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1775.118.101)
	@rpath/libswiftCore.dylib (compatibility version 1.0.0, current version 1205.0.24)
	@rpath/libswiftCoreFoundation.dylib (compatibility version 1.0.0, current version 1.6.0, weak)
	@rpath/libswiftCoreGraphics.dylib (compatibility version 1.0.0, current version 2.0.0, weak)
	@rpath/libswiftDarwin.dylib (compatibility version 1.0.0, current version 0.0.0, weak)
	@rpath/libswiftDispatch.dylib (compatibility version 1.0.0, current version 4.100.1)
	@rpath/libswiftFoundation.dylib (compatibility version 1.0.0, current version 25.102.0)
	@rpath/libswiftIOKit.dylib (compatibility version 1.0.0, current version 1.0.0, weak)
	@rpath/libswiftObjectiveC.dylib (compatibility version 1.0.0, current version 3.0.0)
	@rpath/libswiftXPC.dylib (compatibility version 1.0.0, current version 1.1.0, weak)

bobergj avatar Oct 13 '21 05:10 bobergj

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?

f-meloni avatar Oct 19 '21 17:10 f-meloni

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:

  1. 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.

  1. 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

  2. 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 avatar Dec 02 '21 03:12 bobergj

@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!

AvdLee avatar Nov 09 '22 12:11 AvdLee

How is it going? Any updates?

jesus-mg-ios avatar Dec 28 '22 13:12 jesus-mg-ios

@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.

bobergj avatar Feb 02 '23 04:02 bobergj