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

[query] [electron-updater] Why is a subset of linux distros supported for autoupdates? #8370

Closed xyloflake closed 1 week ago

xyloflake commented 3 months ago

So as the title states, I was looking for a solution to make auto-update work on platforms like arch linux. I read a comment made by @develar that it is by intention - that some platforms are not supported. However I failed to find any issue that mentioned the "why". I'd like to know why it is unsupported.

Thanks in advance!

xyloflake commented 3 months ago

What are the problems associated with implementing it for some distros is another thing I'd like to know :)

mmaietta commented 3 months ago

I'm not familiar as to why more aren't supported. When I joined the crew, originally it was only AppImage. I implemented deb and rpm autoupdates after a significant testing period. For other platforms like arch linux, someone will need to do the implementation (happy to provide guidance) and test the different package managers that could be utilized.

Basically, for implementation, all that is needed are the spawn cmds for the other platforms to be supported. Here's how deb updates were implemented https://github.com/electron-userland/electron-builder/blob/28c7864bc061dfcc077e1476686e496c33e2e4e5/packages/electron-updater/src/DebUpdater.ts#L30-L40

xyloflake commented 3 months ago

I'm not familiar as to why more aren't supported. When I joined the crew, originally it was only AppImage. I implemented deb and rpm autoupdates after a significant testing period. For other platforms like arch linux, someone will need to do the implementation (happy to provide guidance) and test the different package managers that could be utilized.

Basically, for implementation, all that is needed are the spawn cmds for the other platforms to be supported. Here's how deb updates were implemented

https://github.com/electron-userland/electron-builder/blob/28c7864bc061dfcc077e1476686e496c33e2e4e5/packages/electron-updater/src/DebUpdater.ts#L30-L40

I would be super happy to implement for arch linux, am I allowed to do that? I thought of making a PR but was afraid of it being rejected because of what @develar said.

Even before the above instructions were posted, I had figured it out on how to implement it :)

I also have another patch for a repo I work on. It's for electron-builder 24.13.3 since that's the stable version, and it reduces the size by around 7MB! For larger codebases, this could result in an even more compressed installer, which is awesome. However, I've found some anomalies in the codebase that are bugging me before I make the PR. Could you check them out?

Thanks a ton! Can't wait to hear your thoughts! πŸš€

mmaietta commented 3 months ago

I'm not opposed to supporting arch linux if you're willing to put up a PR! We'll just need to flag it as a Beta Feature like the deb/rpm autoupdaters. I'm not sure what you're referring to that develar would reject. Would you mind providing a link to that conversation?

I'm not sure I follow w.r.t. the anomalies. The v25 CI builds are passing correctly.

Re: the node-gyp errors, I'm happy to take a look if you can put together a minimum reproducible repo? (Or do I just use the muffon repo itself with npm run package:all?)

xyloflake commented 3 months ago

I'm not opposed to supporting arch linux if you're willing to put up a PR! We'll just need to flag it as a Beta Feature like the deb/rpm autoupdaters.

Super cool! Will make a PR after patching 24.13.3 in the muffon repo and will test out in the betas if the autoupdates work well or not for arch! Sounds alright?

I'm not sure what you're referring to that develar would reject. Would you mind providing a link to that conversation?

Yes! Sure. Here is the issue in which develar states "Some platforms are intentionally not supported" - #3844

Just like you stated earlier; at that time only appImage was supported.

I'm not sure I follow w.r.t. the anomalies. The v25 CI builds are passing correctly.

Ah, the build info is outdated as of now. What I was trying to reference is

Electron builder internally resets the compression configuration option to normal for Windows, albeit without explicit rationale. Consequently, this patch intervenes to prevent the reset to normal exclusively for Windows systems, as the aforementioned algorithm cannot operate optimally on other platforms.

As in why it's reset to normal without any reason? It works fine without that... Also, only Windows is affected by this "normal" thing iirc.

Re: the node-gyp errors, I'm happy to take a look if you can put together a minimum reproducible repo? (Or do I just use the muffon repo itself with npm run package:all?)

Oh forget that please it may be outdated as of now because I see the CI/CD pipeline getting through fine. At the time of writing that, the CI/CD pipeline also failed πŸ˜„.

muffon does not have the error as we don't use electron-builder v25/v24 alphas for stability purposes. I'll check over the weekend if the issue persists or not and will notify based upon that!

Thank you so much for the support! Is there any community chat server for electron-builder? I'd suggest something like Slack/Discord/Matrix/etc. for chatting about and casually asking about stuff πŸ˜…

mmaietta commented 3 months ago

Very interesting comment regarding using package managers. I could understand the use case there; I guess I interpreted that electron apps were designed to allow distribution outside of package managers or App Stores.

Related, I discovered that there's an electron-builder channel within the Electron discord community https://discord.com/channels/745037351163527189/746375188358365204

xyloflake commented 2 months ago

Very interesting comment regarding using package managers. I could understand the use case there; I guess I interpreted that electron apps were designed to allow distribution outside of package managers or App Stores.

Were designed or were not designed? I feel like you wanted to say NOT designed?

Related, I discovered that there's an electron-builder channel within the Electron discord community https://discord.com/channels/745037351163527189/746375188358365204

Cool, but I think this deserves a dedicated discord server since doing all of development and asking questions might be a pain in one single channel.

Can I also have your discord and reach out so I can ask some more questions if you don't mind?

Thank you so much for the reply! Can you uhh maybe address the compression regarding concerns as well?

mmaietta commented 2 months ago

I interpreted that electron apps were designed to allow distribution outside of package managers or App Stores.

I say "were designed" because that's why I think auto-update support should be allowed for other linux distros. Under-the-hood, the auto-updaters are just calling the package managers anyhow.

--

That's a fair reason. I used to have the Github Discussions page but wasn't receiving enough notifications or had the hours to be able to monitor it. I removed it in favor of just having support tickets as Github Issues for traceability.

--

Re:

As in why it's reset to normal without any reason? It works fine without that... Also, only Windows is affected by this "normal" thing iirc.

I haven't the slightest clue as to why that logic exists. The commit is over 7 years old and was only part of a refactor, so the core reason is obfuscated to me. My best guess is that maximum compression causes the update differential to vary more widely, causing (almost) the entire app to be downloaded on each update instead of optimized for differential downloading.

xyloflake commented 2 months ago

I interpreted that electron apps were designed to allow distribution outside of package managers or App Stores.

I say "were designed" because that's why I think auto-update support should be allowed for other linux distros. Under-the-hood, the auto-updaters are just calling the package managers anyhow.

Okay, I got what you mean now! Yup, that definitely makes sense. So I'll be pushing over a PR most probably over weekend. Might be a bit late since I've got school and stuff...

That's a fair reason. I used to have the Github Discussions page but wasn't receiving enough notifications or had the hours to be able to monitor it. I removed it in favor of just having support tickets as Github Issues for traceability.

That's a very good reason imho. However discord also has a forum feature that makes having supports tickets like GitHub issues possible right from discord! Also, I meant to say that there should be a discord server for specifically contributing to electron-builder monorepo as it may need answers to quite a lot of questions people might have while contributing; just like the legacy stuff about the compression we are referring to.

Re:

As in why it's reset to normal without any reason? It works fine without that... Also, only Windows is affected by this "normal" thing iirc.

I haven't the slightest clue as to why that logic exists. The commit is over 7 years old and was only part of a refactor, so the core reason is obfuscated to me. My best guess is that maximum compression causes the update differential to vary more widely, causing (almost) the entire app to be downloaded on each update instead of optimized for differential downloading.

I think that the specific decision of "max compression but entire app is downloaded on autoupdates" should be passed on to the end developer(?). Since this is all legacy stuff, can we run some tests and get a rough idea of if that can be removed or not? If that can be removed, we can get some actual compression on the installers.

Explicit optimization benchmark: On muffon, before the custom compression type "ultra" was introduced, we used "store". This got us faster builds and no significant size difference between "store" and "maximum". The installers were about 91.3MB in size. After the implementation of "ultra": we now have 64MB installer binaries.

xyloflake commented 2 months ago

My best guess is that maximum compression causes the update differential to vary more widely, causing (almost) the entire app to be downloaded on each update instead of optimized for differential downloading.

I think we should run some real world testing before coming to that conclusion? I don't think the thing about differential builds is correct... The entire app is downloaded anyways (?).

xyloflake commented 2 months ago

@mmaietta how do I test stuff for electron-updater? Do I have to rebuild on CI for every code change?

xyloflake commented 2 months ago

@mmaietta I really tried hard to patch it but I'm having a VERY hard time testing the updates locally. Do you have any tips for testing auto updates locally? I literally have to package the application again and again for literally testing 1 line of code change and then wait for 12 minutes for build and finally figure out that I can't test the auto updates because I get checkForUpdatesAndNotify called, downloadPromise is null when I run the application.

Edit: I found out by reading the documentation -

Note that in order to develop/test UI/UX of updating without packaging the application you need to have a file named dev-app-update.yml in the root of your project, which matches your publish setting from electron-builder config (but in yaml format). But it is not recommended, better to test auto-update for installed application (especially on Windows). Minio is recommended as a local server for testing updates.

xyloflake commented 2 months ago

Also, one more question, how do you determine if the deb, rpm or pacman is to be used?

xyloflake commented 2 months ago

@mmaietta you haven't been replying since days. Are you okay?

mmaietta commented 2 months ago

Been on vacation πŸ™‚

Since this is all legacy stuff, can we run some tests and get a rough idea of if that can be removed or not? If that can be removed, we can get some actual compression on the installers. I think we should run some real world testing before coming to that conclusion? I don't think the thing about differential builds is correct... The entire app is downloaded anyways (?).

If you'd like to run some tests, please feel free to do so. The entire app is not supposed to be getting downloaded anyways unless the diff is genuinely that significant. There's been optimizations in the newer versions of electron-builder that optimizes the asar file ordering to improve differential updates as well.

how do I test stuff for electron-updater? Do I have to rebuild on CI for every code change?

This is the zsh alias set up on my mac for moving electron-builder/updater changes directly into a local project to avoid the CI/CD flow

alias resync="rsync -upaRv --include='*.js' --include='*.d.ts' --include='*.nsi' --include='*.json' --include='*/' --include='*.py*' --include='*.tiff' --exclude='*'  ~/Development/electron-builder/packages/./* node_modules/"

Call cmd resync from the project directory that is at the same level as electron-builder repo folder.

I literally have to package the application again and again for literally testing 1 line of code change and then wait for 12 minutes for build

For electron-updater code, this is the only route unfortunately. There's no way to test it via unit test due to the need for sudo.

Also, one more question, how do you determine if the deb, rpm or pacman is to be used?

This needs an updater added here: https://github.com/electron-userland/electron-builder/blob/553c737b2cf1ad835690f7db3c1907ae88944d15/packages/electron-updater/src/main.ts#L41-L50

And your updater target added here: https://github.com/electron-userland/electron-builder/blob/553c737b2cf1ad835690f7db3c1907ae88944d15/packages/app-builder-lib/src/targets/FpmTarget.ts#L280-L282

xyloflake commented 2 months ago

Been on vacation πŸ™‚

Welcome back! Hope you had fun.

Update: you were 2 days late lol. I've got autoupdates working on pacman and found a bug which I've also fixed. muffon is currently undergoing beta testing for autoupdates on Pacman and as soon as it's declared stable, most probably in 18 hours or so from the time of writing this, I'll make a PR. Everything is ready just not pushing it to be sure that it indeed would work.

Since this is all legacy stuff, can we run some tests and get a rough idea of if that can be removed or not? If that can be removed, we can get some actual compression on the installers. I think we should run some real world testing before coming to that conclusion? I don't think the thing about differential builds is correct... The entire app is downloaded anyways (?).

If you'd like to run some tests, please feel free to do so. The entire app is not supposed to be getting downloaded anyways unless the diff is genuinely that significant. There's been optimizations in the newer versions of electron-builder that optimizes the asar file ordering to improve differential updates as well.

Cool! That sounds incredible. I'd still want to run some tests regarding the efficiency of differential updates. Can you maybe point me to the code where the differential update logic is declared?

how do I test stuff for electron-updater? Do I have to rebuild on CI for every code change?

This is the zsh alias set up on my mac for moving electron-builder/updater changes directly into a local project to avoid the CI/CD flow

alias resync="rsync -upaRv --include='*.js' --include='*.d.ts' --include='*.nsi' --include='*.json' --include='*/' --include='*.py*' --include='*.tiff' --exclude='*'  ~/Development/electron-builder/packages/./* node_modules/"

Call cmd resync from the project directory that is at the same level as electron-builder repo folder.

Turned out you were late lol. I actually started an http server and changed the autoupdate server to generic so I could test it that way, an it worked superbly. I had no problems while testing that way.

I literally have to package the application again and again for literally testing 1 line of code change and then wait for 12 minutes for build

For electron-updater code, this is the only route unfortunately. There's no way to test it via unit test due to the need for sudo.

Yea, figured that out. Thanks.

Also, one more question, how do you determine if the deb, rpm or pacman is to be used?

This needs an updater added here: https://github.com/electron-userland/electron-builder/blob/553c737b2cf1ad835690f7db3c1907ae88944d15/packages/electron-updater/src/main.ts#L41-L50

And your updater target added here: https://github.com/electron-userland/electron-builder/blob/553c737b2cf1ad835690f7db3c1907ae88944d15/packages/app-builder-lib/src/targets/FpmTarget.ts#L280-L282

Again, I've already made the code changes and implemented the autoupdates for Pacman and it works fine as per our beta testers.

I found out that app-builder-lib adds a file called package-type through which you can detect what package was used for the installation.

Thank you for your help! PR for Pacman incoming ASAP.

xyloflake commented 2 months ago

Quick Demo for the time being. May not be able to make a PR today as I have the mentioned bug to fix.

https://github.com/staniel359/muffon/issues/189#issuecomment-2260652678

xyloflake commented 2 months ago

@mmaietta why does the app freeze when electron-updater executes the sudo commands? You can see that in the vid above

xyloflake commented 2 months ago

I think that's because spawnSync is used, which blocks the entire thread.

mmaietta commented 2 months ago

Only spawnSync can be used in order for the app to not quit early and for "restart after update" functionality to be preserved. There's no way to make it async as no progress "listener" is available from the package manager(s)

xyloflake commented 2 months ago

Only spawnSync can be used in order for the app to not quit early and for "restart after update" functionality to be preserved. There's no way to make it async as no progress "listener" is available from the package manager(s)

If I manage to make it async, can I make a PR?

mmaietta commented 2 months ago

That sounds lovely!

xyloflake commented 2 months ago

That sounds lovely!

Cool! Gotta work on it then!

mmaietta commented 2 months ago

Awesome, thank you!

Also, sorry, just saw this question in one of your previous posts:

Cool! That sounds incredible. I'd still want to run some tests regarding the efficiency of differential updates. Can you maybe point me to the code where the differential update logic is declared?

I think you can look at DifferentialDownloader.ts for the differential logic, but the logic is obfuscated amongst multiple files AFAICT in https://github.com/electron-userland/electron-builder/tree/master/packages/electron-updater/src/differentialDownloader

xyloflake commented 2 months ago

Awesome, thank you!

Also, sorry, just saw this question in one of your previous posts:

Cool! That sounds incredible. I'd still want to run some tests regarding the efficiency of differential updates. Can you maybe point me to the code where the differential update logic is declared?

I think you can look at DifferentialDownloader.ts for the differential logic, but the logic is obfuscated amongst multiple files AFAICT in https://github.com/electron-userland/electron-builder/tree/master/packages/electron-updater/src/differentialDownloader

Hi! Thank you for the reply~~

At the time of writing, I am preparing to make a PR for the pacman autoupdate support! Very excited for my first contribution here.

I would like to inform that I indeed did find a way to make quitAndInstall asynchronous. Even managed to preserve relaunch.

https://github.com/electron-userland/electron-builder/blob/84f290990915d7ffa0d2dfe0a1d739aeb2b21560/packages/electron-updater/src/ElectronAppAdapter.ts#L43-L45 I changed this to use the before-quit event instead of quit and used event.preventDefault() to hault the quit.

  onQuit(handler: (exitCode: number) => void): void {
    this.app.once("before-quit", async (event: Electron.Event) => {
      handler(exitCode))
    }
  }

However, there is one downside (not really) - you don't get the exitcode. Why I say "not really" is because, it's not mandatory since the autoupdater will run before the app quits and it doesn't have to worry about how the app quits.

So I removed these lines: https://github.com/electron-userland/electron-builder/blob/84f290990915d7ffa0d2dfe0a1d739aeb2b21560/packages/electron-updater/src/BaseUpdater.ts#L93-L96

And converted the other functions to async as well. It works flawlessly. Is there any reason to "why" before-quit was not used?

mmaietta commented 2 months ago

Well done!! Re:

Is there any reason to "why" before-quit was not used?

I don't recall there being any particular reason as to why. But it may be worth testing out the new async logic on deb/rpm builds as well just in case the before-quit does not work properly. I think it should be a safe change though

xyloflake commented 2 months ago

Well done!! Re:

Is there any reason to "why" before-quit was not used?

I don't recall there being any particular reason as to why. But it may be worth testing out the new async logic on deb/rpm builds as well just in case the before-quit does not work properly. I think it should be a safe change though

Thank you so much!

Then I'm all set to make a PR. As always, will use our beta testers to confirm that it's stable at first πŸ˜„

xyloflake commented 2 months ago

@mmaietta new question: why has the design of the electron.build docs website not changed since a very long time? The website has no dark mode, looks ugly when you take modern design patterns in consideration, looks outdated, has a very weird color scheme and it's hard to tell properties apart.

Why was the styling never changed and are we allowed to change it now since develar's no longer active in this repo?

xyloflake commented 2 months ago

Also @mmaietta, I really want to be a collaborator on this repo, what are the requirements/needs for it? Or is it just a matter of time and contributions?

xyloflake commented 2 weeks ago

@mmaietta should this be closed now?