Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

empty folders aren't removed upon package deletion #1

Closed
vgmoose opened this issue Feb 21, 2018 · 3 comments
Closed

empty folders aren't removed upon package deletion #1

vgmoose opened this issue Feb 21, 2018 · 3 comments

Comments

@vgmoose
Copy link
Member

vgmoose commented Feb 21, 2018

No description provided.

@vgmoose
Copy link
Member Author

vgmoose commented Jul 8, 2018

In addition to this, if there are any remaining files that aren't empty folders (such as left behind configuration files that weren't removed alongside the uninstall) the frontend should prompt the user whether or not they wish to completely remove the entire package hierarchy, or allow the empty folders to remain.

This is a little complicated, because the new definition of a "package" has no concept of a base folder, (a package can install content to anywhere on disk, even multiple places) so this would involve tracking what the specific empty folders were/should've been and then ensuring to only delete files from those directories. I don't think this approach is very good.

Maybe another solution would be to introduce a new manifest/install file flag like "C: " which stands for configuration, so that we know to treat it like a "G: " (don't overwrite) but also, make sure to remove it upon uninstallation. For instance, if we know that save.data is going to be generated in a folder that should be removed, then it can be marked as C, and then even though it won't be downloaded and installed within the package, that it should be removed upon deletion anyway.

And then the other layer is, sometimes the user may want their config files (like save.data) to remain even though they have uninstalled the app. Which is why these files should be prompted for whether or not they should be deleted. This also means that if the user adds any unexpected files to the package (such as for an emulator, if they were to add their own game roms) that they will never be uninstalled, on the grounds that the package manager didn't have knowledge of this file and it's not ours to delete.

That may result in some directories that were created by Get to remain after package uninstallation, provided that the user manually copied some files into that folder. But I think that is good design and better than the alternative of blindly trying to guess whether or not certain files should be deleted on the user's disk. (Another approach might be to move all undecided file to a trash folder, for the user to sort later, but that seems like overkill compared to leaving behind a folder that the user has previously interacted with anyway (plus in the case of roms and emulators, the emulator could be re-installed, and roms would remain on disk as expected))

@vgmoose
Copy link
Member Author

vgmoose commented Aug 1, 2018

committed in 1d37910 is code to remove all empty folders, recursively, from a given top-level directory. This should be used as an option within settings of the frontend (eg, settings -> remove all empty folders) in order to provide cleanup for people that already uninstalled apps on an older version.

For instance, it's not ideal to go and remove all empty folders on the SD card every time a package is removed.

But another short term solution, excluding the manifest changes in the comment above, would be to go through the manifest and get a list of directories that the package interacts with. These would not be recursive, so just a flat list of directories. Then go through those directories (from longest path to shortest) and remove any empty folders from them at a flat level, without recursing.

@vgmoose
Copy link
Member Author

vgmoose commented Sep 24, 2018

in 7767f77, the changed described in the last paragraph of the previous comment was added.

Relevant code: https://github.com/vgmoose/libget/blob/7767f7793ad19b33e5982c4836c57132add81673/src/Package.cpp#L214-L250

In short, while it's going through the manifest, it adds every dirname of the files in the manifest to a unique set of interesting directories. Then it sorts this from largest to smallest, and goes through it to calculate any intermediate directories that aren't already in the set. Then it sorts the new list of the unique folders + these intermediate ones again from largest to smallest. After this, it goes through the list and calls rmdir on them, removing them only if they are empty..

@vgmoose vgmoose closed this as completed Sep 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant