New electron-updater #3084
New electron-updater #3084
Conversation
| @@ -40,7 +40,7 @@ export class InsidersPackager { | |||
| const version = | |||
| this.strategy === 'match-stable' | |||
| ? this.stableVersion | |||
| : semver.inc(this.lastVersion, 'prerelease'); | |||
| : semver.inc(this.lastVersion, 'prerelease', 'insiders'); | |||
There was a problem hiding this comment.
During #3082 debug, at one point I'd advanced the electron-updater dependency to the latest (6.2.1) just to see if any of the problems I'd observed had already been fixed. What I found was that Zui Insiders auto-updates actually broke in places like they used to work before, such as macOS.
I debugged further by putting tons of console debug logging in the electron-updater/out/providers/GitHubProvider.js for both our old and the lastest electron-updater versions (GitHubProvider.js.4.3.8.gz and GitHubProvider.js.6.2.1.gz, respectively). What this revealed is that the "channels" support added in electron-updater v5 did not play nice with the way we'd been creating prerelease version strings in Zui Insiders. Specifically, while the lone trailing digits like in v1.7.1-24 are legal per the semver spec and the spec says "Identifiers consisting of only digits are compared numerically", the debug output showed that electron-updater was treating the trailing digits as the "channel name" (i.e., channel name 24 in this case) and so the lack of any further numbers after that channel name (e.g., -24.1, -24.2) meant that once running a Zui Insiders prerelease version the autoUpdater would not see any of the higher-numbered prereleases (e.g., -25, -26, etc.) as eligible update targets.
Thankfully, a solution did reveal itself, and that's to lean into the channel feature. In addition to the "official" supported channel names like alpha and beta, electron-update also supports "custom" channel names, such as I'm doing here, which results in release version strings like v1.7.1-insiders.24. That makes it happy since it finds insiders as the channel name and 24 as a version that testing confirms is treated like a number (e.g., a -insiders.9 release does see an -insiders.10 release as an eligible update target).
We're not currently trying to do anything fancy with channels (e.g., maintain separate alpha and beta channels, let users bounce between channels, downgrade, etc.) and since testing confirms that this one change gets us to what we need I'm keen to make the minimal change for the maximum benefit. Another lemonade-from-lemons effect is that the presence of the explicit insiders in the version string is another nice self-documenting way to know if a user is running Insiders rather than regular Zui, such as if they're pinging us for Support and we ask them to show their version string from About.
| @@ -91,7 +91,7 @@ | |||
| "electron-localshortcut": "^3.2.1", | |||
| "electron-log": "5.0.0-beta.6", | |||
| "electron-mock-ipc": "^0.3.12", | |||
| "electron-updater": "^4.3.8", | |||
| "electron-updater": "6.2.1", | |||
There was a problem hiding this comment.
Previously we were using the ^ to automatically update to minor/patch electron-updater releases as they became available. I've now spent enough time testing through the release history of electron-updater that I don't wholly trust the stewardship of the project to maintain stability between major releases. We also don't have automated test coverage in this area, which is more risk. Therefore since I've done so many days of testing on this 6.2.1 release I'm keen to lock us in place until we have a specific reason to move to something newer and make the appropriate investment in another round of testing at that time.
| import {autoUpdater} from "electron-updater" | ||
| import {Updater} from "./types" | ||
| import semver from "semver" | ||
| import {app, shell} from "electron" | ||
| import env from "src/app/core/env" | ||
| import links from "src/app/core/links" | ||
| import pkg from "src/electron/pkg" | ||
| import {Updater} from "./types" | ||
| import {getMainObject} from "src/core/main" | ||
|
|
||
| autoUpdater.autoDownload = false | ||
| autoUpdater.autoInstallOnAppQuit = false | ||
| autoUpdater.forceDevUpdateConfig = true | ||
|
|
||
| export class LinuxUpdater implements Updater { | ||
| async check() { | ||
| const latest = await this.latest() | ||
| const {updateInfo} = await autoUpdater.checkForUpdates() | ||
| const latest = updateInfo.version |
There was a problem hiding this comment.
This is the big payoff. What I'm adding here plus the code removal in this same file effectively drops our old "Linux update helper" (the one that tried to look for a newer macOS release as a sign that a newer Linux release is surely available as well) and allows us to use the autoUpdater to check for the latest Linux release. This approach became possible when electron-updater added the "beta" support for Linux autoUpdate in electron-userland/electron-builder#7060 (so, it became part of electron-updater v6). This includes the creation of the latest-linux.yml build metadata which is why we can now just call autoUpdater.checkForUpdates() and get a correct answer for "What's the latest Linux release available relative to the one I'm running?" (i.e., it fixes the "Problem 2" described in #3082).
FWIW, I did test out the "beta" support for true Linux autoUpdate and it didn't work for me, complaining of sha512 checksum mismatch between the build itself and its metadata. Since the scope of my testing/changes was already significant enough, plus it's a "beta" feature after all, I chose to not dig further into that for now and instead maintain feature parity with what we already provide on Linux, i.e., check for a newer release and pop up a notification, but clicking Install just brings up the page in the user's browser where they can download the latest.
I recognize that I'm effectively duplicating some code from mac-win-updater.ts here, but since I'm nervous about my JavaScript skills I wanted to minimize the degree to which I was modifying existing code. @jameskerr if you'd like to push a commit to de-duplicate any of what I've done between these two updater files that's fine with me, since I'm planning on doing a final round of testing after getting approval on the PR anyway. Or if you're ok living with some duplication, once I figure out what it'd take to get Linux autoUpdate working (something I'd definitely want to do soon) we should be able to collapse down to a single file of updater code at that time anyway.
| @@ -6,6 +6,7 @@ import {getMainObject} from "src/core/main" | |||
|
|
|||
| autoUpdater.autoDownload = false | |||
| autoUpdater.autoInstallOnAppQuit = false | |||
| autoUpdater.forceDevUpdateConfig = true | |||
There was a problem hiding this comment.
I previously had not been using it since I always opt to test in personal fork repos, but during my test work I became aware of the apps/zui/dev-app-update.yml file. This allows for quick & dirty update testing in Dev mode since it allows us to fiddle with the current version string in package.json and the owner & repo settings in this YAML file and immediately see how the app would react if it had been bundled/released and was checking for an update at the configured GitHub Releases page. Well, per electron-userland/electron-builder#6863 and the fix in electron-userland/electron-builder#7117, within the jump period between our prior electron-updater version and the latest v6.2.1, they effectively broke it and now require setting this forceDevUpdateConfig in order for the file to be leveraged in Dev mode.
|
I'll also document for the record that during some of the macOS auto-updates, I've seen crashes similar to the one in the screenshot below, which happened during the auto-update from the Zui Insiders
|
|
Here's another "full disclosure" finding: In my testing on Rocky Linux, we find that I can't seem to install the RPM for regular Zui while Zui Insiders is already installed. It was on RPM-based Linux that I first found and filed open issue #1588 where the conflict surfaced during an install of a newer Zui when an older Zui was already installed, which led to the guidance in the Zui installation docs to explicitly uninstall first to avoid this problem, so I'm not altogether surprised that the same symptom has surfaced between Zui and Zui Insiders installs. This is unfortunate because we'd like users to be able to have both installed side-by-side, but there may be problems related to the installers that are out of our control. In any case, I'll make note of this in #1588 and perhaps at some point we can find a decent fix or workaround.
|
|
During a final round of upgrade testing to include Windows and Rocky Linux (RPM installer), I ended up bumping into unrelated bug #3089. Once its fix in #3090 was merged and I caught this branch up with I did have to employ one final trick in the Windows testing. The Likewise for Zui Insiders: After doing a |
Fixes #3082, which has detailed background on what's being fixed here and what else we expect to gain by moving to a newer
electron-updater.Starting from the code on this branch, I've done an exhaustive set of successful upgrade tests in personal fork repos of Zui and Zui Insiders on both macOS and Ubuntu (
.deb). If this PR should get approved, after having updated the branch with any suggestions from reviewers, I'll catch it up withmainand repeat the tests one last time including both Windows and Rocky Linux (.rpm) as well for completeness. I happened to skip those on the first pass since in the past I've found update behavior on Windows closely tracks what we see on macOS and likewise between the different Linux installers, so I figured I'd save a little time on the first pass since I assumed I'd want to re-test again after merging suggestions anyway.Here's a complete run down of all the tests I performed and the results.
Make personal forks of both Zui and Zui Insiders repos per instructions here
v1.6.1-33from the "official" Insiders repo so that way there'd be an existing release to increment from. I then deleted the release and its tag after I made the first proper Insiders release in my fork repo.brimdatatophilrzin both themainbranch and therelease/v1.7.0branchIn the Zui fork, delete the
v1.7.0tag and create a newv1.7.0release based off therelease/v1.7.0branchv1.7.0today, which has the old electron-updaterIn the Zui Insiders fork, create a new release based off the
release/v1.7.0branchv1.7.0was createdIn the Zui Insiders fork, create a new release in my personal repo based off the tip of
mainv1.7.1-0was createdv1.7.0and it found/auto-updated tov1.7.1-0v1.7.0and it did not initially see thev1.7.1-0as an eligible update target, but this is due to the known "Problem 1" described in Linux update bugs #3082. Therefore I manually downloaded theZui---Insiders-1.7.1-0-x64.zipartifact, renamed it toZui---Insiders-1.7.1-0-mac.zip, and uploaded that to the Release. After waiting about 15 minutes, now the app did findv1.7.1-0as an eligible update target.In the Zui fork, advance the version string in
package.jsonto1.8.0and merge thenew-electron-updaterPRIn the Zui fork, create a new
v1.8.0releasev1.7.0Linux release still has the "Problem 1" from Linux update bugs #3082, I manually added theZui-1.8.0-mac.zipto its Release page so Linux users would see this release as an eligible update targetv1.7.0release and it auto-updated tov1.8.0v1.7.0release and it sawv1.8.0as an eligible update targetIn the Zui Insiders fork, create a new Zui Insiders release
v1.8.0was created. Since the older Insiders releases still have the "Problem 1" from Linux update bugs #3082, I manually added theZui---Insiders-1.8.0-mac.zipto its Release page so Linux users would see this release as an eligible update target.v1.7.0release and it auto-updated to thev1.8.0releasev1.7.1-0release and it auto-updated to thev1.8.0releasev1.7.0release and it sawv1.8.0as an eligible update targetv1.7.1-0release and it saw thev1.8.0release as an eligible update target.In the Zui Insiders fork, trigger creation of releases
-insiders.1,-insiders.12, ... ,-insiders.10v1.7.0and auto-updated tov1.8.1-insiders.10v1.7.1-0and auto-updated tov1.8.1-insiders.10v1.8.0and auto-updated tov1.8.1-insiders.10v1.8.1-insiders.9it auto-updated tov1.8.1-insiders.10v1.7.0and it foundv1.8.0as the latest available, but of course clicking the Install button brought up the/releases/latestpage on GitHub such that I was shownv1.8.1-insiders.10as the update targetv1.7.0and it foundv1.8.0as the latest available, but of course clicking the Install button brought up the/releases/latestpage on GitHub such that I was shownv1.8.1-insiders.10as the update targetv1.7.1-0and it foundv1.8.0as the latest available, but of course clicking the Install button brought up the/releases/latestpage on GitHub such that I was shownv1.8.1-insiders.10as the update targetv1.8.0and it foundv1.8.1-insiders.10as the latest available (and I'd not had to manually add any "mac ZIP" file for the release)v1.8.1-insiders.9and it foundv1.8.1-insiders.10as the latest available (and I'd not had to manually add any "mac ZIP" file for the release)In the Zui fork, create a new
v1.9.0releasev1.7.0release and it auto-updated to thev1.9.0releasev1.8.0release and it auto-updated to thev1.9.0releasev1.7.0and it foundv1.8.0as the latest available, but of course clicking the Install button brought up the brimdata.io Downloads that would offerv1.9.0as the update targetv1.8.0and it foundv1.9.0as the latest available (and I'd not had to manually add any "mac ZIP" file for the release)Conclusion: Once users get on to one of the builds running the newer
electron-updater, they'll find the proper "latest" release for both Zui and Zui Insiders without any manual intervention on our part. The test results indicate that auto-update users (macOS) will make this transition without any manual intervention on our part. On Linux, we'll need to manually provide the "mac ZIP" file as part of the first releases that have the newerelectron-updater, but once they make that leap, their apps will keep finding "latest" releases based on just the presence of thelatest-linux.ymlthat's created automatically by our build process. And since our Linux "helper" just lands them on a "manually download the latest" web page whenever a newer release is detected, they complete this hop easily.