electron-userland / electron-builder

A complete solution to package and build a ready for distribution Electron app with “auto update” support out of the box
https://www.electron.build
MIT License
13.69k stars 1.74k forks source link

electron-updater: feature request - custom `minimumSystemVersion` validation #8682

Open kavsingh opened 1 day ago

kavsingh commented 1 day ago

Before minimumSystemVersion was read in-app by electron-updater - https://github.com/electron-userland/electron-builder/pull/8108 - we rolled our own minimumSystemVersion checks since we had a pressing need.

When building our application, we write minimumSystemVersion to the update yaml as the "human readable" product version on macOS, e.g 12.0.0 for Monterey (to match mac.minimumSystemVersion in our electron builder config). The changes in #8108 force minimumSystemVersion to be checked against against os.release. While os.release is a sensible default, unfortunately we cannot switch up to use it in our publish yml files without major disruptions to our user base - app installs already in the wild can end up blocked from updating or erroneously updated.

It would be great to have an option to use a custom validator for minimumSystemVersion, or to disable it and handle minimumSystemVersion ourselves.

Am happy to open a PR for this.

mmaietta commented 1 day ago

Thoughts on a metadata field in the app-update.yml that is of type [key: string]: any? Then during update-available, you can check the update for that information in the UpdateInfo object

We'd probably want to set a file size limit (arbitrarily?) on the app-update.yml file though to prevent abuse of the metadata field that would increase file size and the latency (slow download) of detecting an update is available

kavsingh commented 1 day ago

hi @mmaietta! it's a good suggestion but unfortunately that would only work for a new release of our app onwards. the app has been in general release for quite some time and it wouldn't work for users who already have current and previous versions of the app installed, unless i'm missing something:

already released versions in the wild will continue to look for minimumSystemVersion in update info, expecting a version in line with macOS's product version, and not os.release. they wouldn't be looking for the metadata field.

if we change up our writing of minimumSystemVersion when publishing our app to os.release to match what electron-updater expects: already installed versions would not be able to update. say a user is on macOS 13, and we change minimumSystemVersion published in our update info from 12 as we have it to 21 (an os.release version). those installed apps would then always fail to register an available update.

if we leave our writing of minimumSystemVersion in update info as-is, and adopt the latest electron-updater without modifications: already installed versions would not be affected, but we lose the ability to deprecate support for macOS versions via auto update going forward. again taking a user on macOS 13, and we want to change our minimumSystemVersion to 14 and write it as such. a user using the app with latest electron-updater on macOS 13 would end up getting the update, since os.release would resolve to something like 21, and the check would pass. the user then ends up updating to a version of the app they cannot use.

the added catch is that to get existing installations in line with consuming new metadata, we need them to be able to auto update, which is the core issue 😅

mmaietta commented 1 day ago

If I'm understanding correctly, you want custom minimum system version validation on already-installed versions of your application? If so, that's not possible with electron-updater. Feature requests like this will only apply to new deployments, the currently installed applications use the electron-updater version it was bundled with. The only way out of this situation is to revert your non-backward-compatible changes (or create a separate release branch?) so you can update electron-updater (assuming we proceed with metadata or some other approach), from which then you'd be able to move forward with your non-compatible changes.

My apologies in advance if I missread/interpreted what you're saying

kavsingh commented 1 day ago

hey @mmaietta

thanks so much for keeping up with this!

you want custom minimum system version validation on already-installed versions of your application?

ah no not quite, as you said that would clearly not be possible. we would like to able to customise how electron updater checks minimumSystemVersion here for new releases of our app should we update to the new version of electron-updater

this would let us keep writing the version in our publisher yml as e.g. 12 for macOS Monterey and not the os.release version for compatiblity with already released app versions, while allowing new versions to check minimumSystemVersion against eg sw_vers instead of os.release

let me know if that makes more sense

mmaietta commented 1 day ago

Hmmm sw_vers is MacOS-specific, so we'd need similar setups for linux/win or just have a specific flow for mac which seems messy/hard to track bugs with.

Right now minimumSystemVersion isn't able to be set in the electron-builder config for each OS, which I believe has been a previous request. If I implemented a way to have minimumSystemVersion set for each distributable (i.e. platform-specific mac/nsis/etc.) within electron-builder.config.js, would that resolve your issue instead? Then you don't need to custom set any version in your update yaml.

Regardless, it looks like we'll still run into the field conflicting due to var name. In order to have a platform-specific minimumSystemVersion without overlapping with the already-human-readable mac.minimumSystemVersion, the var will need to be renamed. Now thinking about it further though, we can't rename the var in the UpdateInfo without breaking electron-updater compatibility...so I'm feeling hosed on renaming any vars even with electron-builder in a major-semver alpha release state.

An alternative route would require one migration step in between for your users. What I envision:

I think that'd work?

kavsingh commented 1 day ago

thanks again for all the discussion!

Hmmm sw_vers is MacOS-specific, so we'd need similar setups for linux/win or just have a specific flow for mac which seems messy/hard to track bugs with.

minimumSystemVersion without overlapping with the already-human-readable mac.minimumSystemVersion, the var will need to be renamed. Now thinking about it further though, we can't rename the var in the UpdateInfo without breaking electron-updater compatibility..

totally agree that renaming the variable in electron updater would not be the way to go, and that something like a sw_vers is way too specific to work into electron-updater itself

i was thinking more along the lines of being able to assign a version check function in-app, so that a consumer of the library can do something like:

import { autoUpdater } from "electron-updater";

autoUpdater.validateMinimumSystemVersion = (logger, minimumSystemVersion?: string): boolean => {
  // ... some custom version check that returns false if invalid
}

used in AppUpdater#isUpdateAvailable

if (this.validateMinimumSystemVersion) {
  if (this.validateMinimumSystemVersion(this._logger, updateInfo.minimumSystemVersion) === false) {
    return false;
  }
} else {
  // ... perform current default system version check against os.release
}

essentially the end user can decide if the version is valid for their setup. but i can understand if that might add maintenance overhead, not to mention requests for extending that for other attributes down the line

the intermediary release plan you describe is something we've thought about as well - we can give that more thought on our end, though it might take a while for sufficient uptake. in that case, is it advisable to upgrade electron-builder to v25.x while keeping updater at v5.x? we'd like to keep builder at least up-to-date.

we could also look at making use of the update-not-available event to work around our issue, but that would rely on

mmaietta commented 9 hours ago

Hmmm, I like where you're going with this! Perhaps we can expand the functionality to this.isUpdateSupported(this._logger, updateInfo) to be more generic. If we keep it the way you have it, then we have to have the "hook" specifically for minimumSystemVersion so I'm trying to figure out a way around it. Maybe if isUpdateSupported is set, we default to that logic instead of any validation other than staging matcher and downgrade logic

kavsingh commented 4 hours ago

i think that's a great idea! i was hesistant to make it too generic to avoid breaking any other validation logic electron-updater relies on, but what you suggest makes a lot of sense