-
-
Notifications
You must be signed in to change notification settings - Fork 372
fix: Fix stripped categories in static frameworks #6125
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
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6125 +/- ##
========================================
Coverage ? 86.658%
========================================
Files ? 435
Lines ? 36983
Branches ? 17393
========================================
Hits ? 32049
Misses ? 4890
Partials ? 44
Continue to review full report in Codecov by Sentry.
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| f262e10 | 1226.88 ms | 1250.39 ms | 23.51 ms |
| 9450cb4 | 1221.29 ms | 1232.96 ms | 11.67 ms |
| 76f74df | 1238.29 ms | 1261.22 ms | 22.94 ms |
| a3dfd57 | 1230.78 ms | 1244.91 ms | 14.14 ms |
| 2a7868a | 1226.54 ms | 1256.92 ms | 30.37 ms |
| 9e6569a | 1216.07 ms | 1242.50 ms | 26.43 ms |
| 924de23 | 1222.84 ms | 1248.37 ms | 25.54 ms |
| 891fd1d | 1220.02 ms | 1227.60 ms | 7.57 ms |
| 50a9ff7 | 1221.43 ms | 1238.07 ms | 16.64 ms |
| 2609f7a | 1218.17 ms | 1241.34 ms | 23.17 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| f262e10 | 23.75 KiB | 913.62 KiB | 889.87 KiB |
| 9450cb4 | 23.75 KiB | 908.55 KiB | 884.80 KiB |
| 76f74df | 23.75 KiB | 879.61 KiB | 855.86 KiB |
| a3dfd57 | 23.75 KiB | 913.63 KiB | 889.87 KiB |
| 2a7868a | 23.75 KiB | 880.20 KiB | 856.45 KiB |
| 9e6569a | 23.75 KiB | 904.54 KiB | 880.79 KiB |
| 924de23 | 23.75 KiB | 947.54 KiB | 923.79 KiB |
| 891fd1d | 23.75 KiB | 919.92 KiB | 896.17 KiB |
| 50a9ff7 | 23.75 KiB | 913.63 KiB | 889.89 KiB |
| 2609f7a | 23.75 KiB | 867.04 KiB | 843.29 KiB |
Previous results on branch: fixStaticFrameworks
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d1e459c | 1243.37 ms | 1272.83 ms | 29.47 ms |
| 2d22a60 | 1227.96 ms | 1254.65 ms | 26.69 ms |
| 932cbbd | 1233.27 ms | 1258.42 ms | 25.15 ms |
| 04c83d0 | 1222.96 ms | 1252.37 ms | 29.41 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d1e459c | 23.75 KiB | 969.86 KiB | 946.12 KiB |
| 2d22a60 | 23.75 KiB | 969.86 KiB | 946.11 KiB |
| 932cbbd | 23.75 KiB | 969.78 KiB | 946.03 KiB |
| 04c83d0 | 23.75 KiB | 969.86 KiB | 946.11 KiB |
ee5dfd9 to
b845ffc
Compare
b845ffc to
99f6467
Compare
Yes indeed. |
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.
LGTM, when CI is green. If this crash happens frequently, we should consider adding an entry to the troubleshooting.
99f6467 to
0d5eaee
Compare
|
Thanks @philipphofmann - Since it's only in an alpha and reported by just 1 user I'm thinking it would add more confusion that it solves to put it in the troubleshooting |
bdbfe84 to
9fdf8cc
Compare
This fixes a crash when using Sentry as a static framework. The linker will try to remove these @objc extensions in static frameworks because they are in a file with no other used classes. To avoid this we need to put something else in these files that get referenced from another part of the codebase. This isn't a problem for Swift, but is for objc that uses
performSelectorThis made me realize we don't have a CI test for sentry being used as a static framework. We should add one, but it's a bit tricky to do without SPM, that's why this is still a WIP
#skip-changelog
Closes #6134