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

Support deploying arm64 builds #11683

Merged
merged 11 commits into from Apr 8, 2021
Merged

Support deploying arm64 builds #11683

merged 11 commits into from Apr 8, 2021

Conversation

sergiou87
Copy link
Member

Description

This PR prepares our build scripts to deploy arm64 binaries to Central. Depends on github/central#500

Changes:

  • Add the new builds to the README file. macOS only for now.
  • Add new checks for the current architecture, trying to guess the best build we should try to update to (e.g. if we're running the macOS on Rosetta, we know we should try to update to the arm64 binary).
  • New S3 keys to create a subfolder for each architecture.
  • Take into account the new contexts created (darwin-arm64 and win32-arm64).

Release notes

Notes: no-notes

README.md Outdated
@@ -27,6 +28,7 @@ Want to test out new features and get fixes before everyone else? Install the
beta channel to get access to early builds of Desktop:

- [macOS](https://central.github.com/deployments/desktop/desktop/latest/darwin?env=beta)
- [macOS (M1 Chip)](https://central.github.com/deployments/desktop/desktop/latest/darwin-arm64?env=beta)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- [macOS (M1 Chip)](https://central.github.com/deployments/desktop/desktop/latest/darwin-arm64?env=beta)
- [macOS (Apple Silicon)](https://central.github.com/deployments/desktop/desktop/latest/darwin-arm64?env=beta)

I tend to think this is the "best way" to highlight these things, I think a future Mac could very well be shipping with an M2 or M1X (or so my sources say)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point! I chose M1 because that's how Apple showcases it and I don't see any references to Apple Silicon, but I agree it's probably more accurate and future-proof 😄 Thanks!!

korakit171129
korakit171129 previously approved these changes Mar 22, 2021
@rstolpe
Copy link

rstolpe commented Apr 2, 2021

Question, if m1 user already has PS (intel) installed trough brew, will this (PS m1 support) uninstall and install m1 version when they update?

If not, then I think that's a good ide to have.

@sergiou87 sergiou87 dismissed a stale review via 7621c25 April 6, 2021 15:11
@sergiou87 sergiou87 marked this pull request as ready for review April 6, 2021 15:37
@sergiou87 sergiou87 dismissed a stale review via baa5fe1 April 8, 2021 14:22
@sergiou87 sergiou87 requested a review from tidy-dev April 8, 2021 14:22
@@ -93,6 +93,7 @@ jobs:
- name: Publish production app
run: yarn run publish
env:
npm_config_arch: ${{ matrix.arch }}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what's used to tell our scripts about the target architecture.

Comment on lines +169 to +176
if (
enableUpdateFromRosettaToARM64() &&
remote.app.runningUnderRosettaTranslation === true
) {
const url = new URL(updatesURL)
url.searchParams.set('architecture', 'arm64')
updatesURL = url.toString()
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed because __UPDATES_URL__ is set at compile time, so we can't set it to look for arm64 for users who downloaded the x64 build to then run it on an Apple Silicon machine.

Copy link
Contributor

@tidy-dev tidy-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me. ✨

@sergiou87 sergiou87 merged commit 6a42c3d into development Apr 8, 2021
@sergiou87 sergiou87 deleted the publish-arm64 branch April 8, 2021 14:41
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

6 participants