velopack
Better handling of non-deletable files or directories when migrating from Squirrel
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: https://github.com/XAMPPRocky/remove_dir_all/issues/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.
Hi there,
- I do think that handling failures and continuing via remove_dir_all would be nice/better but wouldn't solve the problem entirely.
- Agreed that we should not fail in the case the legacy folders can not be removed. (just log a warning)
- We may want to search for and try to clean up "app-*" folders on each new update apply to fix this problem fully.
Ideally we'd have all three, but the last two would probably be good enough.
@Noah1989 I saw your comment about using MoveFileExW with MOVEFILE_DELAY_UNTIL_REBOOT, note that this flag requires admin privileges so will not be possible to use from Velopack. Just trying again later (eg. my point 3) will be the better solution in this case.