Skip to content
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

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

Open
bobergj opened this issue Oct 13, 2021 · 5 comments

Comments

@bobergj
Copy link
Contributor

bobergj commented Oct 13, 2021

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:

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)
@f-meloni
Copy link
Member

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?

@bobergj
Copy link
Contributor Author

bobergj commented Dec 2, 2021

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: #449 (comment). 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.

@AvdLee
Copy link
Contributor

AvdLee commented Nov 9, 2022

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

@jesus-mg-ios
Copy link

How is it going? Any updates?

@bobergj
Copy link
Contributor Author

bobergj commented Feb 2, 2023

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

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

No branches or pull requests

4 participants