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

Enable a more portable binary on macOS - Approach 2 #480

Conversation

liamnichols
Copy link
Collaborator

@liamnichols liamnichols commented Apr 8, 2022

Background

To make the periphery binary more portable on macOS, we want to have the lib_internalSwiftSyntaxParser library statically linked into the binary instead of having to link to a dylib version somewhere on the system.

Approach 2

This is a second take on the approach used in #476, the main problem I'm trying to work around in these solutions is that I can't use binary targets in this Package.swift since it breaks swift package describe (SR-15243 and SR-15065).

In this approach, I work around that by using a fork of the apple/swift-syntax repo where I have made some minor modifications to the Package.swift file so that the StaticInternalSwiftSyntaxParser dependency is properly declared on the SwiftSyntax/SwiftSyntaxParser targets directly.

While forking the official dependency is somewhat of an inconvenience, it feels like the 'cleanest' approach since it avoids having to do other workarounds like the one used in the first approach, or like those that have to exist in SwiftLint/Sourcery.

Since the fork is only used to reconfigure the Package.swift, it doesn't feel like that big of a problem (we're just using it to re-tag the releases after patching), but ideally I'd like to transfer the fork/tags into the peripheryapp org as well.

Here are the diffs between the new tags in swift-syntax repo:

swiftlang/swift-syntax@0.50600.1...liamnichols:0.50600.1-static
swiftlang/swift-syntax@0.50500.0...liamnichols:0.50500.0-static

@ileitch
Copy link
Contributor

ileitch commented Apr 15, 2022

Hey @liamnichols, thanks for digging into all of this. I've forked swift-syntax to the peripheryapp org, and given you write access. Feel free to copy over your changes, and then we can get this merged 👍

@liamnichols liamnichols force-pushed the ln/static-lib-internal-swift-syntax-parser-via-fork branch from d671f39 to 0707223 Compare April 15, 2022 18:22
@liamnichols
Copy link
Collaborator Author

Thanks @ileitch! I've pushed the changes up as well as rebasing from master after merging #479 just to be 100% sure that everything is ✅ before merging this 🙇

@ileitch ileitch merged commit 48f3e9b into peripheryapp:master Apr 16, 2022
@liamnichols liamnichols deleted the ln/static-lib-internal-swift-syntax-parser-via-fork branch April 16, 2022 09:11
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

Successfully merging this pull request may close these issues.

2 participants