-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Add FXIOS-14009 Add binary symbol stripping build phase script for Firefox configuration #30361
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
base: main
Are you sure you want to change the base?
Add FXIOS-14009 Add binary symbol stripping build phase script for Firefox configuration #30361
Conversation
|
@NicoHinderling is this usual way for iOS apps to not ship symbols to devices? The shell script makes it seems like it's pretty involved. |
🧹 Tidy commitJust 1 file(s) touched. Thanks for keeping it clean and review-friendly! 💬 Description craftsmanGreat PR description! Reviewers salute you 🫡 ✅ Per-file coverageAll changed files meet the threshold of 35.0%. Generated by 🚫 Danger Swift against 6db8473 |
0103d95 to
1d3ccf3
Compare
Hey @jrmuizel thanks for the quick review! There are definitely other ways that it can be done. I think the main other approach is to just run With a bit of testing, this other approach should be able to net an equivalent size reduction as the script. I just wanted to share the script first since it's something I could easily make a PR for at the very least :)
|
|
@NicoHinderling thanks for creating this PR 🙏 |
| inputFileListPaths = ( | ||
| ); | ||
| inputPaths = ( | ||
| "${DWARF_DSYM_FOLDER_PATH}/${DWARF_DSYM_FILE_NAME}/Contents/Resources/DWARF/${EXECUTABLE_NAME}", |
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.
This seems to be the path for a different "Run Script" phase (not the newly added Strip Symbols one you added). Could you please add it to the correct build phase?
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.
woops! 🙈 good catch
yes definitely. I accidentally placed the input file on the wrong script but that's now resolved. As for the input file path, I work with the main author of that blog post and synced up with them. Apparently |
📜 Tickets
Jira ticket
Github issue
💡 Description
While analyzing the Firefox app, we noticed that binary symbols are currently always included. This is not a requirement for production if we separately upload the dSYMs to a crash reporter, and stripping these symbols can ultimately net a 50mb install size reduction for production users!
🎥 Demos
(TODO: update PR description with before and after info about the size changes)
📝 Checklist