-
Notifications
You must be signed in to change notification settings - Fork 152
GRDB compatiblity with Swift 6.0 #964
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
Conversation
Hello, is there anything I should do for having this PR reviewed and merged? |
@groue no my apologies on the late review! 👁️ |
@swift-ci test |
@groue Was this able to build locally for you using a recent compiler? |
Hello @justice-adams-apple,
Yes, I can build the specified commit Xcode 16.0 ✅
Xcode 16.1 ✅
Xcode 16.2 ✅
Linux builds ✅ I don't know which version specifically, but this PR was recently merged, and at the time we were already relying on the sendability of |
My interpretation of the failure is that the toolchain used by the compatibility suite is not identical to the released toolchains. Since Swift 6 comes with strict concurrency checkings, so it is crucial that the compatibility suite runs with the exact same concurrency annotations as the released toolchains (stdlib, Foundation, Dispatch, etc.) Any difference in |
I hope I haven't uncovered a problem that can not be addressed in this repository. If I have, what is my best option for successfully have this pull request merged? I could perform changes in the GRDB repo, in order to make it compatible with the particular toolchain that is used here, for example? I could add the following line to GRDB: // DispatchQoS already conforms to Sendable, except
// in swiftlang/swift-source-compat-suite, which needs
// this extra conformance.
extension DispatchQoS: @retroactive Sendable { } Still, I'm sorry to repeat myself, but I find it concerning that the compatibility suite does not use the same toolchain as the released one. This weakens the "compatibility" that is tested. Given the sensitivity of Swift 6 to concurrency annotations, the compatibility we need involves the compiler, the standard lib, and core libraries as well (Foundation, Dispatch, Synchronization, Observation, etc.) |
We use nightly toolchains so that we can catch issues before release as opposed to released toolchains which have already shipped
Yeah it's probably best to make the change in the repo because eventually when this compiler is released it would break your project. Sorry for the confusion, let me know if I can help out/answer anything else |
? I don't get it. Isn't it exactly what the compatibility suite is supposed to prevent? In the particular case we're discussing, I find it hard to believe that I was instead expecting an answer that would read like:
|
This makes sense when looking for regressions (such as the loss of the A build failure with a nightly toolchain reveals a regression in the nightly toolchain, which is exactly the purpose of this repository. It should not prevent the merge of a pull request like this one, which submits code that compiles with multiple released compilers. I don't understand why this pull request is not already merged. I question the methodology in place. |
No, if there is a known source compatibility failure caused by a compiler change, it should be fixed in the compiler. There are cases where a new failure is deliberate, such as when miscompiles are fixed, but this is not one of them. I'll investigate the failures here and resolve them. I agree that GRDB should absolutely be part of the source compatibility suite. Thank you @groue for contributing it. I'll follow up here once I've resolved the other source compatibility issues, likely tomorrow. |
Thank you very much, for investigating the failure, and for the reminder of the purpose of this repository 👍 |
To merge this pull request, we need to xfail the project specifically the Swift 6.0 configuration with GitHub Issues tracking the bug. Once the issue is fixed, the suite will automatically mark it as an unexpected pass.
More detail on how to xfail a configuration: https://github.com/swiftlang/swift-source-compat-suite?tab=readme-ov-file#marking-actions-as-expected-failures |
1 similar comment
The The remaining failures are all related to
Which is the known issue reported in swiftlang/swift#78635. |
I pushed a new GRDB commit, for version 7.3.0 instead of the beta version that was initially submitted. The I have no reference for the DispatchQoS issue, so I did not add any "xfail" entry for it. @shahmishal, please feel free to push a correct "xfail" entry to my branch (you have the right to do so), or to suggest a correct "xfail" entry in the GitHub PR review interface, so that this pull request can be merged. The documentation you linked to refers to many internal details I'm not aware of. You'll be much more efficient than I can. Thank you in advance. |
GRDB passed in the 6.0 configuration on the latest run! https://ci.swift.org/job/swift-PR-source-compat-suite-test-macOS/335/artifact/swift-source-compat-suite/PASS_GRDB.swift_6.0_BuildSwiftPackage.log |
Merging this PR based on PASS_GRDB.swift_6.0_BuildSwiftPackage.log |
Thank you |
Pull Request Description
Hello, this pull request adds compatibility check for GRDB with Swift 6.0.
Acceptance Criteria
To be accepted into the Swift source compatibility test suite, a project must:
./project_precommit_check
script runThe last check does not pass, due to the lack of support for Swift 6.0 in
project_precommit_check
(see #963).