andreafrancia / trash-cli

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

If a trash fails before everything is deleted, all the files that *were* deleted *cannot be restored* #322

Closed mlncn closed 6 months ago

mlncn commented 10 months ago

Describe the bug

A trash command can delete many files but then fail to delete the directory they are in, for example. In this case—often when trash-restore is more needed than ever—the deleted files, the whole trash attempt, is not listed in the trash-restore listing.

What precisely happened is i saw a weird folder, and went to delete it before checking if it was a symlink.

node_modules/Butaro -> ../web/themes/butaro

trash node_modules/Butaro/ gave the message:

trash: cannot trash directory 'node_modules/Butaro/'

Then i looked at the folder, realized it was a symlink… and realized it had deleted a very important directory that i did not want to delete.

Then went to look at trash-restore… and the latest, most recent thing listed there was a week ago. No way to restore or recover from this unintentional deletion!

trash-cli version

trash --version
0.23.2.13.2

Are you using the latest version of trash-cli?

No— pretty close though, two versions behind.

Have you tried if the bug is present in the latest version of trash-cli?

No

Please, install the latest version of trash-cli and try again before submitting the bug

My apologies but this is major enough that i am filing and will try this later when i am not trying to recover from the relevant trash-cli-related disaster here.

First of all you need to uninstall any previous version of trash-cli::

$ [sudo] pip uninstall trash-cli # remove the previous version (with pip)
$ [sudo] apt-get remove trash-cli # remove the previous version (with apt)
$ [sudo] yum uninstall trash-cli # remove the previous version (with yum)
$ ... # refer to the package manager of your distribution

Then install the latest version from git::

$ pip install pip install git+https://github.com/andreafrancia/trash-cli

Have done that? Then continue with the bug report.

Operating system:

To Reproduce Copy and paste the commands (and their output) to execute in order to reproduce the behavior:

$ touch foo $ trash-put foo $ ls foo ls: cannot access 'foo': No such file or directory $ trash-list 2020-12-13 18:36:21 /Users/andrea/trash-cli/foo

Expected behavior A clear and concise description of what you expected to happen.

Volumes detail If your issue is related to recognition of volumes, please provide the output of: trash-list --debug-volumes

andreafrancia commented 10 months ago

Cannot reproduce. Please provide the command needed to reproduce under the "To Reproduce" section of the issue template.

andreafrancia commented 9 months ago

I don't know which command you used but if you used something like this:

$ cd node_modules
$ trash-put Butaro/

then trash-put will interpret the "Butaro/" as the path pointed from the link. At this point this is like issuing a `trash-put web/themes/butaro'.

This behaviour is the same used by other linux command like ls:

$ ls -l
total 0
lrwxr-xr-x 1 andrea staff 12 Dec 15 20:54 a-link -> ../foo/a-dir
# this will show the link
$ ls -lad a-link
lrwxr-xr-x 1 andrea staff 12 Dec 15 20:54 a-link -> ../foo/a-dir
# this will show the content of the directory pointed by the link
$ ls -lad a-link/
drwxr-xr-x 2 andrea staff 64 Dec 15 20:54 a-link/

Unfortunately some shell are more prone to append and keep a "/" at the end of a path if used the TAB completion more than one time (this problem does not happen with vanilla Zsh).

The other problem is that trash-put (from day 0) will remove directories even if the -R or -r is not specified. Idea was that if you can always restore the directory there's no need to ask for a -r or -R flag. I'll reconsider this idea.

I don't really understand because you get the trash: cannot trash directory 'node_modules/Butaro/' message. Maybe you launched the command a second time.

In any case the directory will be not deleted, if you launched trash-list you should have noted that the directory will be available in the trash. If you haven't emptied the trash you can check this right now @mlncn

But if you used

$ trash-restore

From node_modules it will not show the butaro directory but running from the ../web directory it will. trash-restore only shows the trash with original location under the current directory and their subdirectories

mlncn commented 9 months ago

Thank you so so much for continuing to look at this!

I have to admit i am very much not a power user— typically i use trash and don't have to look again (OK i see trash and trash-put are identical).

trash-list only shows 14 items— one in February, and the rest between December 11 through 15, and i know i have used trashcli much more often than that.

It does not show the project where i deleted everything through the symlink and could not restore at all with trash-cli. (I was able to recover fully with a git checkout and all, fortunately i didn't lose any data.)

Also my recollection using trash-restore at the time is i also did it from a full directory up, so i got shown a number of projects i had intentionally deleted, but not the unintentional symlink-following disaster!

I'll switch to the latest release of trash-cli and try to reproduce the problem now.

Thanks again.

mlncn commented 9 months ago

OK, now on 0.23.11.10 i have the exact steps to reproduce:

mkdir trash-cli-test
cd trash-cli-test/
mkdir -p path/to/critical
touch path/to/critical/DO-NOT-DELETE
touch path/to/critical/UNABLE-TO-DELETE
chmod -w path/to/critical/UNABLE-TO-DELETE
mkdir -p weird/directory
cd weird/directory/
ln -s ../../path/to/critical/ Critical
cd ../../
trash weird/directory/Critical/

As you surmised, a big part of the problem is that slash at the end, which i admit a had to press "tab" twice to get, and i don't remember doing that before, but maybe i pasted the path in rather than using tab completion. (I use the easy, no-keyboard select + middle-click paste rather extensively.)

But anyway following those steps exactly should get you the same problematic experience of a trash "failing", but actually quite successfully having deleted exactly what i didn't mean to delete, and then failing.

The error message is more helpful than i got before also:

trash: cannot trash directory 'weird/directory/Critical/' (from volume '/')
trash:  `- failed to trash weird/directory/Critical/ in /home/mlncn/.local/share/Trash, because failed to move weird/directory/Critical/ in /home/mlncn/.local/share/Trash/files: [Errno 20] Not a directory: 'weird/directory/Critical/'
trash:  `- failed to trash weird/directory/Critical/ in /.Trash/1000, because trash dir cannot be created because its parent does not exists, trash-dir: /.Trash/1000, parent: /.Trash
trash:  `- failed to trash weird/directory/Critical/ in /.Trash-1000, because error during directory creation: [Errno 13] Permission denied: '/.Trash-1000'

No record in trash-list and the path/to/critical directory is now empty.

The most robust underlying fix would be some way for trashcli to roll back when it encounters an error, or to record what it's done so far when the error hits, but i have no idea how hard-to-impossible that would be!

mlncn commented 9 months ago

@andreafrancia my guess that making a harder-to-delete file might be part of the issue proved not to have anything to do with it, so the issue can be reproduced with this script, two lines shorter than above:

mkdir trash-cli-test
cd trash-cli-test/
mkdir -p path/to/critical
touch path/to/critical/DO-NOT-DELETE
mkdir -p weird/directory
cd weird/directory/
ln -s ../../path/to/critical/ Critical
cd ../../
trash weird/directory/Critical/

Again, note the need for the trailing slash you predicted would be part of how i caused a problem!

andreafrancia commented 6 months ago

I've fixed something around the trailing slash in a today commit.

I haven't tested yet it if this commit will change the behaviour in your example.

andreafrancia commented 6 months ago

I think the problem is solved now.

andrea@colima:/Users/andrea/trash-cli-test$ find -ls
     5734      4 drwxr-xr-x   1 andrea   dialout       128 Mar 28 18:53 .
     5735      4 drwxr-xr-x   1 andrea   dialout        96 Mar 28 18:53 ./path
     5736      4 drwxr-xr-x   1 andrea   dialout        96 Mar 28 18:53 ./path/to
     5737      4 drwxr-xr-x   1 andrea   dialout        96 Mar 28 19:06 ./path/to/critical
     6450      0 -rw-r--r--   1 andrea   dialout         0 Mar 28 19:05 ./path/to/critical/DO-NOT-DELETE
     5739      4 drwxr-xr-x   1 andrea   dialout        96 Mar 28 18:53 ./weird
     5740      4 drwxr-xr-x   1 andrea   dialout        96 Mar 28 18:53 ./weird/directory
     5741      4 lrwxr-xr-x   1 andrea   dialout        23 Mar 28 18:53 ./weird/directory/Critical -> ../../path/to/critical/
andrea@colima:/Users/andrea/trash-cli-test$ trash weird/directory/Critical/
andrea@colima:/Users/andrea/trash-cli-test$ find -ls
     5734      4 drwxr-xr-x   1 andrea   dialout       128 Mar 28 18:53 .
     5735      4 drwxr-xr-x   1 andrea   dialout        96 Mar 28 18:53 ./path
     5736      4 drwxr-xr-x   1 andrea   dialout        96 Mar 28 18:53 ./path/to
     5737      4 drwxr-xr-x   1 andrea   dialout        96 Mar 28 19:06 ./path/to/critical
     6450      0 -rw-r--r--   1 andrea   dialout         0 Mar 28 19:05 ./path/to/critical/DO-NOT-DELETE
     5739      4 drwxr-xr-x   1 andrea   dialout        96 Mar 28 18:53 ./weird
     5740      4 drwxr-xr-x   1 andrea   dialout        64 Mar 28 19:06 ./weird/directory
andreafrancia commented 6 months ago

I've simplified the script for testing the problem in a way that is idempotent and I'm going to put it here for future reference.

mkdir -p trash-cli-test
mkdir -p trash-cli-test/path/to/critical
touch trash-cli-test/path/to/critical/DO-NOT-DELETE
mkdir -p trash-cli-test/weird/directory
rm -f trash-cli-test/weird/directory/Critical
ln -s ../../path/to/critical/ trash-cli-test/weird/directory/Critical
find trash-cli-test/ -ls
trash trash-cli-test/weird/directory/Critical/
find trash-cli-test/ -ls
andreafrancia commented 6 months ago

I've done some tries with the older code and I think that the lost files can be found under ~/.local/share/Trash/files/Critical_*/DO-NOT-DELETE

So maybe the ../web/themes/butaro probably has been moved in ~/.local/share/Trash/files/Butaro*/butaro. The pattern is $TRASH_DIR/files/name_of_the_link-NNN/name-of-the-dir-linked.