studio

studio copied to clipboard

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.

• npm install
• npm run make:macos-arm64
• npm run make:dmg-arm64
• Checkout the DMG at out/Studio-darwin-arm64.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?

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.

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. 🤷

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)

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 https://github.com/create-dmg/create-dmg/issues/72

I reported the initial issue under https://github.com/electron/forge/issues/3604.

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?

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 (https://github.com/electron/packager/issues/1714) and I think we will need to downgrade electron Forge to 7.3.1 until it's fixed.

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 https://github.com/Automattic/studio/pull/201.

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?

@mokagio I think the key is to make it work without --skip-jenkins so it has the 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?

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.

Heads up that we managed to generate the DMG files successfully in https://github.com/Automattic/studio/pull/321 and https://github.com/Automattic/studio/pull/299 using the appdmg package.

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