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

electron-builder: Disable .zip build target on macOS #1177

Closed

Conversation

DeeDeeG
Copy link
Member

@DeeDeeG DeeDeeG commented Dec 30, 2024

Background context

The macOS .zip build target for electron-builder is very computationally intensive (seemingly due to heavy use of 7-zip?) and timing out for us in CI...

Solution

So... disable the .zip build target on macOS entirely.

Note: The .dmg and .zip targets are the default targets enabled by electron-builder on macOS. (See: https://www.electron.build/cli#target again to confirm.) So, by explicitly specifying only the .dmg target, we can effectively disable the .zip target in that way.

Verification process

On my local macOS machine, I did a clean yarn install && yarn build && yarn build:apm && yarn dist run (key part being the yarn dist), and the only binary produced was the .dmg. The .zip target was not built, confirming that the change had the intended effect.

May observe CI to further confirm it does not build the .zip build target. UPDATE: Worked as intended, only the .dmg was made, not the .zip. Logs show this, and furthermore, the "macos-13 Binaries.zip" CI asset only contained a .dmg not a .zip.

Potential downsides

Might want to look into the various third-party not-maintained-by-us things (Homebrew?) that might possibly rely on the macOS .zip build target and give them a heads-up about this. Or... see alternative approaches:

Alternative approaches / Potential ToDo

We could make our own .zip of the Pulsar.app fairly easily, I suppose. I have not done so at this time, so as to make the CI fix available for review as fast as I could. But it wouldn't be too hard to figure out how to zip the .app locally, and then write the equivalent steps into the "build pulsar binaries" GitHub Actions workflow file?

May need to bug GitHub Actions Runners team with bug report as well, as this really shouldn't be timing out (stalling, flaking) IMO, and it didn't used to.

Copy link
Member

@confused-Techie confused-Techie left a comment

Choose a reason for hiding this comment

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

Based on the conversation I've read on Discord about this change, this seems to make perfect sense to me. And with a change so small and simple and being able to confirm behavior in our build step, I'd say lets merge this!

But a note, we will want to make sure to update the release automation accordingly

The macOS .zip build target for electron-builder is very computationally
intensive (seemingly due to heavy use of 7-zip?) and timing out for us
in CI. So... disable the .zip build target on macOS entirely.

Note: The .dmg and .zip targets are the default targets enabled by
electron-builder on macOS. (See: https://www.electron.build/cli#target)
So, by explicitly specifying only the .dmg target,
we can effectively disable the .zip target.
@DeeDeeG DeeDeeG force-pushed the disable-macos-zip-build-target-electron-builder branch from 1aa3d65 to 9e94520 Compare December 30, 2024 05:16
@DeeDeeG
Copy link
Member Author

DeeDeeG commented Dec 30, 2024

Updated PR body text and force-pushed with updated commit message (no change in the diff).

We can't really be sure any issues are due to blockmaps, and from some local testing it now seems unlikely the blockmaps themselves are the problem. (Leaving them disabled to the extent we can, because we still don't use them, and making them and not accidentally uploading them is more hassle than not making them in the first place.)

@savetheclocktower
Copy link
Contributor

If we do still want to distribute a ZIP version on macOS, do we have a plan for how we're going to do that?

@DeeDeeG
Copy link
Member Author

DeeDeeG commented Dec 30, 2024

I would suggest we make a little script (JS? bash?) that can package the .app from dist/mac/ dir into a .zip, and read package.json to get the version string properly when naming the .zip file. (example: Pulsar.app --> Pulsar-1.124.0-dev-mac.zip)

We should be careful not to regress final zip size too badly. My own quick local testing showed the built-in macOS zip command's default settings made a .zip that was twice as big as electron-builder's result using 7-zip. So we should either use 7-zip or tweak the compression speed/size tradeoff settings for the zip command, etc...

And/or maybe bug GitHub Actions Runner team with a bug report so we can revert this PR ASAP (or even not land this one for now). I do like making the .zip available. If we can't revert this, then we may want/need to reach out to e.g. Homebrew and make sure their packaging scripts can use the .dmg, if possible...

@DeeDeeG
Copy link
Member Author

DeeDeeG commented Dec 30, 2024

See also: #1178

Different approach: assumes CI will flake/fail, and work around it by leaning into retries (shorter timeouts to retry more "impatiently"/more readily, more retries to "get more chances at success" -- since I think successful runs tend to succeed quickly, and timed out runs appear to badly hang or seize up... So might as well start over if a run takes much longer than the usual successful ones?)

@DeeDeeG
Copy link
Member Author

DeeDeeG commented Jan 7, 2025

Closing, I prefer #1178, and research/testing on that is almost done/landable.

For the record: Blame the .dmg target, not the .zip target or the blockmaps. They were framed by async and misleading logging that put them at the scene of the crime, but they were innocent.

@DeeDeeG DeeDeeG closed this Jan 7, 2025
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.

3 participants