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

New electron-updater #3084

Merged
merged 5 commits into from
Jun 12, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion apps/insiders/src/insiders-packager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.


return version;
}
Expand Down
2 changes: 1 addition & 1 deletion apps/zui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Contributor Author

@philrz philrz Jun 7, 2024

Choose a reason for hiding this comment

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

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.

"esbuild": "^0.18.14",
"eslint": "^8.11.0",
"eslint-config-prettier": "^8.5.0",
Expand Down
27 changes: 9 additions & 18 deletions apps/zui/src/domain/updates/linux-updater.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
import {app, shell} from "electron"
import fetch from "node-fetch"
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
const current = app.getVersion()
Comment on lines +1 to +16
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

if (semver.lt(current, latest)) {
return latest
Expand All @@ -22,19 +26,6 @@ export class LinuxUpdater implements Updater {
shell.openExternal(this.downloadUrl())
}

private async latest() {
const resp = await fetch(this.latestUrl())
if (resp.status === 204) return app.getVersion()
const data = await resp.json()
return data.name
}

private latestUrl() {
const repo = getMainObject().appMeta.repo
const platform = "darwin-x64" // If the mac version exists, the linux does too
return `https://update.electronjs.org/${repo}/${platform}/${app.getVersion()}`
}

private downloadUrl() {
if (env.isInsiders) {
return pkg.repository + "/releases/latest"
Expand Down
1 change: 1 addition & 0 deletions apps/zui/src/domain/updates/mac-win-updater.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {getMainObject} from "src/core/main"

autoUpdater.autoDownload = false
autoUpdater.autoInstallOnAppQuit = false
autoUpdater.forceDevUpdateConfig = true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.


export class MacWinUpdater implements Updater {
async check() {
Expand Down
51 changes: 28 additions & 23 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5579,7 +5579,7 @@ __metadata:
languageName: node
linkType: hard

"@types/semver@npm:^7.3.3, @types/semver@npm:^7.3.4":
"@types/semver@npm:^7.3.3":
version: 7.3.4
resolution: "@types/semver@npm:7.3.4"
checksum: 577dc7f607fbf46cf185f61ba726e4cfecea41c4b17e55069e70a64b6cd92658f8286aa075a607303455658e87ed045d3d14c32a513c0eff381d64aecd7b72a7
Expand Down Expand Up @@ -7299,16 +7299,6 @@ __metadata:
languageName: node
linkType: hard

"builder-util-runtime@npm:8.7.3":
version: 8.7.3
resolution: "builder-util-runtime@npm:8.7.3"
dependencies:
debug: ^4.3.2
sax: ^1.2.4
checksum: eaf55ffcbde90a65639d7e102626ea819d0c0e4b296984755dab8d9df2bd85c56fb586e536a175b9229477d7e122c8f553aa232827aa7e275b43274b87a6e3e5
languageName: node
linkType: hard

"builder-util-runtime@npm:9.2.4":
version: 9.2.4
resolution: "builder-util-runtime@npm:9.2.4"
Expand Down Expand Up @@ -9219,18 +9209,19 @@ __metadata:
languageName: node
linkType: hard

"electron-updater@npm:^4.3.8":
version: 4.3.8
resolution: "electron-updater@npm:4.3.8"
"electron-updater@npm:6.2.1":
version: 6.2.1
resolution: "electron-updater@npm:6.2.1"
dependencies:
"@types/semver": ^7.3.4
builder-util-runtime: 8.7.3
fs-extra: ^9.1.0
js-yaml: ^4.0.0
lazy-val: ^1.0.4
builder-util-runtime: 9.2.4
fs-extra: ^10.1.0
js-yaml: ^4.1.0
lazy-val: ^1.0.5
lodash.escaperegexp: ^4.1.2
lodash.isequal: ^4.5.0
semver: ^7.3.4
checksum: 1814fe7c99eb6c1cb2251fc24eb62f51df716e44c891d5433e8e036a77ff2beec241249f7ba26b33a60cf134158c2160a74117dff7751b4d2e59c57e688ae17a
semver: ^7.3.8
tiny-typed-emitter: ^2.1.0
checksum: 92a064610a3c9df747dce9c3eccd69c48adb2c8b37b9d7c13d1c39f7e2a9ffaef4e909ab50e6973d557566bde6de7ec9c64e2cc3a2cdef18b3220c5d91333e1c
languageName: node
linkType: hard

Expand Down Expand Up @@ -13287,7 +13278,7 @@ __metadata:
languageName: node
linkType: hard

"js-yaml@npm:4.1.0, js-yaml@npm:^4.0.0, js-yaml@npm:^4.1.0":
"js-yaml@npm:4.1.0, js-yaml@npm:^4.1.0":
version: 4.1.0
resolution: "js-yaml@npm:4.1.0"
dependencies:
Expand Down Expand Up @@ -13759,6 +13750,13 @@ __metadata:
languageName: node
linkType: hard

"lodash.escaperegexp@npm:^4.1.2":
version: 4.1.2
resolution: "lodash.escaperegexp@npm:4.1.2"
checksum: 6d99452b1cfd6073175a9b741a9b09ece159eac463f86f02ea3bee2e2092923fce812c8d2bf446309cc52d1d61bf9af51c8118b0d7421388e6cead7bd3798f0f
languageName: node
linkType: hard

"lodash.isequal@npm:^4.5.0":
version: 4.5.0
resolution: "lodash.isequal@npm:4.5.0"
Expand Down Expand Up @@ -18197,6 +18195,13 @@ __metadata:
languageName: node
linkType: hard

"tiny-typed-emitter@npm:^2.1.0":
version: 2.1.0
resolution: "tiny-typed-emitter@npm:2.1.0"
checksum: 709bca410054e08df4dc29d5ea0916328bb2900d60245c6a743068ea223887d9fd2c945b6070eb20336275a557a36c2808e5c87d2ed4b60633458632be4a3e10
languageName: node
linkType: hard

"tiny-warning@npm:^1.0.0, tiny-warning@npm:^1.0.3":
version: 1.0.3
resolution: "tiny-warning@npm:1.0.3"
Expand Down Expand Up @@ -19701,7 +19706,7 @@ __metadata:
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
esbuild: ^0.18.14
eslint: ^8.11.0
eslint-config-prettier: ^8.5.0
Expand Down
Loading