Skip to content
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

Temporary fix for keytar on Apple Silicon #11969

Closed

Conversation

dennisameling
Copy link
Contributor

@dennisameling dennisameling commented Apr 8, 2021

Related to #9691 (comment)

There are issues with Electron arm64 builds on Apple Silicon when using keytar. Related issue: atom/node-keytar#346.

Keytar will (likely quite soon) switch to N-API prebuilds which eliminates the needs for them to publish prebuilds for every Node + Electron version (currently at 92 (!) prebuilds https://github.com/atom/node-keytar/releases/tag/v7.6.0), see atom/node-keytar#331. I've created a temporary Keytar release based on that PR: https://www.npmjs.com/package/@dennisameling/keytar-temp

The alternatives to merging this PR are:

A release for testing on Apple Silicon can be found here: https://github.com/dennisameling/desktop/releases/tag/2.7.3-test

@trulyronak
Copy link

trulyronak commented Apr 8, 2021

Screen Shot 2021-04-08 at 1 36 38 PM

Can't open it (m1 mac)

I downloaded it from releases

@trulyronak
Copy link

Following steps here, I ran xattr -cr GitHub\ Desktop.app, and got a new error:

Screen Shot 2021-04-08 at 1 41 01 PM

@billygriffin
Copy link
Contributor

@trulyronak Thanks! Based on @dennisameling's response here (#9691 (comment)), I think the fs admin error is expected.

@sergiou87
Copy link
Member

For those interested, I uploaded a test build that fixes both keytar (with the changes from this PR) and fs-admin issues (by including my changes in atom/fs-admin#105 and atom/fs-admin#106)!

Please, give it a try and let me know if it works for you. It definitely does for me!

Download: https://s3.amazonaws.com/github-desktop/releases/2.7.3-test4-0e14ef24/GitHubDesktop-arm64.zip

@jensenharris
Copy link

For those interested, I uploaded a test build that fixes both keytar (with the changes from this PR) and fs-admin issues (by including my changes in atom/fs-admin#105 and atom/fs-admin#106)!

Please, give it a try and let me know if it works for you. It definitely does for me!

Download: https://s3.amazonaws.com/github-desktop/releases/2.7.3-test4-0e14ef24/GitHubDesktop-arm64.zip

Thanks for creating this release!

It almost works on macOS 11.3 for me, in that it launches but there's an immediate error:

Failed to execute installGlobalLFSFilter: Unknown system error -86

From this point, the app is nonfunctional. The UI shows up, but when I try to add a repo, for instance, the Add Repo button never gets activated so I cannot continue.

@jensenharris
Copy link

It looks like -86 maps to Bad CPU type in executable

The Apple Silicon machine I'm testing this on does not have Rosetta 2 installed.

@sergiou87 is it possible that there's still some singular Intel dependency in your package and Desktop is trying to run something in the background that requires Intel emulation (and which fails without Rosetta 2 installed?)

@spacentropy
Copy link

@jensenharris @sergiou87 it seems that the git bundled with Github Desktop in the provided build is x86_64.
image
The app itself works, thanks!

@sergiou87
Copy link
Member

Ouch, great catch! I'll look into that, thank you!! ❤️

@jensenharris
Copy link

Ouch, great catch! I'll look into that, thank you!! ❤️

Thank you! I'm looking forward to the updated Apple Silicon build!

@sergiou87
Copy link
Member

Thankfully this PR is not needed anymore (thank you for all your work @dennisameling!! ❤️ ), and the last beta is already working on Apple Silicon devices 😄

It still has git binaries for x86_64, but I almost have sorted that out, stay tuned… 😄

@sergiou87 sergiou87 closed this Apr 28, 2021
@sergiou87
Copy link
Member

sergiou87 commented Apr 28, 2021

@jensenharris @spacentropy could you try this build out? [Link removed because it wasn't 100% arm64 yet, sorry!]

Specially if you don't have Rosetta… git and git-lfs are built for arm64 in that release, so hopefully there is nothing else missing!! 🤞

@sergiou87
Copy link
Member

I noticed two binaries escaped the upgrade to arm64. I took care of them here: #12091 and will upload a new test build soon. Sorry about that!

@sergiou87
Copy link
Member

sergiou87 commented Apr 29, 2021

Here we go, new arm64 build that should be 100% arm64: [Link removed because now it's in beta!] 🤞

EDIT: You can now download the arm64 beta from https://central.github.com/deployments/desktop/desktop/latest/darwin-arm64?env=beta

@n13
Copy link

n13 commented May 21, 2021

Here we go, new arm64 build that should be 100% arm64: [Link removed because now it's in beta!] 🤞

EDIT: You can now download the arm64 beta from https://central.github.com/deployments/desktop/desktop/latest/darwin-arm64?env=beta

This is sooo awesome!! It works!! M1 baby! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants