Fixes #119 (removes reg keys during uninstall)

[hashhar]

I added some code to remove the registry key created at HKCU:\Software\OpenLiveWriter during uninstall.

[hashhar]

Why are the uninstalled, local_key, machine_key and key variables used? I can't see them being used anywhere (at least in this file)?

[kathweaver]

I've tried this several times and the settings for OpenLiveWriter are still on my computer. The referenced keys are gone.

[hashhar]

Okay. I don't I have the latest version installed. I'll try again.

Thanks for letting me know.

[kathweaver]

The local directory Is still there, even though the keys are gone.

[hashhar]

Oh, got it. The settings are getting reset but the folders aren't deleted. I'll add some code to remove the folders as well.

[kathweaver]

Yes, the registry keys are gone! So good job on that.

[hashhar]

It seems that this issue is an inherent design problem with Squirrel. Both GitHub Atom and VSCode had issues reporting this.
Some people were able to get the directories deleted upon installation when the installation was done as an administrator.
So what should be the next step? Should we keep this on hold or make a script that runs after uninstallation which will do the cleanup. (I can write the script but am confused about how might I launch the script after uninstallation).

[hashhar]

I found this. Seems like Squirrel has some events we can tap into.

static bool ShowTheWelcomeWizard;

using (var mgr = new UpdateManager(updateUrl))
{
    // Note, in most of these scenarios, the app exits after this method
    // completes!
    SquirrelAwareApp.HandleEvents(
      onInitialInstall: v => mgr.CreateShortcutForThisExe(),
      onAppUpdate: v => mgr.CreateShortcutForThisExe(),
      onAppUninstall: v => mgr.RemoveShortcutForThisExe(),
      onFirstRun: () => ShowTheWelcomeWizard = true);
}

[hashhar]

Would this work (in OpenLiveWriter.ApplicationMain.cs? I handle the uninstall event and remove the shortcuts and the registry keys?

private void ConfigureSquirrel()
        {
            SquirrelAwareApp.HandleEvents(onAppUninstall: OnAppUninstall);
        }

        private void OnAppUninstall(Version obj)
        {
            var downloadUrl = UpdateSettings.CheckForBetaUpdates ?
                UpdateSettings.BetaUpdateDownloadUrl : UpdateSettings.UpdateDownloadUrl;
            using (var manager = new UpdateManager(downloadUrl))
            {
                manager.RemoveShortcutsForExecutable("OpenLiveWriter.exe", ShortcutLocation.Desktop);
                manager.RemoveShortcutsForExecutable("OpenLiveWriter.exe", ShortcutLocation.StartMenu);
                manager.RemoveShortcutsForExecutable("OpenLiveWriter.exe", ShortcutLocation.AppRoot);
                string OLWRegKey = @"Software\Open Live Writer";
                Registry.CurrentUser.DeleteSubKeyTree(OLWRegKey);
                manager.RemoveUninstallerRegistryEntry();
            }
        }

[anaisbetts]

If you handle Squirrel events, you need to handle all of them (i.e. opting into Squirrel Events via the Attribute means that you're saying "I want to handle everything myself", which means you need to handle install and update (usually just by calling manager.InstallShortcutsForExecutable)

[dnfclas]

@hashhar, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, DNFBOT;

[martinwoodward]

How you feeling about this change @hashhar? I see you have a bit of stuff commented out still, do you want to remove those parts entirely before we merge?

[hashhar]

This is partially finished. The chocolatey uninstaller has been fixed but the squirrel uninstaller still faces the same issue.

If you want to merge only the chocolatey part, I'll have to undo the last commit.

[hashhar]

It is now ready for merging.

Just for ease of reviewing it:
The updated chocolateyUninstall.ps1 does the following:

1. Removes the data in AppData\Local and AppData\Roaming.
2. Deletes the HKCU:\Software\OpenLiveWriter registry key so that a fresh installation will prompt for reconfiguration of OpenLiveWriter.

[ScottIsAFool]

@gep13 would you be able to quickly verify the chocolatey side of things for this PR please?

[gep13]

@ScottIsAFool LGTM

[kathweaver]

Interesting -- when I try to test this Appveyer throws an error message "Project Not Found or access denied". It opens this address: https://ci.appveyor.com/project/ScottHanselman/openlivewriter/build/1.0.229

@ScottIsAFool do you know why?

[ScottIsAFool]

Ah, interesting. Yeah, this is because we have moved to the dotnetfoundation's AppVeyor account and ScottH has closed his account which is what we used to be using. If @hashhar were to make an arbitrary commit to this branch and push it, it would force a rebuild on appveyor using the new account and then you can test it.

[kathweaver]

Just deleted keys manually and files, did an install, then uninstalled, and the keys and directory are still there. When I got to the directory, there is a .dead and an Update application file left. I am not using Chocolately.

[hashhar]

Yup. This won't fix the normal install. This tracks only the Chocolatey part of it. I will try again for the normal installer with a fresh approach.

[RobDolinMS]

@kathweaver Would you mind testing this again with the most recent change?

[hashhar]

@RobDolinMS The change I made only affects installations done using Chocolatey. I couldn't get the Squirrel installer to do the same thing. I'll give it another try.

[ferzkopp]

Smoke-tested https://ci.appveyor.com/api/buildjobs/2blqkh9smt88a9j4/artifacts/Releases%2FOpenLiveWriterSetup.exe without success.

What is supposed to be removed? Everything under HKCU:\Software\OpenLiveWriter?

Steps:

• uninstalled existing OpenLiveWriter setup via control panel
• cleaned up some searchable files
  ...\AppData\Local\Microsoft\CLR_v4.0_32\UsageLogs\OpenLiveWriter.exe.log file
  ...\AppData\Roaming\OpenLiveWriter folder
• installed https://ci.appveyor.com/api/buildjobs/2blqkh9smt88a9j4/artifacts/Releases%2FOpenLiveWriterSetup.exe
• previously configured blogs are there
• uninstalled OLW again via control panel
• check registry - lots of entries (see attached screenshot)
  
• reinstall official OLW again
• previously configured blogs are there
• repeat procedure, using the modern uninstaller (Settings - Apps & Features).

[hashhar]

@ferzkopp Currently I've only been able to make it work for Chocolatey packages. I've had trouble trying to do the same with the Squirrel installer (the one you get from the .exe). I really want to try again but this year has been specially busy (final year of college) and I haven't had the time to look more into it. I'll work on it again during the Christmas week.

[ferzkopp]

@hashhar Got it.

I just looked through the Squirrel documentation - it doesn't mention "Uninstall" anywhere even once. The Squirrel code also does not seem to provide a hook to call an external script on uninstall. Looks like this will be a feature request dependency for Squirrel (opened issue to track this: ).

Also, the Squirrel version used by OLW is out of date: createinstaller.cmd references .\src\managed\packages\squirrel.windows.1.2.1\tools\Squirrel.exe (from Nov 5, 2015) but Squirrel is up to 1.5.1 already. Probably this should be updated as well.

Question: Could one use something more standard such as WiX? I could help with that, i,e. creating a simple WiX installer config that can build an MSI (or EXE) from a bin-drop isn't hard.

[hashhar]

I think one of the reasons Squirrel was chosen is because it's extremely fast. I think it has hooks because I have a stale branch lying around somewhere where I tried. I'll look into more detail and report within a day.

[ferzkopp]

Squirrel issue got closed already with some info; code provides a hook via the C# based onAppUninstall event handler. Not the best design in my view due to the tight coupling of OLW code with Squirrel, but this should get the job done with few DeleteSubKeyTree() calls in the handler.

[anaisbetts]

@ferzkopp It is the best design because Squirrel's event handler is just interpreting command-line parameters on your behalf, you're free to do it yourself instead :)

[hashhar]

I have it working now. I'll test some more and there are a few Debug messages and exception handling to be added. I think two days would be enough for me.

[hashhar]

@paulcbetts I have two questions.

1. How do I make the installer add icons? [FIXED]
2. How does the Uninstall method work?
3. Also, can I set up the SquirrelAwareApp.HandleEvents in different places. eg. I set the install, update and firstRun methods in one file, while I set the uninstall method much earlier in the file? [It doesn't seem to work that way?]

[hashhar]

This is working now. Could somebody manually test this?

Also, should I write some tests around this?
And should an option be provided during uninstallation to decide whether to keep data or remove it?

[ferzkopp]

AppVeyor did not seem to have created a bin drop with your changes.
...
Collecting artifacts...
No artifacts found matching 'releases*' path

[anaisbetts]

How does the Uninstall method work?

All of the Squirrel hooks work the same, it's super simple - they just run your executable with a special command line, and wait for it to finish - on install, it's --squirrel-install x.y.a.b (i.e. your version #), and on uninstall, it runs --squirrel-uninstall x.y.a.b (also your version #).

Also, can I set up the SquirrelAwareApp.HandleEvents in different places. eg. I set the install, update and firstRun methods in one file, while I set the uninstall method much earlier in the file? [It doesn't seem to work that way?]

So, SquirrelAwareApp.HandleEvents is just a helper method. Basically the idea is, if you're invoked with --squirrel-[install,uninstall,update,obsolete], your app should do what it needs to do,
then instantly exit, because Setup itself is waiting on you to finish. Lots of people didn't grok this or messed up that 2nd part, so I tried to make a helper for them that would run a callout, then exit immediately. HandleEvents is just a switch statement + Environment.Exit(0).

Now that you know how it works and that it's super dumb, you can write code to handle it any way you want, as long as you guarantee that, as fast as possible, you do what you need to do for uninstall, then exit and return a zero error code

This is working now. Could somebody manually test this?

Testing Squirrel hooks is super super easy, because all you need to do is:

1. Copy in the files as if they were installed, whatever
2. Just run the executable with --squirrel-uninstall X.Y.A.B in Command Prompt and see what it does. Attach a debugger, add a bunch of Console logging, whatever. Go to town!

Unlike other install frameworks, you don't need to create an installer package whenever you want to test your custom actions, you just run the app yourself

[hashhar]

@ferzkopp See #541. You can test it out locally in the meantime. Or, if you want I can create a single combined PR.

[ferzkopp]

@hashhar I'd like to test an "official" build created by the build system - not a local build. How can one trigger AppVeyor to rebuild this branch?

[hashhar]

It won't ever rebuild it correctly unless I merge the commits from that other PR. I guess I should do that. 🤔

[hashhar]

@ferzkopp Here, I fixed it.

[ferzkopp]

Tested the latest binary - registry cleanup worked, but the Desktop shortcut wasn't removed.

It still points to
"C:\Users\ferzkopp\AppData\Local\OpenLiveWriter\Update.exe" --processStart OpenLiveWriter.exe
in
"C:\Users\ferzkopp\AppData\Local\OpenLiveWriter\app-0.6.0"
and when double clicked, the left behind Update.exe creates an error log in
C:\Users\ferzkopp\AppData\Local\OpenLiveWriter\SquirrelSetup.txt
containing
2016-12-17 15:02:31> Program: Starting Squirrel Updater: --processStart OpenLiveWriter.exe
2016-12-17 15:02:31> Unhandled exception: System.IO.DirectoryNotFoundException: Could not find a part of the path 'C:\Users\ferzkopp\AppData\Local\OpenLiveWriter\packages\RELEASES'.
...

[hashhar]

Wow, that's weird because in my case the Desktop shortcut wasn't even created. Lemme check if I missed something.

EDIT: Found the fix. It was a reported Squirrel issue and has been fixed in one of the newer packages. I'll update the nuget packages.

[ScottIsAFool]

Changes look good to me (although I haven't run it)

[hashhar]

@ScottIsAFool It works now. I had to change quite a lot.

A summary here:

• Added code to handle Squirrel events for Update, Install, Uninstall and FirstRun. This allows proper clean installs and uninstalls.
• Updated the nuget packages because there were a lot of upstream fixes. I did make sure that none of the updated packages had any changed behaviour or breaking changes.
• Fixed the build script for newer versions of Nuget (they don't seem to support w.x.y.z anymore).

Also, I needed feedback on a thing. Would it be good to add a dialog box asking the user if they want to retain their preferences both at the time of a new installation or uninstallation?

[anaisbetts]

Also, I needed feedback on a thing. Would it be good to add a dialog box asking the user if they want to retain their preferences both at the time of a new installation or uninstallation?

You cannot add dialogs to Squirrel installs. Your installer/uninstaller has ~15sec to finish whatever it's doing, and dialogs can block longer than that.