fix(iOS): Fix dSYM upload for main app in Expo EAS builds by adding inputPaths to build phase#5617
fix(iOS): Fix dSYM upload for main app in Expo EAS builds by adding inputPaths to build phase#5617
Conversation
…ehavior Add test suite to validate the current implementation of the Upload Debug Symbols to Sentry build phase: - Verifies build phase is created with correct shell script - Confirms inputPaths are not currently included - Ensures existing build phase is not recreated These tests establish a baseline before implementing the fix for issue #5288.
…ition Add inputPaths to the 'Upload Debug Symbols to Sentry' build phase to establish proper build dependencies. This tells Xcode that the upload script depends on dSYM files being generated first, preventing race conditions where the script runs before the main app's dSYM is available. The inputPaths include: - DWARF file path: Contains the actual debug symbols - dSYM folder path: The complete dSYM bundle This ensures proper build ordering in EAS builds and other CI environments where timing can cause the main app's dSYM to be unavailable during the build phase, while framework dSYMs are already present. Fixes #5288 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Add tests to verify that the Upload Debug Symbols build phase now correctly includes inputPaths for establishing build dependencies: - Verifies inputPaths array is present with correct length - Validates DWARF file path is included - Validates dSYM folder path is included These tests ensure the fix for #5288 works as expected.
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
|
@sentry review |
Android (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 1e7a472+dirty | 348.80 ms | 362.55 ms | 13.75 ms |
| 526494a+dirty | 422.80 ms | 438.90 ms | 16.10 ms |
| af9331b | 449.77 ms | 479.20 ms | 29.43 ms |
| 2b89ce9 | 413.69 ms | 442.58 ms | 28.89 ms |
| 1ef8a04+dirty | 415.16 ms | 415.22 ms | 0.06 ms |
| 3bd3f0d+dirty | 447.21 ms | 472.31 ms | 25.10 ms |
| b3b5b0d | 399.82 ms | 419.20 ms | 19.38 ms |
| f234eb4+dirty | 407.62 ms | 429.64 ms | 22.02 ms |
| 20daa0a | 359.51 ms | 374.90 ms | 15.39 ms |
| 266bc7e+dirty | 485.02 ms | 551.94 ms | 66.92 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 1e7a472+dirty | 17.75 MiB | 19.70 MiB | 1.96 MiB |
| 526494a+dirty | 43.75 MiB | 47.99 MiB | 4.24 MiB |
| af9331b | 17.75 MiB | 19.68 MiB | 1.94 MiB |
| 2b89ce9 | 17.75 MiB | 19.68 MiB | 1.94 MiB |
| 1ef8a04+dirty | 43.75 MiB | 48.05 MiB | 4.29 MiB |
| 3bd3f0d+dirty | 17.75 MiB | 19.70 MiB | 1.95 MiB |
| b3b5b0d | 17.75 MiB | 19.68 MiB | 1.94 MiB |
| f234eb4+dirty | 17.75 MiB | 19.74 MiB | 1.99 MiB |
| 20daa0a | 17.75 MiB | 20.15 MiB | 2.41 MiB |
| 266bc7e+dirty | 43.75 MiB | 47.99 MiB | 4.24 MiB |
Previous results on branch: antonis/ios-dsym
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3af9af5+dirty | 409.33 ms | 470.80 ms | 61.47 ms |
| 2b591dd+dirty | 405.68 ms | 440.26 ms | 34.58 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3af9af5+dirty | 43.75 MiB | 48.41 MiB | 4.66 MiB |
| 2b591dd+dirty | 43.75 MiB | 48.41 MiB | 4.66 MiB |
iOS (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 664c66f+dirty | 1195.94 ms | 1194.80 ms | -1.14 ms |
| 493c1a1+dirty | 1220.50 ms | 1221.30 ms | 0.80 ms |
| a3f3291+dirty | 1206.37 ms | 1208.44 ms | 2.08 ms |
| d1fd647+dirty | 1218.16 ms | 1225.82 ms | 7.65 ms |
| 1c38acd+dirty | 1214.43 ms | 1216.56 ms | 2.12 ms |
| bb4ea33+dirty | 1223.90 ms | 1217.83 ms | -6.06 ms |
| c94a927+dirty | 1211.33 ms | 1223.31 ms | 11.97 ms |
| 2b89ce9+dirty | 1241.19 ms | 1254.20 ms | 13.01 ms |
| 21c9e75+dirty | 1206.20 ms | 1223.54 ms | 17.35 ms |
| 64cd15c+dirty | 1213.50 ms | 1223.54 ms | 10.04 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 664c66f+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
| 493c1a1+dirty | 3.41 MiB | 4.58 MiB | 1.17 MiB |
| a3f3291+dirty | 3.41 MiB | 4.66 MiB | 1.25 MiB |
| d1fd647+dirty | 3.19 MiB | 4.56 MiB | 1.37 MiB |
| 1c38acd+dirty | 3.44 MiB | 4.67 MiB | 1.23 MiB |
| bb4ea33+dirty | 3.44 MiB | 4.59 MiB | 1.15 MiB |
| c94a927+dirty | 3.19 MiB | 4.56 MiB | 1.37 MiB |
| 2b89ce9+dirty | 3.19 MiB | 4.48 MiB | 1.29 MiB |
| 21c9e75+dirty | 3.19 MiB | 4.38 MiB | 1.19 MiB |
| 64cd15c+dirty | 3.19 MiB | 4.38 MiB | 1.19 MiB |
Previous results on branch: antonis/ios-dsym
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 2b591dd+dirty | 1216.28 ms | 1207.98 ms | -8.30 ms |
| 3af9af5+dirty | 1218.91 ms | 1221.09 ms | 2.18 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 2b591dd+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
| 3af9af5+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
iOS (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 664c66f+dirty | 1215.37 ms | 1221.30 ms | 5.92 ms |
| 493c1a1+dirty | 1207.58 ms | 1211.80 ms | 4.22 ms |
| a3f3291+dirty | 1219.54 ms | 1217.40 ms | -2.14 ms |
| d1fd647+dirty | 1219.35 ms | 1233.18 ms | 13.83 ms |
| 1c38acd+dirty | 1184.71 ms | 1190.31 ms | 5.59 ms |
| bb4ea33+dirty | 1208.98 ms | 1218.50 ms | 9.52 ms |
| c94a927+dirty | 1227.14 ms | 1239.64 ms | 12.50 ms |
| 2b89ce9+dirty | 1229.30 ms | 1239.40 ms | 10.10 ms |
| 21c9e75+dirty | 1237.78 ms | 1247.66 ms | 9.88 ms |
| 64cd15c+dirty | 1216.31 ms | 1214.04 ms | -2.26 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 664c66f+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
| 493c1a1+dirty | 3.41 MiB | 4.58 MiB | 1.17 MiB |
| a3f3291+dirty | 3.41 MiB | 4.66 MiB | 1.25 MiB |
| d1fd647+dirty | 2.63 MiB | 3.99 MiB | 1.36 MiB |
| 1c38acd+dirty | 3.44 MiB | 4.67 MiB | 1.23 MiB |
| bb4ea33+dirty | 3.44 MiB | 4.59 MiB | 1.15 MiB |
| c94a927+dirty | 2.63 MiB | 3.99 MiB | 1.36 MiB |
| 2b89ce9+dirty | 2.63 MiB | 3.91 MiB | 1.28 MiB |
| 21c9e75+dirty | 2.63 MiB | 3.81 MiB | 1.18 MiB |
| 64cd15c+dirty | 2.63 MiB | 3.81 MiB | 1.18 MiB |
Previous results on branch: antonis/ios-dsym
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 2b591dd+dirty | 1212.70 ms | 1223.00 ms | 10.30 ms |
| 3af9af5+dirty | 1212.94 ms | 1219.61 ms | 6.67 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 2b591dd+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
| 3af9af5+dirty | 3.38 MiB | 4.60 MiB | 1.22 MiB |
Android (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d1fd647+dirty | 374.46 ms | 409.51 ms | 35.05 ms |
| c9e95bd+dirty | 339.32 ms | 401.24 ms | 61.92 ms |
| 526494a+dirty | 361.10 ms | 410.84 ms | 49.74 ms |
| 3bd3f0d+dirty | 334.38 ms | 402.19 ms | 67.81 ms |
| 1ef8a04+dirty | 450.73 ms | 482.38 ms | 31.65 ms |
| 9f211e3+dirty | 371.00 ms | 432.51 ms | 61.51 ms |
| 266bc7e+dirty | 378.00 ms | 392.81 ms | 14.81 ms |
| ee69ed5+dirty | 411.19 ms | 447.04 ms | 35.85 ms |
| 8490686+dirty | 344.38 ms | 364.37 ms | 19.98 ms |
| cdf7e97+dirty | 389.79 ms | 418.13 ms | 28.34 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d1fd647+dirty | 7.15 MiB | 8.43 MiB | 1.28 MiB |
| c9e95bd+dirty | 7.15 MiB | 8.41 MiB | 1.26 MiB |
| 526494a+dirty | 43.94 MiB | 48.82 MiB | 4.88 MiB |
| 3bd3f0d+dirty | 7.15 MiB | 8.43 MiB | 1.28 MiB |
| 1ef8a04+dirty | 43.94 MiB | 48.87 MiB | 4.93 MiB |
| 9f211e3+dirty | 7.15 MiB | 8.41 MiB | 1.26 MiB |
| 266bc7e+dirty | 43.94 MiB | 48.82 MiB | 4.88 MiB |
| ee69ed5+dirty | 43.94 MiB | 48.87 MiB | 4.93 MiB |
| 8490686+dirty | 7.15 MiB | 8.43 MiB | 1.28 MiB |
| cdf7e97+dirty | 43.94 MiB | 49.22 MiB | 5.29 MiB |
Previous results on branch: antonis/ios-dsym
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3af9af5+dirty | 355.09 ms | 392.68 ms | 37.59 ms |
| 2b591dd+dirty | 385.98 ms | 448.84 ms | 62.86 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3af9af5+dirty | 43.94 MiB | 49.27 MiB | 5.33 MiB |
| 2b591dd+dirty | 43.94 MiB | 49.27 MiB | 5.33 MiB |
…tion The previous fix (8064fc2) added inputPaths without escaped quotes, which caused CocoaPods to fail with a parse error. Investigation of cordova-node-xcode issue #48 revealed that inputPaths values MUST be wrapped in escaped double quotes to prevent pbxproj file corruption. Without quotes: inputPaths: ['$(PATH)'] ❌ Corrupts file With quotes: inputPaths: ['" $(PATH)"'] ✅ Works correctly This commit supersedes the previous implementation and provides the correct solution for establishing build dependencies while avoiding serialization issues. Updated tests to validate inputPaths use proper escaping. Ref: apache/cordova-node-xcode#48
| expect(addBuildPhaseSpy).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('includes inputPaths with escaped quotes to avoid cordova-node-xcode serialization bug', () => { |
There was a problem hiding this comment.
Q: Why cordova-node-xcode and what bug is it being avoided?
There was a problem hiding this comment.
Good catch @lucas-zimerman 👍
We are actually not using cordova-node-xcode directly but we use @expo/config-plugins that depends on it.
The issue was breaking the build and the fix was to add escape quotes.
I've updated the test description with 987f21b to make this less confusing.
| inputPaths: [ | ||
| '"$(DWARF_DSYM_FOLDER_PATH)/$(DWARF_DSYM_FILE_NAME)/Contents/Resources/DWARF/$(PRODUCT_NAME)"', | ||
| '"$(DWARF_DSYM_FOLDER_PATH)/$(DWARF_DSYM_FILE_NAME)"', | ||
| ], |
There was a problem hiding this comment.
magical 2 lines fix :D
| const expectedShellScript = | ||
| "/bin/sh `${NODE_BINARY:-node} --print \"require('path').dirname(require.resolve('@sentry/react-native/package.json')) + '/scripts/sentry-xcode-debug-files.sh'\"`"; |
There was a problem hiding this comment.
This const parameter seems to be used on all tests from Upload Debug Symbols to Sentry build phase.
Would it be possible to move them outside it test but within the describe block?
| const callArgs = addBuildPhaseSpy.mock.calls[0]; | ||
| const options = callArgs[4]; |
There was a problem hiding this comment.
We don't really need callArgs, all added tests seems to be using the same code:
const callArgs = addBuildPhaseSpy.mock.calls[0];
const options = callArgs[4];
We could transform this block on a function that receives the spy and return the options. What do you think?
EDIT: We don't need to pass addBuildPhaseSpy since it's inside the describe block, we can simply get it from there.
lucas-zimerman
left a comment
There was a problem hiding this comment.
Thank you for the PR! I left a few suggestions on the tests but overall it's looking good!
📢 Type of change
📜 Description
💡 Motivation and Context
Fixes #5288
💚 How did you test it?
CI, Manual
📝 Checklist
sendDefaultPIIis enabled🔮 Next steps