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

Swift 6-ify the package manifest #1955

Merged
merged 9 commits into from
Jul 2, 2024
Merged

Conversation

glbrntt
Copy link
Collaborator

@glbrntt glbrntt commented Jul 1, 2024

Motivation:

v2 will require the Swift 6 language mode. In order to continue developing v1 and v2 on main we'll need a Swift 6 specific manifest version in addition to the 5.x version.

Moving forward we'll have two package manifests, Package.swift for 5.x containing only v1, and [email protected] using tools version 6.0 targetting v1 and v2.

Modifications:

  • Remove v2 targets from Package.swift
  • Add a second manifest and make it warning free with Swift 6, this requires turning static lets to static vars.
  • Conditionalise some imports in protoc-gen-grpc-swift (as it is shared by v1 and v2)
  • Fix an error in the plugin for Swift 6

Result:

v2 now only compiles with a Swift 6 compiler

glbrntt added 6 commits July 1, 2024 15:21
Motivation:

v2 will require the Swift 6 language mode. In order to continue
developing v1 and v2 on main we'll need a Swift 6 specific manifest
version in addition to the 5.x version.

Moving forward we'll have two package manifests, `Package.swift` for 5.x
containing only v1, and `[email protected]` using tools version 6.0
targetting v1 and v2.

Modifications:

- Remove v2 targets from Package.swift
- Add a second manifest and make it warning free with Swift 6, this
  requires turning `static let`s to `static var`s.
- Conditionalise some imports in `protoc-gen-grpc-swift` (as it is
  shared by v1 and v2)
- Fix an error in the plugin for Swift 6

Result:

v2 now only compiles with a Swift 6 compiler
@glbrntt glbrntt force-pushed the swift-6-manifest branch from 0937b62 to 71b1efe Compare July 1, 2024 16:13
@glbrntt glbrntt marked this pull request as ready for review July 1, 2024 16:37
@glbrntt glbrntt requested a review from gjcairo July 1, 2024 16:37
@glbrntt glbrntt added semver/none No version bump required. 🔨 semver/patch No public API change. and removed semver/none No version bump required. labels Jul 1, 2024
.atomics
],
path: "Sources/GRPCCore",
swiftSettings: [.swiftLanguageVersion(.v5)]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to adopt Swift 6 language version for the v2 packages? Or not yet?

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'm going to make the changes in a separate PR to keep the diff small(er). This way we can do it module by module (or at least do the larger ones separately)

Comment on lines 174 to 175
if options.v2 {
#if compiler(>=6.0)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we fatalError (or error in some other way) if the compiler is not Swift 6+ but the v2 option is set?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, good question. I think we should actually put the v2 option behind the compiler guard too tbh.

@glbrntt glbrntt requested a review from gjcairo July 2, 2024 12:26
@glbrntt glbrntt merged commit 36ffcb4 into grpc:main Jul 2, 2024
17 checks passed
@glbrntt glbrntt deleted the swift-6-manifest branch July 2, 2024 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔨 semver/patch No public API change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants