Transmission "cache cleaning" is wrong

[mikedld]

Blocklists directory could contain a cached blocklist file corresponding to one downloaded from single URL provided in settings. The file Transmission uses to store cached blocklist in this case is "[config dir]/blocklists/blocklist.bin". However, this directory could also contain additional blocklist files put there manually by user; those files are either already in the format used by Transmission for blocklist cache (in which case they should have ".bin" extension) or in the format that Transmission is able to parse (in which case they should not have ".bin" extension and Transmission will parse and create/update cache files named "[config dir]/blocklists/[original file base name].bin" during initialization, or on SIGHUP if transmission-daemon is used). The issue is, both ".bin" and non-".bin" files are being removed, while only the former are considered "cache" (and even then, some of those could be user-provided).

Resume directory, if managed solely by Transmission without user's intervention, could only contain files with ".resume" extension. Those files are used to track torrent download/seed progress/statistics, current files location (complete and incomplete files could be stored in different directories on per-torrent basis), file download priorities and "do not download" flags, bandwidth/seeding limits overrides, paused state, real file names on disk (in case any files/directories inside torrent were renamed by user). As you could see, none of this information could be considered "cache". Each ".resume" file though should have a corresponding ".torrent" file in "[config dir]/torrents" directory (with matching base name); and only those ".resume" files which do not have corresponding ".torrent" files could be safely removed.

I see cleaners are simple XML files, while they used to be implemented in Python. I would've made a pull request right away if they still were in Python, but I suppose this XML description format doesn't provide what's necessary for a proper cleaning implementation (correct me if I'm wrong). Thus I propose you to either fix Transmission cleaning (if that's possible) or remove this cleaner.

[cfpp2p]

blocklist directory could contain non .bin manually added user files and there would be no method of determining specifically which the user might not want for future editing and production of the resulting liked named .bin. A file named blocklist.bin is produced from a valid blocklist url but even so we still don't know if the user renamed a different .bin to this for whatever reason. Summed up I don't see how to safely remove anything from the blocklist directory other than parsed files which produced no valid results and .bin but this is not something XML could possibly determine.

There was an issue with blocklist.tmp (actually a file descriptor) being leaked but it's been fixed.
https://trac.transmissionbt.com/changeset/14230
https://trac.transmissionbt.com/ticket/5583

resume files could be safely cleaned if no exactly like named torrent file was found.

As mikedld proposes, this needs to be fixed or the cleaner removed.

[tiemay]

As you've both pointed out, since fixing this might be tricky from a technical standpoint, how about we relabel the options so users can make their own informed decisions? Would the above patch be ok?

[cfpp2p]

I think that would be OK if we changed:
"Delete the blocklists" to something like this "Delete ALL the blocklists (blocklists will need update to work)"

and

"Remove all torrents" to "Remove all torrents (associated data will remain)"

The user needs to be informed of the final results detail.

[tiemay]

What's the associated data that will remain? Keep in mind that we want the wording to be succinct, so there will be less work for translators.

[cfpp2p]

The data that remains is the torrent's downloaded and/or seed data. Transmission clients, for instance the web client, use wording like this when removing torrent(s):

"Remove from list"
"Trash data and remove from list"

I think that it should be clear that the associated torrent data is not being removed as well.

[tiemay]

And the seed data would be ~/.config/transmission/stats.json?

[cfpp2p]

No, what I meant was the actual files downloaded or the actual files supplied by the user to seed/upload. The files associated with the .torrent and .resume files don't always have to be downloaded but can also be user supplied and therefore seeded/uploaded.

The stats file is overall stats for the application as a whole and I don't know why that should be deleted.

[tiemay]

Ok, how about ^that^
With <warning> the user will get a popup letting them know that the blocklists option will reset their blocklist settings, and I tweaked some of the wording.

[cfpp2p]

Looks fine to me :)

[tiemay]

oops, I think I might have done something wrong when I was testing the cleaner. For some reason after deleting the blocklists, my blocklist settings where also reset to defaults (enable/disable state for using blocklists, url for the list, enable/disable state for automatic updates), which doesn't make sense because those seem to be stored in ~/.config/transmission/settings.json. Now when I delete the blocklists, the settings remain in tact, is this the case for you as well? If so I'll change the warning to "Blocklists will need update to work." like you suggested.

[cfpp2p]

"Blocklists will need update to work.

That works for me.

[tiemay]

Fixed the warning and sent a pull request

[tiemay]

Changes where merged, thank you @cfpp2p and @mikedld for reporting

[az0]

Yes, thank you. :)