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 5.10 #25

Merged
merged 7 commits into from
Nov 26, 2024
Merged

Swift 5.10 #25

merged 7 commits into from
Nov 26, 2024

Conversation

rasberik
Copy link
Contributor

@rasberik rasberik commented Nov 21, 2024

Checklist

  • All tests are passed.
  • Added tests.
  • Searched existing issues to make sure not duplicated.

Motivation and Context

Version bump for dependencies to latest and package swift version 5.10

apple/swift-argument-parser 1.1.2 → 1.5.0
apple/swift-syntax 509.0.1 → 510.0.3
apple/swift-tools-support-core 0.2.3 → 0.7.1
jpsim/Yams 5.0.0 → 5.1.3

Tools & apple/swift-format 509.0 → 510.10

Impact on Existing Code

For users, expecting none (any reliable way to verify?)

@rasberik
Copy link
Contributor Author

rasberik commented Nov 21, 2024

@ra1028 Do you know if its ok to use swift-syntax & swift-format 600 for swift 5.10?
No issues from my checks but not sure if there is anything I might miss or dont know..

Or does it need to strictly match to swift version used in package..?

@ra1028
Copy link
Owner

ra1028 commented Nov 22, 2024

@rasberik
swift-syntax doesn't depend on swift toolchains so it runs with no error but not very safe if there's syntax diff.
You should update the readme to describe the compatibility.

@rasberik
Copy link
Contributor Author

rasberik commented Nov 22, 2024

@ra1028 Thanks

https://swiftpackageindex.com/swiftlang/swift-syntax/600.0.1/documentation/swiftsyntax/swift-version

We require that SwiftSyntax builds with the latest released compiler and the previous major version (e.g. with Swift 5.8 and Swift 5.7).

And has build compatibility results here: https://swiftpackageindex.com/swiftlang/swift-syntax#readme

But after your comment and deeper look, I feel there are unknown variables that are hard to verify and swift-mod is compiled with Xcode 15, may be its safer to leave it to another version & migration.

I updated the deps to use 510

@rasberik rasberik marked this pull request as ready for review November 22, 2024 09:32
@@ -10,7 +10,7 @@ jobs:
strategy:
matrix:
xcode_version:
- "15.0.1"
- "15.4"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[note] Required to bump to get Swift 5.10

@@ -26,11 +26,11 @@ jobs:
run: make test
macOS:
name: Test on macOS
runs-on: macos-13
runs-on: macos-14
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[note] Required to get Xcode 15.4 support

Package.swift Outdated
Comment on lines 17 to 18
.package(url: "https://github.com/apple/swift-argument-parser.git", .upToNextMinor(from: "1.5.0")),
.package(url: "https://github.com/apple/swift-syntax.git", .upToNextMinor(from: "510.0.3")),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Xcode 15.4 and 16.1 persistently complain with failed resolution (cannot find the repo)

Had to use xcodebuild -resolvePackageDependencies on both Package.swift

Copy link
Owner

Choose a reason for hiding this comment

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

Not sure if related but the swift related repos have been migrated to under swiftlang/ from apple/ so please update them according to it.
Also, can you use exact version instead of upToNextMinor or from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Not related it turned out, updated: 3dbb137

@@ -301,10 +301,10 @@ You can also install swift-mod by downloading `swift-mod.zip` from the latest Gi

### Swift Version Support

`swift-mod` depends on [SwiftSyntax](https://github.com/apple/swift-syntax) that the version in use must match the toolchain version until Swift 5.7.
Copy link
Contributor Author

@rasberik rasberik Nov 22, 2024

Choose a reason for hiding this comment

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

[note] Very difficult sentence that I couldnt understand, simplified

Copy link
Contributor Author

Choose a reason for hiding this comment

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

until Swift 5.7 is not if important historical matter or forgotten to update

|5.8 and later|latest |
|5.8 |0.2.0 |
|5.9 |0.2.0 |
|5.10 |0.2.1 |
Copy link
Contributor Author

Choose a reason for hiding this comment

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

[note] To be: 0.2.1

Copy link
Owner

@ra1028 ra1028 left a comment

Choose a reason for hiding this comment

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

It mostly looks good but left some comments

Package.swift Outdated
Comment on lines 17 to 18
.package(url: "https://github.com/apple/swift-argument-parser.git", .upToNextMinor(from: "1.5.0")),
.package(url: "https://github.com/apple/swift-syntax.git", .upToNextMinor(from: "510.0.3")),
Copy link
Owner

Choose a reason for hiding this comment

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

Not sure if related but the swift related repos have been migrated to under swiftlang/ from apple/ so please update them according to it.
Also, can you use exact version instead of upToNextMinor or from?


import PackageDescription

let package = Package(
name: "Tools",
dependencies: [
.package(url: "https://github.com/apple/swift-format.git", .upToNextMinor(from: "509.0.0")),
.package(url: "https://github.com/apple/swift-format.git", .upToNextMinor(from: "510.1.0")),
Copy link
Owner

Choose a reason for hiding this comment

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

Ditto

@rasberik rasberik requested a review from ra1028 November 26, 2024 08:39
Copy link
Owner

@ra1028 ra1028 left a comment

Choose a reason for hiding this comment

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

LGTM 🚀

@ra1028 ra1028 merged commit 9fc17e4 into ra1028:master Nov 26, 2024
2 checks passed
@rasberik rasberik deleted the swift-5.10 branch November 26, 2024 09:14
@rasberik rasberik mentioned this pull request Nov 27, 2024
3 tasks
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