-
-
Notifications
You must be signed in to change notification settings - Fork 104
Description
When try_legacy_migration fails to remove any of the old app-* dirs, the user is presented with an error message, that the app is now "corrupted" and needs to be re-installed. However, the app can actually be started and is working fine, since deleting the old app-* dirs is the final step before re-starting the app.
In the old Squirrel implementation un-deletable app-* directories (for example due to an open cmd terminal in that directory) would just accumulate in this case, but since they are attempted to be removed on each update, they would be cleaned up eventually.
I think failure to remove these directories should not cause the whole update to fail. However, leaving them around is also not a good idea. Since the migration only happens once, they would probably stay forever, which is not ideal.
The remove_dir_all crate is used to delete the old directories. Currently, it does not support recovery from error, it just bails on the first non-deletable file or directory. There is an old feature request there to handle these kinds of situations better: XAMPPRocky/remove_dir_all#41
My plan would be to implement the feature from that request, with the additional option to also mark the files/dirs for deletion upon reboot by Windows. (I don't think that we need the user to ask to reboot - the deletion is not so important and can wait until the machine is rebooted for other reasons)
I noticed that Velopack already uses a forked version of remove_dir_all. I am not sure if it is better to implement the more general feature (error transformer) upstream and merge that into the fork, or just add the specific behavior needed for Velopack in the fork.