Added: Support for Specifying Pre-Uninstall Script

Sewer56 commented 5 months ago

Summary

This PR adds the ability to specify a script that will be executed as a pre-uninstall step when the application is uninstalled via the InnoSetup generated Windows installer.

An example test callback I made with my work project. (RIP Emoji)

Purpose

Certain kinds of applications require custom cleanup logic for deleting leftover files left behind when a program is removed. This is common in applications that allow a person to specify custom locations for storing user data.

A common, universally understood example is Steam. In Steam you can add additional storage locations where you want your games to be stored. For example, on a secondary HDD or SSD.

In other situations, you may also want your application to perform an additional series of steps (for example, undo external changes) made to data outside of the Application folder. Think of programs that 'patch' external components, for example ExplorerPatcher.

As this sort of logic cannot always be statically declared ahead of time, a 'hook-in' system is required to perform cleanup steps on uninstall.

Key Changes

Added a new SetupUninstallScript configuration option which specifies a path to a script file relative to the application directory.
- Any file can be specified, as long as the extension has an association.
- Though I personally recommend batch files .bat, as just about every Windows system will support them.
The SetupUninstallScript is executed from the uninstall wizard before rest of the uninstallation continues.
The uninstall script is invoked via the [UninstallRun] section in the generated InnoSetup script.

In addition:

Updated the README and CLI example help with documentation for the new SetupUninstallScript option.
Added misc. test code for the new variable.
- Tested like the other config variables, and not much more.

Design Motivations

I have decided to limit this to a file name, with no escape / quotes for the time being. This is in order to keep things simple, for both us and the end user.

Some Related Ramblings Below.

I've originally implemented this to work such that you could specify a full commandline to execute, for instance YourApp.exe --uninstall-app. However I wound up stepping back from that design for the time being due to concerns around parameter quoting and testing.

In particular, splitting up an input into a parameter array is non-trivial, there is a lot of additional mental complexity involved. For example, say the user has a space in their app name and wants to run "Your App.exe" --uninstall-app. Because many of the existing fields say 'this maps to X in InnoSetup' it is unclear to the user if:

You should use InnoSetup's escaping style ""Your App.exe"" --uninstall-app (escape with ").
- In which case part of InnoSetup leaks through the PupNet abstraction.
You should use .NET escaping style. \"Your App.exe\".
You should not escape at all.

Unfortunately, for better or worse, the answer here is, inconsistent. The custom ini parser in PupNet strips the quotes by default if the value starts with a quote. So you might wind up with something like this being valid YourApp.exe "Some Folder"; but if the EXE name has a space, you suddenly have to add a second set of quotes like ""Your App.exe" "Some Folder"".

This unfortunately creates inconsistency and might lead to frustrating users.

So I decided to keep it simple, point to a script and let a script handle it for the time being. This also makes things potentially easier to test for some niche use cases.

Support for a commandline could still be added in the future, either directly in the current field, or in a separate field. It is just that a more in-depth design would probably be required there.

In any case, if anything else is missing here, let me know.

kuiperzone commented 5 months ago

Let me take a look at this, and I'll merge it in due course. Thank you.

erri120 commented 5 months ago

+1

erri120 commented 5 months ago

Thanks for the merge, will this change be in 1.9 or some intermediate release 1.8.1?

kuiperzone commented 5 months ago

Hi Erri,

It will be released with 1.9. I can see that you put a lot of effort into the commit and I am sorry that I cannot tell you when that will be. I have other things which take up my time at the moment. Please do feel free to fork the project if that's what you want to do and I'll be glad to answer any technical questions that I can. Otherwise, there will be a release at some point in the future (prob several months).

erri120 commented 5 months ago

Hi Erri,

It will be released with 1.9. I can see that you put a lot of effort into the commit and I am sorry that I cannot tell you when that will be. I have other things which take up my time at the moment. Please do feel free to fork the project if that's what you want to do and I'll be glad to answer any technical questions that I can. Otherwise, there will be a release at some point in the future (prob several months).

Thanks for the info and no worries 👍🏻

kuiperzone / PupNet-Deploy