Enable a more portable binary on macOS - Approach 1

[liamnichols]

Background

• Using StaticInternalSwiftSyntaxParser? #473

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 1

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 achieve that by extracting things out into a small placeholder-type package (StaticSwiftSyntaxParser) that declares the binary target and the dependency on swift-syntax's SwiftSyntaxParser library for the 5.6 version. This doesn't support 5.5, although it technically could but its just a little confusing because the SwiftSyntaxParser target was only added to swift-syntax in 5.6.

I feel like this approach is a little hackier than the second approach for a couple of reasons:

1. It would be nice for periphery to use StaticInternalSwiftSyntaxParser when compiling with Swift 5.5 too, but because the placeholder/wrapper package doesn't have a 5.5 version, I can't do that. But we can just use 0.50600.1 since its source-compatible with Swift 5.5, its just a bit more confusing when you look at Package.swift, especially because there is a discrepancy with Linux too as a result. It's even complicated to try and explain here
2. We have to declare the StaticSwiftSyntaxParser dependency, but its not technically imported in the project which is also a bit confusing.

[liamnichols]

@ileitch, please could you give this approval to run?

1 workflow awaiting approval

I'll add some more details shortly, but the added dependency to StaticSwiftSyntaxParser is hosted as a gist temporarily here: https://gist.github.com/liamnichols/92f8fdcf2864d0fd1619a18828acafb8

[liamnichols]

Sorry, it looks like I had to change the swift-tools-version and bump it to 5.3 in order to be able to declare a .binaryTarget. While I didn't have to do this in Package.swift of this repo, I did it anyway since you wouldn't be able to build using Swift 5.2 toolchain anymore anyway because of the dependency.

Please re-approve the workflow again 🙈 I guess this has to happen until I merge something so sorry if it fails again 😬

[liamnichols]

Ok, I underestimated the amount of backwards/linux compatibility in the project and what that meant here 😄

Using StaticInternalSwiftSyntax parser is not possible on Linux, so we should stick with using SwiftSyntax regularly. In this change, I have done the following:

• Bumped swift-tools-version to 5.3 since we never supported 5.2 anyway (there's a fatalError in Package.swift)
• Choose the right version of swift-syntax based on the compiler version. Since swift-syntax 0.50600.1 can be compiled with Swift 5.5, there is no need to use version 0.50500.0.
• Changed the guards around import SwiftSyntaxParser (a new module added to 0.50600.1) to use #if canImport(SwiftSyntaxParser) rather than #if swift(=>5.6) because of the point above
• When on macOS (and using swift-syntax 0.50600.1), also include StaticSwiftSyntaxParser dependency that will force the project to statically link lib_internalSwiftSyntaxParser instead of using the dylib.
  • I had to move StaticSwiftSyntaxParser it into it's own package rather than just including the .binaryTarget as part of this Package.swift due to issues described here: Using StaticInternalSwiftSyntaxParser? #473 (comment)
  • If this works, and we're happy to go with this direction, my suggestion would be to turn the gist into a real repo under peripheryapp org

Please could you re-run the checks 🙏 I only have my hands on an M1 Mac now so I can't install older versions of Xcode and I don't have Docker setup to test Swift on Linux locally 😓 It should be good though 👍

[ileitch]

This is awesome work, Liam! I will hopefully have some time to fully digest this on the weekend. I've added you as a collaborator so that I'm not blocking you on these checks 😄 .

[liamnichols]

Besides the caching issues solved in #479, the checks are good now and this is working 👍

I've left an inline comment about the approach for using StaticInternalSwiftSyntaxParser since there is a secondary alternative to the one I've used here 🙇

[liamnichols]

Ok, so recap on this.. I've had to move this out of Package.swift into a dedicated dependency because using .binaryTarget in Package.swift currently breaks swift package describe, which the tests rely on. SR-15243 (and SR-15065 when working around things with a local binary target).

If you'd be happy with this change and want to proceed, I'd recommend that you push the gist up to it's own repository under the peripheryapp org, then tag it with something like 5.6 so we can do this:

Suggested change

	name: "StaticSwiftSyntaxParser",
	url: "https://gist.github.com/liamnichols/92f8fdcf2864d0fd1619a18828acafb8.git",
	.branch("main")
	name: "StaticSwiftSyntaxParser",
	url: "https://github.com/peripheryapp/static-swift-syntax-parser.git",
	.exact("5.6")

---

Another alternative that I had considered was to just fork https://github.com/apple/swift-syntax entirely and branch off 0.50600.1 and tweak the original Package.swift so that it uses StaticInternalSwiftSyntaxParser instead of the dylib. We'd then be able to update line 5 to something like this:

  #if os(macOS) && compiler(>=5.5) || compiler(>=5.6)
- let swiftSyntaxVersion: Package.Dependency.Requirement = .exact("0.50600.1")
+ let swiftSyntaxVersion: Package.Dependency.Requirement = .exact("0.50600.1-static")
  #elseif compiler(>=5.5)
  let swiftSyntaxVersion: Package.Dependency.Requirement = .exact("0.50500.0")

As well as a few other tweaks

[liamnichols]

I tried this out in #480

[liamnichols]

Closing this in favour of #480