Skip to content

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
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
@rwidmark
Copy link

rwidmark 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 stale reviews from ghost and korakit171129 via 7248295 April 6, 2021 11:56
@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
- 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.

7 participants