search

mountainpenguin / pyrt

python rtorrent webUI
GNU General Public License v3.0
34 stars 11 forks source link

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

Open vista- opened 7 years ago

vista- commented 7 years ago

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.

mountainpenguin commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

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?...

vista- commented 7 years ago

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
mountainpenguin commented 7 years ago

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 commented 7 years ago

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

vista- commented 7 years ago

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!

mountainpenguin commented 7 years ago

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.

vista- commented 7 years ago

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. :)

mountainpenguin commented 7 years ago

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