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.

Sign up for GitHub

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 1 #476

Closed
wants to merge 7 commits into from
Closed

Enable a more portable binary on macOS - Approach 1 #476

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
13dd6bf
Update Package.swift to always use SwiftSyntax 5.6 and to use StaticI…
liamnichols Mar 29, 2022
d1385f9
Remove compiler version checks from imports to SwiftSyntaxParser
liamnichols Apr 5, 2022
772ea82
Extract lib_intenralSwiftSyntaxParser dependency into seperate packag…
liamnichols Apr 5, 2022
7c06224
Bump swift-tools-version to 5.3 to support binaryTargets
liamnichols Apr 6, 2022
75e5f71
Run 'swift package update StaticSwiftSyntaxParser'
liamnichols Apr 6, 2022
124ba30
Add back missing support for Swift 5.3 and 5.4. Only use StaticSwiftS…
liamnichols Apr 7, 2022
45eff87
Use SwiftSyntax 0.50500.0 on Linux when compiling with Swift 5.5 tool…
liamnichols Apr 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions Package.resolved
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,15 @@
{
"object": {
"pins": [
{
"package": "StaticSwiftSyntaxParser",
"repositoryURL": "https://gist.github.com/liamnichols/92f8fdcf2864d0fd1619a18828acafb8.git",
"state": {
"branch": "main",
"revision": "4134efa908ee6932f62f243cd1815000e51f44c6",
"version": null
}
},
{
"package": "AEXML",
"repositoryURL": "https://github.com/tadija/AEXML",
Expand Down
64 changes: 25 additions & 39 deletions Package.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,47 +1,35 @@
// swift-tools-version:5.2
// swift-tools-version:5.3
import PackageDescription

// Use the appropriate version of SwiftSyntax based on the current compiler
#if compiler(>=5.5)
let swiftSyntaxVersion: Package.Dependency.Requirement = .exact("0.50600.1")
liamnichols marked this conversation as resolved.
Show resolved Hide resolved
#elseif compiler(>=5.4)
let swiftSyntaxVersion: Package.Dependency.Requirement = .exact("0.50400.0")
#elseif compiler(>=5.3)
let swiftSyntaxVersion: Package.Dependency.Requirement = .exact("0.50300.0")
#else
fatalError("This version of Periphery does not support Swift <= 5.2.")
#endif

var dependencies: [Package.Dependency] = [
.package(url: "https://github.com/apple/swift-system", from: "1.0.0"),
.package(url: "https://github.com/jpsim/Yams", from: "4.0.0"),
.package(url: "https://github.com/tadija/AEXML", from: "4.0.0"),
.package(url: "https://github.com/apple/swift-argument-parser", from: "1.0.0"),
.package(name: "SwiftIndexStore", url: "https://github.com/kateinoigakukun/swift-indexstore", from: "0.0.0")
.package(name: "SwiftIndexStore", url: "https://github.com/kateinoigakukun/swift-indexstore", from: "0.0.0"),
.package(name: "SwiftSyntax", url: "https://github.com/apple/swift-syntax", swiftSyntaxVersion)
]
#if swift(>=5.6)
dependencies.append(
.package(
name: "SwiftSyntax",
url: "https://github.com/apple/swift-syntax",
.exact("0.50600.1")
)
)
#elseif swift(>=5.5)
dependencies.append(
.package(
name: "SwiftSyntax",
url: "https://github.com/apple/swift-syntax",
.exact("0.50500.0")
)
)
#elseif swift(>=5.4)
dependencies.append(
.package(
name: "SwiftSyntax",
url: "https://github.com/apple/swift-syntax",
.exact("0.50400.0")
)
)
#elseif swift(>=5.3)

// When on macOS, using SwiftSyntax for 5.6, also include StaticSwiftSyntaxParser to statically link internal dependencies
#if os(macOS) && compiler(>=5.5)
dependencies.append(
.package(
name: "SwiftSyntax",
url: "https://github.com/apple/swift-syntax",
.exact("0.50300.0")
name: "StaticSwiftSyntaxParser",
url: "https://gist.github.com/liamnichols/92f8fdcf2864d0fd1619a18828acafb8.git",
.branch("main")
Comment on lines +28 to +30
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried this out in #480

)
)
#else
fatalError("This version of Periphery does not support Swift <= 5.2.")
#endif

#if os(macOS)
Expand Down Expand Up @@ -72,13 +60,11 @@ var peripheryKitDependencies: [PackageDescription.Target.Dependency] = [
.product(name: "SwiftIndexStore", package: "SwiftIndexStore")
]

#if swift(>=5.6)
peripheryKitDependencies.append(
.product(
name: "SwiftSyntaxParser",
package: "SwiftSyntax"
)
)
// Using Swift 5.5+, we need the SwiftSyntaxParser library, but on macOS specifically we also want to use the statically linked version
#if os(macOS) && compiler(>=5.5)
peripheryKitDependencies.append(.product(name: "StaticSwiftSyntaxParser", package: "StaticSwiftSyntaxParser"))
#elseif compiler(>=5.5)
peripheryKitDependencies.append(.product(name: "SwiftSyntaxParser", package: "SwiftSyntax"))
#endif

var targets: [PackageDescription.Target] = [
Expand Down
2 changes: 1 addition & 1 deletion Sources/PeripheryKit/Syntax/MultiplexingSyntaxVisitor.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Foundation
import SystemPackage
import SwiftSyntax
#if swift(>=5.6)
#if canImport(SwiftSyntaxParser)
import SwiftSyntaxParser
#endif

Expand Down
2 changes: 1 addition & 1 deletion Sources/PeripheryKit/Syntax/UnusedParameterParser.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Foundation
import SystemPackage
import SwiftSyntax
#if swift(>=5.6)
#if canImport(SwiftSyntaxParser)
import SwiftSyntaxParser
#endif

Expand Down
2 changes: 1 addition & 1 deletion Tests/PeripheryTests/Syntax/TypeSyntaxInspectorTest.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Foundation
import XCTest
import SwiftSyntax
#if swift(>=5.6)
#if canImport(SwiftSyntaxParser)
import SwiftSyntaxParser
#endif
@testable import TestShared
Expand Down