Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Bump requirements to Swift 5.4, migrate to @resultBuilder #442

Merged
merged 14 commits into from
Sep 8, 2021

Conversation

MaxDesiatov
Copy link
Collaborator

@MaxDesiatov MaxDesiatov commented Aug 22, 2021

This updates the project to use Swift 5.4 across all platforms. Swift 5.4 is now also the required version, which allows us to use @resultBuilder instead of the deprecated version of this attribute from Swift 5.3.

Use carton 0.11.0 or later from now on to build with SwiftWasm 5.4.0.

@MaxDesiatov MaxDesiatov added the continuous integration Changes related to the continuous integration process label Aug 22, 2021
@MaxDesiatov MaxDesiatov changed the title Test with SwiftWasm 5.4 on CI Migrate to Swift 5.4 Sep 2, 2021
@MaxDesiatov MaxDesiatov changed the title Migrate to Swift 5.4 Bump requirements to Swift 5.4, migrate to @resultBuilder Sep 2, 2021
@MaxDesiatov MaxDesiatov marked this pull request as ready for review September 2, 2021 19:15
@MaxDesiatov MaxDesiatov requested a review from a team September 2, 2021 21:01
@ezraberch
Copy link
Member

We can switch back to the Swift 5.4 version of SwiftLint now. You can use mayk-it/action-swiftlint@3.2.2 for this.

@ezraberch
Copy link
Member

Getting some new warnings:

  1. warning: in tools version 5.4.0 and later, use 'executableTarget()' to declare executable targets

Can be fixed by switching to executableTarget in Package.swift for the 5 executable targets.

  1. warning: assuming you mean 'Color.red'; did you mean 'Optional<Color>.red' instead?

I'm not exactly sure what's going on here. Seems to no longer be an issue with Xcode 13, so could just ignore for now.

  1. 'CGTK' gtk+-3.0.pc: warning: prohibited flag(s): -Wl,-framework,Cocoa, -Wl,-framework,Carbon, -Wl,-framework,CoreGraphics
    'CGDK' gdk-3.0.pc: warning: prohibited flag(s): -Wl,-framework,Cocoa, -Wl,-framework,Carbon, -Wl,-framework,CoreGraphics

Know which flags are the problem here?

  1. warning: '--enable-test-discovery' option is deprecated; tests are automatically discovered on all platforms

Looks like a carton issue.

@MaxDesiatov
Copy link
Collaborator Author

MaxDesiatov commented Sep 7, 2021

Getting some new warnings:

  1. warning: in tools version 5.4.0 and later, use 'executableTarget()' to declare executable targets

Can be fixed by switching to executableTarget in Package.swift for the 5 executable targets.

Done

  1. warning: assuming you mean 'Color.red'; did you mean 'Optional<Color>.red' instead?

I'm not exactly sure what's going on here. Seems to no longer be an issue with Xcode 13, so could just ignore for now.

Yup, I have no idea either, doesn't look critical at the moment

  1. 'CGTK' gtk+-3.0.pc: warning: prohibited flag(s): -Wl,-framework,Cocoa, -Wl,-framework,Carbon, -Wl,-framework,CoreGraphics
    'CGDK' gdk-3.0.pc: warning: prohibited flag(s): -Wl,-framework,Cocoa, -Wl,-framework,Carbon, -Wl,-framework,CoreGraphics

Know which flags are the problem here?

Not sure, nothing's changed on that front in Tokamak codebase, I suspect some updates to SwiftPM in Swift 5.4 could be the culprit.

  1. warning: '--enable-test-discovery' option is deprecated; tests are automatically discovered on all platforms

Looks like a carton issue.

Fixed in swiftwasm/carton#257

I assume all warnings that could be resolved in Tokamak are resolved here?

Copy link
Member

@ezraberch ezraberch left a comment

Choose a reason for hiding this comment

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

Yup, looks good.

@MaxDesiatov MaxDesiatov merged commit 3d9558f into main Sep 8, 2021
@MaxDesiatov MaxDesiatov deleted the ci/swift-5.4 branch September 8, 2021 09:18
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
continuous integration Changes related to the continuous integration process
Development

Successfully merging this pull request may close these issues.

2 participants