-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[HOLD for payment 2022-12-15] DEV - building desktop staging locally on a M1 mac crashes App at launch #12930
Comments
Triggered auto assignment to @johncschuster ( |
From Slack, it looks like these are @kidroca's repro steps: Steps:
|
Triggered auto assignment to @danieldoglas ( |
@danieldoglas, I have an M1, but I'm not 100% sure I know what to expect to see if I were to run these steps and get a success or failure. Can you try following @kidroca's steps here to see if you can reproduce the reported behavior? |
I can capture a video |
@johncschuster, @danieldoglas Huh... This is 4 days overdue. Who can take care of this? |
Looking into this today. |
Interesting, even considering I am a developer and I have apple certificates, it also breaks for me. |
just adding Kidroca`s comment here.
|
Yeah, I think this is the related electron issue This is what I've found out so far: We're using App/config/electronBuilder.config.js Line 36 in a3e8db6
https://developer.apple.com/documentation/security/hardened_runtime
I think the fix I shared earlier electron/electron#35355 (comment) is valid because:
Desktop is using Electron, and Electron works like a browser so it needs to have the JIT entitlement enabled <key>com.apple.security.cs.allow-jit</key>
<true/> Actually even if
Another point of view is "Why then the Desktop app works when it's downloaded from web?" I guess an alternative fix would be to set the target as x86 explicitly |
@kidroca thanks for further explaining. I think we can apply that solution, since I was able to simulate it as well. Do you want to bring up a PR? |
Yes, though I can submit a PR tomorrow, is that OK? |
Yep, no problem! |
For the protocol - this is the content of the crash report, before applying the fix M1 Crash Report
|
Closing this |
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.2.36-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2022-12-15. 🎊 After the hold period, please check if any of the following need payment for this issue, and if so check them off after paying:
As a reminder, here are the bonuses/penalties that should be applied for any External issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
@johncschuster I was C+ reviewer in the PR of this issue. I guess payment will be made through this issue. Also, since it was merged within 3 business days, is it eligible for bonus?
|
|
sorry for the delay @sobitneupane, I'll check that out and answer you! |
Hi @sobitneupane! I'm so sorry for the delay on this! Can you apply to the job in Upwork? I'll get payment issued right away! |
@johncschuster Thanks. Submitted proposal. |
Great! I've just sent the offer. Let me know when you accept so I can knock out the payment. |
@johncschuster Accepted offer. |
Paid! 🎉 |
Thanks @johncschuster |
@danieldoglas Does this issue require regression steps? I'm thinking "no" because it doesn't have to do with new.expensify.com, but I'm willing to be wrong! |
@johncschuster i don't think so. We can close this! |
Closing this since I think we just missed closing it. |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Action Performed:
Expected Result:
App should not crash
Actual Result:
App crashed
Workaround:
unknown
Platform:
Where is this issue occurring?
Version Number:
Reproducible in staging?: Needs reproduction
Reproducible in production?: Needs reproduction
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos:
Expensify/Expensify Issue URL:
Issue reported by: @kidroca
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1669051761252729
View all open jobs on GitHub
The text was updated successfully, but these errors were encountered: