Create DMGs for x64 and arm64 builds #135

wojtekn · 2024-05-15T11:13:24Z

Related to https://github.com/Automattic/dotcom-forge/issues/6949

The PR was initially opened by @p-jackson : https://github.com/Automattic/local-environment/pull/266

Electron forge has a built in ability to create DMGs. But when I try and use it to create a DMG for the Intel build on my M1 mac there's an error.
I'm trying a different approach where I don't use electron forge, but instead invoke electron-installer-dmg directly, which is the package used under the hood by electron forge.

Seems to do the trick, but I need

Confirm that the DMGs themselves don't have to be signed in some way
That a DMG built on M1 can still be opened on an Intel (I assume the DMG format is the same for both architectures)

Proposed Changes

~~Depend directly on electron-installer-dmg~~
Use create-dmg CLI tool to create DMG files
Add make:dmg-arm64, make:dmg-x64, and make:dmg-universal scripts to create DMGs
Notarize DMG files
Update buildkite pipeline so that dev builds also generate DMGs for testing.

Testing Instructions

I don't think it's possible to test this on buildkite now before merging. It's been restricted to only build from trunk, so I can't sneakily have it run other branches. But it is testable locally.

`arm64`

npm install
npm run make:macos-arm64
npm run make:dmg-arm64
Checkout the DMG at out/Studio-darwin-arm64.dmg

`x64`

npm install
npm run make:macos-x64
npm rebuild fs-xattr --cpu universal
npm run make:dmg-x64
Checkout the DMG at out/Studio-darwin-x64.dmg

`universal`

npm install
npm run make:macos-universal
npm rebuild fs-xattr --cpu universal
npm run make:dmg-universal
Checkout the DMG at out/Studio-darwin-universal.dmg

The same should work for all platforms: arm64, x64 and universal.

Pre-merge Checklist

Have you checked for TypeScript, React or other console errors?

wojtekn · 2024-05-15T11:57:56Z

I brought the PR originally opened by @p-jackson to this repository, rebased it, solved conflicts, and tested it locally.

I can build all three Mac binaries, but createDMG still fails for Intel and Universal.

wojtekn · 2024-05-15T12:10:40Z

.buildkite/pipeline.yml

@@ -42,6 +42,7 @@ steps:
          .buildkite/commands/install-node-dependencies.sh
          node ./scripts/prepare-dev-build-version.mjs
          npm run make:macos-{{matrix}}
+          npm run make:dmg-{{matrix}}


Should it happen after we notarize the binary?

Here's what the docs say:

Alternatively, you can put apps, kernel extensions, and other software in a container, like a disk image, and notarize the container. The notary service accepts disk images (UDIF format), signed flat installer packages, and ZIP archives. It processes nested containers as well, like packages inside a disk image.

Important: If you distribute your software via a custom third-party installer, you need two rounds of notarization. First you notarize the installer’s payload (everything the installer will install). You then package the notarized (and stapled, as described in Staple the ticket to your distribution) items into the installer and notarize it as you would any other executable. If you use a network installer, separately notarize both the installer and the items it downloads.

So, I think in our case, given we distribute both .app and .dmg, we'll want to notarize both. If we were only distributing the DMG, we could notarize just that and trust Apple's tooling will handle it.

I tried a local notarization of a DMG built from this branch, as of d538a17, and this diff worked:

--- a/fastlane/Fastfile +++ b/fastlane/Fastfile @@ -45,13 +45,20 @@ lane :set_up_signing do |_options| end desc 'Notarize the compiled binary' -lane :notarize_binary do |_options| +lane :notarize_binary do Dir[File.join(BUILDS_FOLDER, '**', 'Studio.app')].each do |path| notarize( package: path, api_key_path: APPLE_API_KEY_PATH ) end + Dir[File.join(BUILDS_FOLDER, '**', 'Studio-*.dmg')].each do |path| + notarize( + bundle_id: APPLE_BUNDLE_IDENTIFIER, + package: path, + api_key_path: APPLE_API_KEY_PATH + ) + end end desc 'Ship the binary to internal testers'

wojtekn · 2024-05-15T15:53:05Z

The error I'm getting:

Error: dlopen(/Volumes/Sites/local-environment/node_modules/fs-xattr/build/Release/xattr.node, 0x0001): tried: '/Volumes/Sites/local-environment/node_modules/fs-xattr/build/Release/xattr.node' (mach-o file, but is an incompatible architecture (have 'x86_64', need 'arm64e' or 'arm64')), '/System/Volumes/Preboot/Cryptexes/OS/Volumes/Sites/local-environment/node_modules/fs-xattr/build/Release/xattr.node' (no such file), '/Volumes/Sites/local-environment/node_modules/fs-xattr/build/Release/xattr.node' (mach-o file, but is an incompatible architecture (have 'x86_64', need 'arm64e' or 'arm64'))
    at Module._extensions..node (node:internal/modules/cjs/loader:1327:18)
    at Module.load (node:internal/modules/cjs/loader:1091:32)
    at Module._load (node:internal/modules/cjs/loader:938:12)
    at Module.require (node:internal/modules/cjs/loader:1115:19)
    at require (node:internal/modules/helpers:130:18)
    at Object.<anonymous> (/Volumes/Sites/local-environment/node_modules/fs-xattr/index.js:3:15)
    at Module._compile (node:internal/modules/cjs/loader:1241:14)
    at Module._extensions..js (node:internal/modules/cjs/loader:1295:10)
    at Module.load (node:internal/modules/cjs/loader:1091:32)
    at Module._load (node:internal/modules/cjs/loader:938:12) {
  code: 'ERR_DLOPEN_FAILED'
}

As @p-jackson explained under https://github.com/Automattic/local-environment/pull/266#issuecomment-2058532243, fs-xattrs couldn't load xattr.node file for arm64 and file for x64 was available there.

During some debugging I tried today, it started working on my local. After reverting changes in the package.json and node_modules/ code to the initial state, I couldn't reproduce it. Now it doesn't work again. 🤷

wojtekn · 2024-05-15T16:21:00Z

Worth noting, there is a newer version of the fs-xattr and it includes some refactoring:

https://github.com/LinusU/fs-xattr/releases/tag/v0.4.0

Also, appdmg uses it only to set one extended attribute:

util.callbackify(() => xattr.set(path, 'com.apple.FinderInfo', buf))(cb)

mokagio · 2024-05-16T10:49:35Z

fastlane/Fastfile

+      filename_core: 'darwin-universal',
+      extension: 'dmg',
+      name: 'Mac Universal (DMG)'
+    },


Glad the DRY from #117 turned out to be useful. I wasn't expecting us to add new files to upload, but there you go. Happy it was here.

wojtekn · 2024-05-16T11:40:42Z

I tried using a CLI tool for DMG, and it works on local for all cases. However, the Buildkite pipeline fails with:

Error: Command failed: create-dmg --volname Studio.app --volicon /opt/ci/builds/builder/automattic/studio/assets/studio-app-icon.icns --window-size 710 502 --background /opt/ci/builds/builder/automattic/studio/assets/dmg-background.png --icon Studio 533 122 --icon-size 80 --app-drop-link 533 354 /opt/ci/builds/builder/automattic/studio/out/Studio-darwin-arm64.dmg /opt/ci/builds/builder/automattic/studio/out/Studio-darwin-arm64/Studio.app
--
  | Searching for mounted interstitial disk image using /dev/disk5s...
  | /var/folders/cv/v953tj997c95mt4pl1k9lqzw0000gn/T/createdmg.tmp.XXXXXXXXXX.kfx2Aguk:394:406: execution error: Finder got an error: AppleEvent timed out. (-1712)
  | Failed running AppleScript

It may be similar to create-dmg/create-dmg#72

wojtekn · 2024-05-21T08:26:44Z

I reported the initial issue under electron/forge#3604.

mokagio · 2024-06-03T09:57:42Z

Gotta smile... the PR changes the DMG logic, and the Windows build fails 🙃

I've seen this error already in another app.

The workaround there was to change the versioning style in prepare-dev-build-version.mjs , which was originally copied from this repo.

// Usually, dev builds have major.minor.patch-dev.suffix .
//
// const devVersion = `${ packageJson.version.split( '-' )[ 0 ] }-dev.${ currentCommitShort }`;
//
// However, we're getting build failures (only in Windows, it seems) in parsing the last component separated by a '.'
// As such, let's try adding a '-' instead.
//
// See [redacted]
//
// Interesting, also, we seem to be getting the error only for commits that begin with letters, not numbers.
// See [redacted]
const devVersion = `${ packageJson.version.split( '-' )[ 0 ] }-dev-${ currentCommitShort }`;

I find it odd that this issue was not in Studio before. @wojtekn maybe a dependency update introduced a bug here?

wojtekn · 2024-06-03T10:11:17Z

I find it odd that this issue was not in Studio before. @wojtekn maybe a dependency update introduced a bug here?

Yes, this is caused by the Electron Forge upgrade to 7.4.0. I've merged it today and rebased this branch to see if it fixes DMG issue by the chance.

There is an open issue for that (electron/packager#1714) and I think we will need to downgrade electron Forge to 7.3.1 until it's fixed.

fluiddot · 2024-06-03T11:06:30Z

There is an open issue for that (electron/packager#1714) and I think we will need to downgrade electron Forge to 7.3.1 until it's fixed.

This will be solved in #201.

fluiddot

I've just tested the Apple silicon DMG and noticed that the installer has no background:

IIRC, it should show the following image as background:

mokagio · 2024-06-25T04:24:00Z

I've just tested the Apple silicon DMG and noticed that the installer has no background:

@fluiddot @wojtekn the missing background appears to be a known side-effect of using --skip-jenkins. That is, the option bypasses running the AppleScript that configures the aesthetic of the DMG. See the source code.

I tested the DMG on both Apple Silicon (M1 Max) and Intel. They both worked.

@wojtekn assuming we are okay to iterate on the DMG UI, I think this would be good to merge?

…ually

wojtekn · 2024-06-25T10:09:19Z

@mokagio I think the key is to make it work without --skip-jenkins so it has the background.

fluiddot · 2024-06-25T11:16:12Z

@fluiddot @wojtekn the missing background appears to be a known side-effect of using --skip-jenkins. That is, the option bypasses running the AppleScript that configures the aesthetic of the DMG. See the source code.

I tested the DMG on both Apple Silicon (M1 Max) and Intel. They both worked.

@wojtekn assuming we are okay to iterate on the DMG UI, I think this would be good to merge?

As shared by Wojtek, it would be nice if we could solve the missing background and icon positioning. Not sure if we tried before but I created a draft PR using https://github.com/LinusU/node-appdmg which successfully created DMG files.

fluiddot · 2024-07-01T10:12:20Z

Heads up that we managed to generate the DMG files successfully in #321 and #299 using the appdmg package.

* Use `appdmg` in `made-dmg` script * Do not install `create-dmg` in Buildkite * Rebuild specific node native modules * [TEST] Log native module rebuild * Conditionally rebuild `fs-xattr` before packaging DMG to fix dlopen issue (#321) * Install Rosetta on macOS Try to address https://buildkite.com/automattic/studio/builds/1543#01904ffb-8498-4d13-aa5a-e2b3c6dbb302 * Rebuild fs-xatrr after binary has been built, conditionally * Revert "Install Rosetta on macOS" This reverts commit 8a4cab7. * Address typos in Buildkite pipeline * Avoid verbose output when rebuilding `fs-xattr` --------- Co-authored-by: Gio Lodi <gio.lodi@automattic.com>

fluiddot

LGTM 🎊 ! I confirmed that all DMG files work as expected for all macOS variants.

wojtekn · 2024-07-01T13:19:32Z

Thanks @fluiddot and @mokagio for bringing this over the finish line! 🥳

wojtekn added the Studio App label May 15, 2024

wojtekn self-assigned this May 15, 2024

wojtekn requested review from p-jackson, mokagio and a team and removed request for p-jackson May 15, 2024 11:13

wojtekn force-pushed the update/build-dmg-manually branch from 2c114f4 to 0f3746c Compare May 15, 2024 11:48

wojtekn commented May 15, 2024

View reviewed changes

wojtekn force-pushed the update/build-dmg-manually branch from 619784d to fcbc10a Compare May 15, 2024 15:04

mokagio reviewed May 16, 2024

View reviewed changes

jkmassel force-pushed the update/build-dmg-manually branch from 806036f to 569b579 Compare May 17, 2024 15:40

p-jackson and others added 10 commits June 3, 2024 10:59

Invoke electron-installer-dmg directly to create DMG

395fd63

Upload dev build dmgs on buildkite

173e6e1

Distribute universal DMG

bcc88f9

Fix universal dmg script

01c984c

Try a CLI tool for creating DMG files

e490b63

Fix lint issues

4739de4

Try suggested fix

498aa7a

Notarize DMG files

4659c3e

Add DMGs to CI artifacts

57750eb

Remove unnecessary trailing comments from pipeline

f8140f6

wojtekn force-pushed the update/build-dmg-manually branch from e9a8df5 to f8140f6 Compare June 3, 2024 09:05

fluiddot mentioned this pull request Jun 3, 2024

Downgrade Electron Forge to version 7.3.1 #201

Merged

1 task

Merge branch 'trunk' into update/build-dmg-manually

9a99bd9

fluiddot reviewed Jun 3, 2024

View reviewed changes

Merge remote-tracking branch 'origin/trunk' into update/build-dmg-man…

e311f51

…ually

This was referenced Jun 25, 2024

Print message to explain missing background with --skip-jenkins create-dmg/create-dmg#182

Merged

Print create-dmg output for better logs #296

Closed

Add more Buildkite logs groups in macOS builds

00453c7

fluiddot mentioned this pull request Jun 25, 2024

Build DMG files with appdmg #299

Merged

1 task

fluiddot and others added 3 commits July 1, 2024 12:26

Rebuild native module fs-xattr on release builds

adee140

Remove package electron-installer-dmg

3ac04d1

fluiddot approved these changes Jul 1, 2024

View reviewed changes

fluiddot merged commit dc07384 into trunk Jul 1, 2024
13 checks passed

fluiddot deleted the update/build-dmg-manually branch July 1, 2024 10:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create DMGs for x64 and arm64 builds #135

Create DMGs for x64 and arm64 builds #135

wojtekn commented May 15, 2024 •

edited by fluiddot

Loading

wojtekn commented May 15, 2024

wojtekn May 15, 2024

mokagio May 16, 2024

mokagio May 16, 2024

wojtekn commented May 15, 2024 •

edited

Loading

wojtekn commented May 15, 2024

mokagio May 16, 2024

wojtekn commented May 16, 2024

wojtekn commented May 21, 2024

mokagio commented Jun 3, 2024 •

edited

Loading

wojtekn commented Jun 3, 2024

fluiddot commented Jun 3, 2024

fluiddot left a comment

mokagio commented Jun 25, 2024

wojtekn commented Jun 25, 2024 •

edited

Loading

fluiddot commented Jun 25, 2024

fluiddot commented Jul 1, 2024

fluiddot left a comment •

edited

Loading

wojtekn commented Jul 1, 2024

Create DMGs for x64 and arm64 builds #135

Create DMGs for x64 and arm64 builds #135

Conversation

wojtekn commented May 15, 2024 • edited by fluiddot Loading

Proposed Changes

Testing Instructions

arm64

x64

universal

Pre-merge Checklist

wojtekn commented May 15, 2024

wojtekn May 15, 2024

Choose a reason for hiding this comment

mokagio May 16, 2024

Choose a reason for hiding this comment

mokagio May 16, 2024

Choose a reason for hiding this comment

wojtekn commented May 15, 2024 • edited Loading

wojtekn commented May 15, 2024

mokagio May 16, 2024

Choose a reason for hiding this comment

wojtekn commented May 16, 2024

wojtekn commented May 21, 2024

mokagio commented Jun 3, 2024 • edited Loading

wojtekn commented Jun 3, 2024

fluiddot commented Jun 3, 2024

fluiddot left a comment

Choose a reason for hiding this comment

mokagio commented Jun 25, 2024

wojtekn commented Jun 25, 2024 • edited Loading

fluiddot commented Jun 25, 2024

fluiddot commented Jul 1, 2024

fluiddot left a comment • edited Loading

Choose a reason for hiding this comment

wojtekn commented Jul 1, 2024

wojtekn commented May 15, 2024 •

edited by fluiddot

Loading

`arm64`

`x64`

`universal`

wojtekn commented May 15, 2024 •

edited

Loading

mokagio commented Jun 3, 2024 •

edited

Loading

wojtekn commented Jun 25, 2024 •

edited

Loading

fluiddot left a comment •

edited

Loading