transmission
transmission copied to clipboard
transmission
feat: add verify option for transmission-show
@srussel To make this PR easier, I've added a new PR at https://github.com/transmission/transmission/pull/6123 that changes tr_verify_worker to no longer require a tr_torrent in order to work.
I am really unsure about tr_torrentNewOffline() because tr_torrent just was not designed to work without a tr_session, e.g. any call to tr_torrent::unique_lock() gets propagated up to this->session->unique_lock(). But with 6123 there is no need to create a tr_torrent at all.
@srussel To make this PR easier, I've added a new PR at #6123 that changes
tr_verify_workerto no longer require atr_torrentin order to work.
That would be great. I'm unfamiliar with the transmission codebase and the change I did to get verify to work without a session felt like a hack.
What is the preference for updating my change after #6123 lands? Merge main and keep adding commits? Or rebase and force push?
What is the preference for updating my change after 6123 lands? Merge main and keep adding commits? Or rebase and force push?
I generally prefer rebase+force-push but TBH whatever's easier for you is fine. Generally it's all effectively the same in the end since PRs like 6112 and 6123 each get squash-merged into main as a single commit.
I've updated the PR with the following changes
- use the mediator interface in verify worker
- use a callback in the verify function
- in show.cc, if unsorted, print the file status as files are checked instead of waiting until the end.
The remaining comments look straightforward and I will work on them sometime in the next few days.
I'm getting back up to speed on this. I've rebased and pushed the changes I already have. I think that addresses almost everything.
Github won't let me reply to the review thread on the sleep() probably due to the code changing so I have quoted and added my response below.
Thoughts Charles?
IMO our std::this_thread::sleep_for() call is a little crude, but it's a one-liner that is working fine, is portable code, needs no error-handling and no ifdefs. I'd keep it as-is unless there's a known issue with it.
My understanding of that comment is that when running a session with other torrents downloading, you want to ensure the verify does not slow down the other torrents.
tr_verify_worker runs in its own thread, so there shouldn't be any issue of verify blocking the session thread.
I've simplified my change a bit so it is just a bool flag to disable the sleep.
Just to be clear, I have no issue with the verify worker thread sleeping and have left this as the default. The issue I have is when using transmission-show, I am interactively using the command line. I run transmission-show --verify ... then sit there staring at the screen waiting for the command to finish. In this case sleeping for 100ms per second is an unacceptable delay. It makes the verify take 10% longer for no reason. The change to disable the sleep only occurs when using the tr_torrentSynchronousVerify function, which does not use the worker thread, and is only used by transmission-show. There is no change to the behavior when using verify worker in session.cc.
