electron-builder icon indicating copy to clipboard operation
electron-builder copied to clipboard

GithubPublisher does not respect `detectUpdateChannel` setting

Open rotu opened this issue 1 year ago • 5 comments

  • Electron-Builder Version: 25.1.7
  • Node Version: 22.9.0
  • Electron Version: 32.1.2
  • Electron Type (current, beta, nightly): current

N/A

  • Target: nsis

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.

rotu avatar Oct 11 '24 22:10 rotu

~~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.

mmaietta avatar Oct 12 '24 20:10 mmaietta

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."

rotu avatar Oct 12 '24 23:10 rotu

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

mmaietta avatar Oct 13 '24 00:10 mmaietta

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”.

rotu avatar Oct 13 '24 00:10 rotu

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

mmaietta avatar Oct 16 '24 18:10 mmaietta

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions[bot] avatar Dec 16 '24 00:12 github-actions[bot]

Not stale

rotu avatar Dec 16 '24 02:12 rotu

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 30 days.

github-actions[bot] avatar Mar 01 '25 00:03 github-actions[bot]

Still not stale

rotu avatar Mar 01 '25 01:03 rotu

I provided you a patch-package to supply this functionality for your personal project. As it currently stands, I'm not implementing this in electron-builder unless more devs request this functionality.

mmaietta avatar Mar 01 '25 02:03 mmaietta

I provided you a patch-package to supply this functionality for your personal project. As it currently stands, I'm not implementing this in electron-builder unless more devs request this functionality.

Thanks! As much as I'd expect and want this to be default, I appreciate the sense of direction. Much better to have the issue closed as "not planned" rather than closed as stale. I'll submit a PR to update the docs.

rotu avatar Mar 03 '25 16:03 rotu

Let me know if you need a patch-package for the latest v26.x.x electron-builder 🙂

mmaietta avatar Mar 03 '25 17:03 mmaietta

Let me know if you need a patch-package for the latest v26.x.x electron-builder 🙂

I don't - patch-package is a bit scary to me. Especially patching a build tool!

The hardest part with this issue was figuring out what was happening and why. My coworker said "I followed the instructions to set up release channels" and in particular was misled by this statement in the docs:

All you have to do to release using channels is to define the channel in the version tag of the package.json. Add “-beta” or “-alpha” (nothing for “latest”) to automatically build for the related channel.

Since I'm using quasar which generates the electron-builder.json at runtime, I'm working around this by setting channel explicitly. I'm sure this could also be done with build hooks instead.

import packageJson from './package.json' with { type: 'json' };
import { prerelease } from 'semver';

function getReleaseChannel() {
   let result = prerelease(packageJson.version)?.[0] ?? 'latest';
   if (!['beta', 'dev', 'latest'].includes(result)) {
      console.warn(
         `⚠️ Inferred release channel "${result}" is not recognized. Defaulting to "dev"`,
      );
      return 'dev';
   }
   return result;
}

publish: {
  provider: 'github',
  channel: getReleaseChannel(),
},

rotu avatar Mar 03 '25 18:03 rotu