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.63k stars 1.74k forks source link

GithubPublisher does not respect `detectUpdateChannel` setting #8589

Open rotu opened 1 week ago

rotu commented 1 week ago

I'm using electron-builder with detectUpdateChannel = true, generateUpdatesFilesForAllChannels = true, and publish.provider = "github", and it will not set the channel based on the version (e.g. 1.2.3-beta.4 in my package.json. All builds wind up creating a latest.yml file.

electron-builder does emit the channel-specific yaml if I set publish.channel explicitly.

It seems the logic to do this is typically in SomePublisherClass.checkAndResolveOptions^1 but GitHubPublisher has no such function, instead doing all options logic in the constructor^2.

mmaietta commented 6 days ago

Hmmm, I don't recall any changes to that logic being done in recent history. Did this work in previous versions of electron-builder or does it always occur for you?

Ah, I see now. I'm not sure how to approach this though as enabling update channel detection on github would be a breaking change. It's already default true, so this would change the behavior of every updater in the electron-builder ecosystem.

rotu commented 6 days ago

I'd say the obvious choice is to accept that this is a breaking change and release it in the next major version, with prominent documentation.

Otherwise, it be done in a two-phase way. The Github provider could emit a warning message like "detectUpdateChannel is not explicitly set. In a future version, this will be defaulted to true. Please set it to explicitly true or false to silence this warning."

mmaietta commented 6 days ago

It already defaults true https://github.com/electron-userland/electron-builder/blob/c84843053a8f9e0b6af14c6b2ed33c5f82d495b3/packages/app-builder-lib/src/options/PlatformSpecificBuildOptions.ts#L168-L172

rotu commented 6 days ago

It already defaults true

Yes, and since that’s not respected downstream, you may as well retcon the behavior as “default: false for github, true for other providers”.

mmaietta commented 2 days ago

Tbh, I don't want to make this change. It's a hefty breaking change for current publish logic, which is a highly sensitive area (along with electron-updater)

Instead, I've put together a patch-package that you can use on top of v25.1.8 (latest) app-builder-lib+25.1.8.patch

diff --git a/node_modules/app-builder-lib/out/publish/PublishManager.js b/node_modules/app-builder-lib/out/publish/PublishManager.js
index 800352b..223cfe7 100644
--- a/node_modules/app-builder-lib/out/publish/PublishManager.js
+++ b/node_modules/app-builder-lib/out/publish/PublishManager.js
@@ -414,8 +414,12 @@ async function getResolvedPublishConfig(platformPackager, packager, options, arc
     if (!isGithub && provider !== "bitbucket") {
         return options;
     }
-    let owner = isGithub ? options.owner : options.owner;
-    let project = isGithub ? options.repo : options.slug;
+    const o = options;
+    if (o.channel == null && channelFromAppVersion != null) {
+        o.channel = channelFromAppVersion;
+    }
+    let owner = o.owner;
+    let project = isGithub ? o.repo : o.slug;
     if (isGithub && owner == null && project != null) {
         const index = project.indexOf("/");
         if (index > 0) {
@@ -452,15 +456,15 @@ async function getResolvedPublishConfig(platformPackager, packager, options, arc
         }
     }
     if (isGithub) {
-        if (options.token != null && !options.private) {
+        if (o.token != null && !o.private) {
             builder_util_1.log.warn('"token" specified in the github publish options. It should be used only for [setFeedURL](module:electron-updater/out/AppUpdater.AppUpdater+setFeedURL).');
         }
         //tslint:disable-next-line:no-object-literal-type-assertion
-        return { owner, repo: project, ...options };
+        return { owner, repo: project, ...o };
     }
     else {
         //tslint:disable-next-line:no-object-literal-type-assertion
-        return { owner, slug: project, ...options };
+        return { owner, slug: project, ...o };
     }
 }
 //# sourceMappingURL=PublishManager.js.map
\ No newline at end of file