search

Squirrel / Squirrel.Windows

An installation and update framework for Windows desktop apps
MIT License
7.35k stars 1.03k forks source link

squirrel packages entire electron.exe in every update (electron-winstaller) (fix already in master) #1784

Closed MichaelBelousov closed 2 years ago

MichaelBelousov commented 2 years ago

Squirrel version(s) 2.01-eef3746

Description electron-winstaller's vendored squirrel's bsdiff fails to diff electron.exe's which differ only by 4 bytes, the 4 references to updated versions embedded in the exe. This causes squirrel to package the entire electron.exe every update.

As discussed in #1287, this is already fixed in master. But for some reason it was never cherry-picked to the development branch from which the electron-winstaller vendored binaries are from

looks like master has a fix here: afe47c9 but the version in electron winstaller is here: eef3746 according to git branch --contains eef37460ae that squirrel version (2.0.1) comes from the develop branch which does not have this fix

The PR to master where a fix was added https://github.com/Squirrel/Squirrel.Windows/pull/1421

Steps to recreate See #1287

  1. use electron-forge/electron-packager/electron-winstaller to build a squirrel package for your electron app
  2. bump the patch version for your app
  3. use electron-forge/electron-packager/electron-winstaller to generate an update for your app this invokes their vendored 2.01 squirrel

Expected behavior A small-ish patch for the app's renamed electron.exe executable is generated rather than the entire file placed in the update.

Actual behavior squirrel's bsdiff fails because there is only is no bsdiff-extra-data generated by diffing this case, and there is a divide by zero error when trying to write an empty byte stream for the diff. The error is taken as reason to just stuff the binary file in the update if diffing failed.

MichaelBelousov commented 2 years ago

@anaisbetts (sorry for the ping, I believe you're still the de facto maintainer/permission-holder) if I make a PR cherry-picking this fix into the development branch and then talk to the electron-winstaller maintainers about releasing a new version picking up the change, will the PR get accepted?

MichaelBelousov commented 2 years ago

@robmen is this something that you can accept via PR?

robmen commented 2 years ago

@MichaelBelousov so I'm clear, is this simply a commit (or two) in master that didn't get ported to develop? If so, I can cherry-pick them in my next round of changes (tonight).

MichaelBelousov commented 2 years ago

yes exactly. The description above references the exact commit. I've been using a fork with the one commit cherry-picked. For this to be picked up by electron consumers such as myself, a new version of electron-winstaller would have to be released as well consuming this. Do you know who can be asked from their side to do so?

robmen commented 2 years ago

Okay, I have the changes in master in develop adn everything should be straightened out there now. I'm evaluating my next step in the process automation.

However, your real question is what is the process for election-wininstaller to get an updated Squirrel with all these fixes. For that, I still need to lean on @anaisbetts as I haven't done that yet. Perhaps this should be the time I learn from @anaisbetts how but that means it'll happen pretty slowly. 😄

MichaelBelousov commented 2 years ago

@robmen would it be appropriate for me to open a new tracking issue for updating the electron-winstaller package with the latest squirrel?