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 compatibility #108

Merged
merged 20 commits into from
Jul 11, 2024
Merged

Swift 6 compatibility #108

merged 20 commits into from
Jul 11, 2024

Conversation

Supereg
Copy link
Member

@Supereg Supereg commented Jul 1, 2024

Swift 6 compatibility

♻️ Current situation & Problem

This PR fixes the swift concurrency flag. There weren't any concurrency warnings left when compiling with the latest Swift 6 toolchain that is part of Xcode 16 beta 3. Note, that Xcode 15 will trigger some warnings as some symbols aren't annotated as Sendable in the previous SDK. Further, some sendability checks changed.

This PR changes some actor isolations of some types and interfaces. Most importantly, we add @MainActor isolation to the Module/configure() method. We found, that a lot of modules manipulate @MainActor isolated properties in their configure method (those that are relevant for UI components) but cannot do so without spawning a new Task, as the configure method previously didn't have @MainActor guarantees. Except for out-of-band module loading, configure() would have always been called on the Main Actor (through SwiftUI initialization). Making all module interactions to be isolated to @MainActor greatly simplifies some of our internal state handling.

⚙️ Release Notes

  • Fix swift strict concurrency flag.
  • Resolve some strict concurrency warnings.

Breaking Changes

  • Module/configure() is now isolated to @MainActor.
  • Spezi/loadModule(_:ownership:) and Spezi/unloadModule(_:) are now isolated to @MainActor.

📚 Documentation

Minor documentation files.

✅ Testing

Test target was updated to latest concurrency changes as well.

📝 Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

@PSchmiedmayer PSchmiedmayer added the enhancement New feature or request label Jul 1, 2024
@PSchmiedmayer PSchmiedmayer linked an issue Jul 1, 2024 that may be closed by this pull request
1 task
@PSchmiedmayer
Copy link
Member

Thank you for looking into this @Supereg!

@Supereg Supereg marked this pull request as ready for review July 10, 2024 10:37
@Supereg Supereg requested a review from PSchmiedmayer July 10, 2024 10:44
@Supereg
Copy link
Member Author

Supereg commented Jul 10, 2024

@PSchmiedmayer the Xcode 15 toolchain still triggers some concurrency warnings that are not triggered with the Xcode 16 beta 3 toolchain (e.g., Logger has now Sendable conformance with the latest SDKs). Should we aim to host some Xcode 16 runners soon? This could also be useful to enable Swift 6 language mode in which Concurrency warnings are treated as errors which would make efforts in StanfordSpezi/.github#50 to be non-essential.

We could either move with all runners to the beta toolchain for the Spezi Org. Seems relatively stable by now and is probably the easiest to maintain. Otherwise, we could allocate some dedicated Swift 6 runners if we have the capacity.

Let me know what you think.

Copy link

codecov bot commented Jul 10, 2024

Codecov Report

Attention: Patch coverage is 95.69892% with 4 lines in your changes missing coverage. Please review.

Project coverage is 85.85%. Comparing base (5ada3f3) to head (d71107c).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #108      +/-   ##
==========================================
- Coverage   87.64%   85.85%   -1.78%     
==========================================
  Files          46       45       -1     
  Lines        1375     1392      +17     
==========================================
- Hits         1205     1195      -10     
- Misses        170      197      +27     
Files Coverage Δ
...ilities/Communication/ProvidePropertyWrapper.swift 100.00% <100.00%> (ø)
...bilities/Communication/StorageValueCollector.swift 100.00% <ø> (ø)
...abilities/Communication/StorageValueProvider.swift 100.00% <ø> (ø)
...pezi/Capabilities/Lifecycle/LifecycleHandler.swift 88.89% <ø> (ø)
...ifications/RegisterRemoteNotificationsAction.swift 85.11% <ø> (ø)
...apabilities/Observable/EnvironmentAccessible.swift 100.00% <100.00%> (ø)
...Capabilities/Observable/ModelPropertyWrapper.swift 69.77% <100.00%> (+3.11%) ⬆️
...ilities/ViewModifier/ModifierPropertyWrapper.swift 80.00% <100.00%> (+2.23%) ⬆️
...pabilities/ViewModifier/ViewModifierProvider.swift 94.45% <100.00%> (ø)
Sources/Spezi/Dependencies/DependencyManager.swift 78.45% <ø> (-19.82%) ⬇️
... and 15 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5ada3f3...d71107c. Read the comment docs.

@Supereg Supereg changed the title Fix swift concurrency flag Swift 6 compatibility Jul 10, 2024
Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

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

Thank you for all the improvements to get the package Swift 6-ready.

I have also installed Xcode 16.0 Beta 3 on our runners so we check that with the latest encode version and validate that the package builds with Xcode 16 and Swift 6.

One thing I noticed. Out of curiosity I tried to update the Swift tools version in the Package.swift to 6.0 but got multiple compiler errors without any direct references to source code errors. I am not sure if you can verify this on your machine or if this is a general issue that might be related to the Xcode beta?

Would be great if we bump the tools version to 6.0 after Swift 6 is released.

Supereg added a commit to StanfordSpezi/.github that referenced this pull request Jul 11, 2024
# Allow to specify Swift version with xcodebuild

## ♻️ Current situation & Problem
This PR adds a new input to specify the swift version when running with
xcodebuild.


## ⚙️ Release Notes 
* Add new `swiftVersion` input to the xcodebuild-or-fastlane action.
* Specify more clearly that the `test` input only has an effect when
using xcodebuild.


## 📚 Documentation
--


## ✅ Testing
Verified in StanfordSpezi/Spezi#108 (both with
not specifying swift version and with specifying a swift version).


## 📝 Code of Conduct & Contributing Guidelines 

By submitting creating this pull request, you agree to follow our [Code
of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md):
- [x] I agree to follow the [Code of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md).
@Supereg Supereg merged commit fffb691 into main Jul 11, 2024
16 checks passed
@Supereg Supereg deleted the fix/swift-concurrency branch July 11, 2024 12:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Complete Concurrency Checking
2 participants