Optional rollback Cleanup() on update

[axllent]

Hi! Unless I'm missing something, I cannot see any way to automatically remove the rollback *.bak file after a successful update. Could this be something that can either be manually run (*updater.Updater.Cleanup()) or otherwise passed as an Updater variable, as I don't see the point of always having *.bak files left on the system when the upgrade was successful?

[mouuff]

Hello! You are right. When you are on windows calling Cleanup() by yourself is not going to work because the temporary file you are trying to delete is your own executable which have been renamed (and you can't delete yourself on windows).

Here is few options:

• Call Cleanup() only on the next run.
• Call Cleanup() from another process when the program is closing. (This is a function we could make available natively, lets call it CleanupLater())
• After an update you can force the program to restart (and then call Cleanup())

Something else we could do is to move the files to a temporary directory, that way it will be cleaned up on next reboot, but I m not a big fan of doing that.

[axllent]

I came across this issue too with my module. I suggest a different approach - detect if the OS is Windows, and if so move the file to the os.TempDir(), else delete it (https://github.com/axllent/ghru/blob/master/ghru.go#L265-L275). This keeps !Windows systems clean at least, and Windows tidy.

I don't think that running cleanups on (re)start is a good approach as the user running it isn't necessarily an admin/root on the second run. When upgrading it on Linux for instance, one would typically run something like sudo myapp upgrade or whatever, but then go back to running it as a user straight after.

[mouuff]

I checked how other updaters are doing this self replacement part. Most of the time they just rename the executable and keep it in the same directory. (just like us). This is what go-selfupdate and go-update are doing. Only difference with us is that they also try to self remove, and if it fails they just hide the binary. In their case the behavior is different on windows and on linux, and I m not a big fan of that.

Example: https://github.com/inconshreveable/go-update/blob/8152e7eb6ccf8679a64582a66b78519688d156ad/apply.go#L168

After doing some research I also found that we could probably use this (but tbh I don't really consider this solution atm): MoveFileEx(szExistingFile, NULL, MOVEFILE_DELAY_UNTIL_REBOOT); Doc: https://docs.microsoft.com/en-us/windows/win32/api/winbase/nf-winbase-movefileexa?redirectedfrom=MSDN

I also like the solution of running a new process after exiting. I used this solution on another project and we never had issues with that.

Since other projects are doing roughly the same thing as us. I don't think this is a priority right now.

If you find useful information on that subject then fell free to post it here. I m still open to your solution! but I really want to make sure it is not going to cause problems. :)

[axllent]

I don't think that other projects are doing it the same way though - Yes, they almost all try initially rename the binary because on Windows you cannot delete the running executable, and on all systems it provides both a simple permissions test (that the user running the update has permissions to write to the path), and finally provides a temporary rollback if the update fails due to extraction problems etc. But the point wasn't the renaming of the current binary to *.old, it is that go-rocket-update leaves the old binary in the same path after a 100% successful update - without cleaning it up.

This leaves myapp and myapp.bak in my path after an update, and requires me to run myapp a second time (with the adequate user permissions) after an update just to auto-delete myapp.bak - this logic just doesn't make any sense to me, and it is messy.

Personally I can't see how trying to immediately remove the old binary after a successful update can cause any problems - either a single trial & error method can be used (try delete, if that fails try move to os.TempDir()), or a switch based on OS can be used as Windows is the only system that I'm aware of that doesn't allow deletion of the running executable. If that fails (which is unlikely because you were already able to rename the original and write a new file, set permissions etc), well then one is simply left with what you have now; an *.old file in your path. I don't see what problems this can cause?

The first example you give tries to hide the .old file (Windows-specific) if the deletion process fails - which is a method I haven't seen before, but that seems a bit messy as hideFile() is very Windows-specific, and it simply ignores errors). I would not suggest using this method unless there is some security consideration why it wouldn't be moved to os.TempDir().

Again, just my feedback/thoughts ;-)