search

andreafrancia / trash-cli

Command line interface to the freedesktop.org trashcan.
GNU General Public License v2.0
3.63k stars 177 forks source link

Is whitelisting non-physical fstypes really a good idea? #256

Open jakanakaevangeli opened 1 year ago

jakanakaevangeli commented 1 year ago

For trash-empty and trash-list we decided not to include non-physical fstypes unless they are whitelisted by a hardcoded variable in list_mount_points.py. This was done in https://github.com/andreafrancia/trash-cli/commit/22e655b3d5facabd947661c3fbcff56aaf8f168e and https://github.com/andreafrancia/trash-cli/pull/230/files to fix bug https://github.com/andreafrancia/trash-cli/issues/214.

I argue that keeping a hardcoded whitelist of such fstypes is sub-optimal for two reasons:

  1. We will have to maintain it forever as long as new fstypes are being created, see https://github.com/andreafrancia/trash-cli/issues/250, https://github.com/andreafrancia/trash-cli/issues/255, I'm experiencing a similar issue with fstype=fuse.mergerfs https://github.com/trapexit/mergerfs, and so on.

  2. trash-put and trash-empty use a different heuristic for finding volumes, which can result in trash-empty silently ignoring trash directories created by trash-put. For example, after a few months of trash-cli usage, I've only found by chance that my trash folder in a mergerfs volume had tens of GBs of trashed files despite regularly running trash-empty.

As an alternative, I propose that, for trash-list and trash-empty, we go back to considering both physical and non-physical fstypes. We should ignore filesystems without a trash-directory, but this was already done in pull request https://github.com/andreafrancia/trash-cli/pull/237.

rcy17 commented 1 year ago

I agree on that whitelisting non-physical fstypes is not reasonable enough. However, I would propose that we can scan all mount points and ignore those with option ro or nodev, given that docker and other things may bring dozens of virtual filesystems.

In fact, I think what confuses the current strategy is just those filesystems marked as 'nodev' in /proc/filesystems but no 'nodev' in mount options, fuse for example.

andreafrancia commented 1 year ago

Yes we need to remove the need of using fs type whitelist. trash-empty, trash-list and trash-restore should handle data only from trash directories that are actually there.

trash-put should also print a warning when it is going to write in a trash-directory that would not be listed in trash-empy/trash-list.