-
-
Notifications
You must be signed in to change notification settings - Fork 372
meta: Enable interoperability mode for Swift 5.9 #5823
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
meta: Enable interoperability mode for Swift 5.9 #5823
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5823 +/- ##
========================================
Coverage ? 86.714%
========================================
Files ? 423
Lines ? 36611
Branches ? 17347
========================================
Hits ? 31747
Misses ? 4819
Partials ? 45 Continue to review full report in Codecov by Sentry.
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 65ebbdb | 1233.96 ms | 1255.79 ms | 21.83 ms |
| 8d944ac | 1236.92 ms | 1254.91 ms | 18.00 ms |
| 9be5373 | 1215.92 ms | 1239.44 ms | 23.52 ms |
| 5196f0d | 1213.35 ms | 1231.37 ms | 18.02 ms |
| 64c2b2b | 1233.96 ms | 1260.20 ms | 26.24 ms |
| 2a7868a | 1226.54 ms | 1256.92 ms | 30.37 ms |
| ebc72be | 1221.24 ms | 1249.66 ms | 28.42 ms |
| 9e6569a | 1216.07 ms | 1242.50 ms | 26.43 ms |
| 52f3b6e | 1219.57 ms | 1239.70 ms | 20.13 ms |
| cd9727b | 1236.04 ms | 1254.41 ms | 18.37 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 65ebbdb | 23.75 KiB | 902.33 KiB | 878.59 KiB |
| 8d944ac | 23.75 KiB | 919.69 KiB | 895.94 KiB |
| 9be5373 | 23.75 KiB | 866.50 KiB | 842.75 KiB |
| 5196f0d | 23.75 KiB | 876.93 KiB | 853.19 KiB |
| 64c2b2b | 23.75 KiB | 908.55 KiB | 884.80 KiB |
| 2a7868a | 23.75 KiB | 880.20 KiB | 856.45 KiB |
| ebc72be | 23.75 KiB | 908.22 KiB | 884.47 KiB |
| 9e6569a | 23.75 KiB | 904.54 KiB | 880.79 KiB |
| 52f3b6e | 23.75 KiB | 920.54 KiB | 896.79 KiB |
| cd9727b | 23.75 KiB | 879.25 KiB | 855.51 KiB |
Previous results on branch: itay/cocoa-483-unable-to-build-with-xcode-26-for-visionos
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 0be2621 | 1230.81 ms | 1250.80 ms | 19.98 ms |
| eaea273 | 1232.87 ms | 1250.43 ms | 17.55 ms |
| d52e85f | 1212.76 ms | 1239.94 ms | 27.18 ms |
| f034e79 | 1227.37 ms | 1252.53 ms | 25.16 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 0be2621 | 23.75 KiB | 913.63 KiB | 889.87 KiB |
| eaea273 | 23.75 KiB | 913.63 KiB | 889.88 KiB |
| d52e85f | 23.75 KiB | 913.09 KiB | 889.34 KiB |
| f034e79 | 23.75 KiB | 913.08 KiB | 889.33 KiB |
29fd986 to
77f04f9
Compare
philipphofmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We must do the same for the new Package file
sentry-cocoa/.github/workflows/release.yml
Lines 134 to 139 in 48e5c8a
| - name: Change path of the framework | |
| run: | | |
| sed -i '' 's/url.*//g' Package.swift | |
| sed -i '' 's/checksum: ".*" \/\/Sentry-Static/path: "Sentry.xcframework.zip"/g' Package.swift | |
| sed -i '' 's/checksum: ".*" \/\/Sentry-Dynamic/path: "Sentry-Dynamic.xcframework.zip"/g' Package.swift | |
| shell: bash |
We must trigger an alpha release after merging this to ensure this doesn't break the release, please.
Also, I see a high risk that both Package files get out of sync. Do you have any ideas how we can avoid this?
Unfortunately not many, I tried extracting the code to a common file, but there is no way to import / use them :( |
philipphofmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately not many, I tried extracting the code to a common file, but there is no way to import / use them :(
@itay, what we could do is to store the current git diff between those two files in the repo by using
git diff --no-index Package.swift Package@swift-5.9.swift > expected.patch
And then we could validate that the files don't change with something like in CI
git diff --no-index Package.swift Package@swift-5.9.swift > current.patch
diff expected.patch current.patch
If we do this, I think we should also add a make command that updates the patch for you and the error message in CI should reference this command.
Up to you if you believe it's worth the effort. Or we find some way to validate both package files by using SPM. That would actually be better cause it's an integration test. Or we could also do both. Patch and integration test. Up to you to decice.
Implemented the linter here: #5854 |
philprime
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, approving this with unresolved conversations. I believe we should merge #5839 either before (if possible) or ASAP after this PR.
5d61a7d to
19a250a
Compare
fee877c to
2c9251c
Compare
Changes the GCC C language to compiler default.
Aims to fix: #5667