pyrt icon indicating copy to clipboard operation
pyrt copied to clipboard

"Delete" button does not delete individual files, but folders

Open vista- opened this issue 8 years ago • 10 comments

I have this in my .rtorrent.rc to make sure all files get downloaded into the same 'downloads' directory:

d.set_directory=/home/vista/media/downloads

When I deleted a torrent that was just a .zip, without a parent folder, my entire 'downloads' directory was deleted.

vista- avatar Oct 13 '16 13:10 vista-

Damn, I'm really sorry about that!

This should have been prevented by pull request #41 (commit a2be86b9b1e04b233fe2477647ed5a747743a9db) about 3 years ago...

I imagine this might be something to do with trailing slashes, but I will investigate...

mountainpenguin avatar Oct 13 '16 13:10 mountainpenguin

This really shouldn't be happening, I'm thinking perhaps something strange about relative/absolute paths?

If you're willing, do you think you could download and run this script and paste the output?

Download it to the pyrt directory and run it with python2 test_script.py

Example output:

odin :: ~/pyrt » python2 test_script.py
rootDir: /home/mp-torrent/torrents/downloading
base_path: /home/mp-torrent/torrents/downloading
abs_path: /home/mp-torrent/torrents/downloading/filename.pdf

The important part is that base_path and rootDir must match to avoid deleting the downloads directory, and I'm not sure how they can't match...

mountainpenguin avatar Oct 14 '16 09:10 mountainpenguin

Incidentally, I set my downloads directory in .rtorrent.rc with this line:

directory = /home/mp-torrent/torrents/downloading

Perhaps the rtorrent API uses a default value if you use d.set_directory instead of directory?...

mountainpenguin avatar Oct 14 '16 09:10 mountainpenguin

Hi, sorry for the late reply.

The script gives me this output:

rootDir: /home/vista/dev/pyrt
base_path: /home/vista/media/downloads
abs_path: /home/vista/media/downloads/debian-8.5.0-amd64-netinst.iso

vista- avatar Oct 16 '16 10:10 vista-

Hmm OK, I think this must be to do with the d.set_directory command.

I updated the gist with a couple of extra lines, could you try running it again please? https://gist.github.com/mountainpenguin/8ce4673e295a0fb47b59566533dd6dec

mountainpenguin avatar Oct 16 '16 11:10 mountainpenguin

Hmm, actually I don't think that'll work, it's essentially the same command as getRootDir()

I think I might have to write in a hacky method that ensures that aren't extra files in the base_path I should probably write in a confirmation dialog for deletion too

mountainpenguin avatar Oct 16 '16 11:10 mountainpenguin

Here is the output

default: ./
directory: /home/vista/media/downloads
rootDir: /home/vista/dev/pyrt
base_path: /home/vista/media/downloads
abs_path: /home/vista/media/downloads/debian-8.5.0-amd64-netinst.iso

And here is the output when I set directory = /home/vista/media/downloads

default: /home/vista/media/downloads
directory: /home/vista/media/downloads
rootDir: /home/vista/media/downloads
base_path: /home/vista/media/downloads
abs_path: /home/vista/media/downloads/debian-8.5.0-amd64-netinst.iso

Your previous suggestion seems to fix the problem!

If you could add a confirmation dialog with the filename / directory about to be deleted, that could prevent similar accidents from happening; or at least a check whether the default directory is set.

Thanks again for the help!

vista- avatar Oct 16 '16 11:10 vista-

Yeah it looks like I'll have to write this the hard way! I'll write in some extra paranoid checks and a confirmation dialog.

Is the difference with your configuration that partially downloaded torrents are kept somewhere else before they are moved into the downloads directory when completed? Because I think my configuration won't provide that feature.

It's been a long time since I've messed around with my javascript so it may take me some time to remember how I do things on this project (plus my javascript is probably pretty rusty).

Sorry for all the trouble, and I hope you didn't lose too much data.

mountainpenguin avatar Oct 16 '16 11:10 mountainpenguin

I do not use the "move on complete" feature, so it's not a concern to me. As far as I saw, this "d.set_directory" config value is indeed used as part of that.

Nothing irreplaceable was lost, don't worry about it. :)

vista- avatar Oct 16 '16 11:10 vista-

It should now choose the correct file if the default root directory is unusual, and provide a general confirmation before allowing any deletion.

I'll leave this open until I merge testing with master

mountainpenguin avatar Oct 17 '16 14:10 mountainpenguin