Revert "fix(nsis): NSIS Uninstaller registry entry format change (#4069)"

[elvisvoer]

It looks like the change of the registry format has not been well thought through and it causes lots of issues (#4092 and #4126).

The change was made for pure pedantic reasons and didn't consider backwards compatibility with older installer. In our case, we are wrapping the NSIS package with an MSI wrapper that has a bit more functionality than the one provided by electron-builder which no longer works after this change.

My suggestion is to revert the commit all together and leave the registries as they were before.

This reverts commit 7518aee.

[stilettk]

There is also another issue regarding this commit:
#4057

[damianobarbati]

@elvisvoer was this released? Or do you know when this is going to be released?
I have 2k users (free application) with broken auto-update (in infinite loop!) 😱

[elvisvoer]

@damianobarbati pre-release 22.2.0 has it

[menelike]

Folks, IMHO this should end up in 21.2.x as long as 22.x.x is not production-ready.

Second, it should explain how already affected and released applications should be cleaned up. Or even better https://github.com/electron-userland/electron-builder/blob/master/packages/app-builder-lib/templates/nsis/include/installer.nsh should handle that cleanup.

Don't get me wrong, this is an open-source project and I am very grateful for all the efforts which have been put into this project, but this change has made the life of many developers already miserable
#4092
#4018 (comment)
#4069 (comment)
and just reverting this for a soon to come release is simply not enough.

I would have contributed myself, but I just have no experience with NSIS scripts (and the risk of making the situation worse is just too high)

Just my 2 ¢.

[mastergberry]

So...what are people supposed to do who migrated to the newer version?

We need a path back from new -> old...

[elvisvoer]

@mastergberry we do this in an init script that we include in our windows yaml file

; to make sure we execute this code only when the installer is invoked
!macro customInit
DeleteRegKey HKLM "Software\Microsoft\Windows\CurrentVersion\Uninstall\{ProductID}"
DeleteRegKey HKLM "Software\WOW6432Node\Microsoft\Windows\CurrentVersion\Uninstall\{ProductID}"
!macroend

[mastergberry]

Ok thanks @elvisvoer I will attempt this the next time we upgrade...just petrified because I had to fix like 3 different "bugs" because of the original chnage with hacky nsis scripts...hope I can just nuke those 2 new keys and then watch everything work...

[damianobarbati]

@elvisvoer are you using that NSIS include with the pre-release 22.2.0 or with the current release 21.2.0?
Is it fixing the install/autoupdate problem for you?

[elvisvoer]

@mastergberry yeah, it should work. We are already in alpha with our app and I got to verify the update from the faulty version to the pre-release one.

@damianobarbati we use the pre-release but we've never had problems with the update (or none that we know of) so maybe if you give me some more details I can help.

[damianobarbati]

@elvisvoer your script seems to work!
It prevents my installer from breaking when the GUID changed from xxx-xxx-xxx to {xxx-xxx-xxx}.
I'm currently testing upgrades from several versions of my previous app to the one running your include.nsh.

Is it safe to always keep that script in place? To clean previous uninstall entries from registry / control panel?

[elvisvoer]

@damianobarbati I think it is. This is what we do at the moment. However, the scripts will pile up so a way to mitigate is to make a "required" release: a version that all old applications will see as last update and which knows where the new updates are, this requires a bit of infra changes but it is a way to clean up technical debt

[jmeinke]

@elvisvoer Wouldn't it make sense to extend your script for the HKCU current user installation entries?

; related to:
; https://github.com/electron-userland/electron-builder/issues/4092
; https://github.com/electron-userland/electron-builder/pull/4443#issuecomment-562962960
!macro customInit
  DeleteRegKey HKCU "Software\Microsoft\Windows\CurrentVersion\Uninstall\${UNINSTALL_APP_KEY}"
  DeleteRegKey HKCU "Software\WOW6432Node\Microsoft\Windows\CurrentVersion\Uninstall\${UNINSTALL_APP_KEY}"
  DeleteRegKey HKLM "Software\Microsoft\Windows\CurrentVersion\Uninstall\${UNINSTALL_APP_KEY}"
  DeleteRegKey HKLM "Software\WOW6432Node\Microsoft\Windows\CurrentVersion\Uninstall\${UNINSTALL_APP_KEY}"
!macroend

[elvisvoer]

@jmeinke we use the oneClick installer which installs for all users but I think your script cover more cases so it's better

[faern]

This change broke apps again. As soon as the GUID changes, an upgrade won't find the old app and you end up with two apps in the Windows list of installed programs. We have previously had to work around this by custom code renaming registry entries ourselves in the installer, and now we needed one more of those hacks (mullvad/mullvadvpn-app#2023).

This issue is a thread about the problem, and how the best would be to be able to specify the GUID as a custom string. Virtually every program uses the program name instead of a generated UUID anyway: #4247. Sadly a bot closed the issue, but it's still highly relevant. Please re-open if you have the access to do so.