search

qbittorrent / qBittorrent

qBittorrent BitTorrent client
https://www.qbittorrent.org
Other
28.34k stars 3.98k forks source link

qBittorrent 4.5 fails to overwrite existing files #18136

Closed baccccccc closed 1 year ago

baccccccc commented 1 year ago

qBittorrent & operating system versions

qBittorrent: 4.5.0 (64-bit) Operating system: Windows 11, version 22H2 (build 22623.1020) Qt: 6.4.0 libtorrent-rasterbar: 2.0.8.0

What is the problem?

The problem seems to be new to version 4.5. It did not happen before, not even in version 4.5 beta.

The issue happens if the file already exists in the destination folder, and downloading to temporary directory (“use another path for incomplete torrent”) is enabled. (Note: in case temporary directory is disabled, qBittorrent has no issue overwriting files. The problem does not seem to exist in this case.)

At first, downloads completes as expected to the temporary folder. But once it's time to move the file to the final location, it fails. The log includes the following message.

Failed to move torrent. Torrent: "<bla>.mp4". Source: "C:\Drive\Current\qB". Destination: "C:\Drive\<bla>". Reason: "<bla>.mp4 storage move failed. file_stat (C:\Drive\Current\qB\<bla>.mp4): The operation completed successfully"

Note that the final part of the message does not make much sense. (In other scenarios, e.g. if there's an actual lack of permissions to the file, the same error message is usually much more actionable. E.g., it says “access is denied” or something along those lines.)

So, in this case, it's not a permissions issue. I validated that qBittorrent has all the rights to write to the destination folder and the file itself. (I can do that, while running as the same user as the application.) I have also validate that the permissions to the existing file and the new file (downloaded to the temporary folder) are exactly the same. Moreover, the existing file does not have any attributes (e.g., read-only) which could also interfere with the overwrite attempt.

Steps to reproduce

  1. Add a new torrent to the app.
  2. Assign torrent to a category which has temporary folder enabled.
  3. Create a file with the same name, as being downloaded, in the target folder (not the temporary folder.)
  4. Download torrent.
  5. Observe the file getting stuck in the temporary folder even after completion.

Additional context

This happens repeatedly, across multiple files in multiple folders. It's not a one-off issue.

It might be actually a libtorrent 2.0 issue. I have not validated it on a libtorrent 1.x fork.

Log(s) & preferences file(s)

No response

thalieht commented 1 year ago

Of course someone would complain about this 🙃! Meanwhile people have been wailing for years that they don't want files to be overwritten, ever.

baccccccc commented 1 year ago

so, are you saying that's by design? I might agree that it can be indeed the desired behavior for some people. However, in this case:

also, this new behavior needs to be consistent with the scenarios where temporary folder is not enabled. That is, if we suddenly decide that existing files should never be overwritten, that should not happen regardless of whether the temporary folder is used or not. (Today, it looks like the files get overwritten when temporary folder is disabled, but fail to overwrite when temporary folder is enabled.)

finally, if we admit that duplicate filenames is a problem, I would prefer a slightly different solution. (And that probably should be a different thread with a feature request.) Oftentimes, there are different torrents with overlapping names and paths. I would love qBittorrent to detect such torrents once they're added, and flag them. (Like a warning status or something.) So that I can take action (e.g., download to a different folder or rename files manually) before starting the torrent. But it still needs to provide an option to force download to the same folder (and overwrite files) because that's sometimes an actual desired behavior to me.

...and, taking one step forward, we might want to implement the following. If an existing file is detected, not only provide user with a choice to blindly overwrite or not overwrite. (Which is good enough, but not perfect in my opinion.) But also provide a third option, which: run a consistency check on the existing file. And then, overwrite automatically—but only in case the file is indeed the same as the one to be downloaded. That would be 100% ideal solution to me.

tsweet64 commented 1 year ago

To explain what appears to be going on with the "Operation completed successfully" error:

the stat() syscall is used to query information about a specified file. Before moving, qBittorrent appears to run stat() on the destination file. This is of course supposed to fail, meaning that no such file exists and a file can be safely moved to that name without overwriting anything. If stat() completes successfully, this means that a file exists and we shouldn't overwrite it. Operating systems return errno(3) codes when a syscall is executed, and this is how standard, universal error messages like "Operation not supported", "No space on device", or in this case "Operation completed successfully" are obtained. qBittorrent appears to just print the errno string in the log message for any sort of file operation error. Usually, that makes sense, so the user knows exactly whether something was a permission error, out of space, etc, but I can see how it's confusing here (checking whether the file exists completed successfully).

I've not developed for this so I don't know the exact ideal solution, but perhaps adding (additionally to the system error messages) a specific note to the log entry describing the issue as a refusal to overwrite existing files would be useful.

sserdda-liamE commented 1 year ago

I'm +1 for "let it fail successfully and let the user manually decide what to do with the files." It's just safer. This doesn't even need an option to change the behavior. The user should learn from his mistake an not download things with the same name to the same directory. Having one universal behavior saves headaches: "Waaah it overwrote my files!" "Well, did you have it set to overwrite your files?" "Where?" "Settings." "Oh, I guess I did.'

Just never overwrite and let them either fix their behavior or live with switching between temp and move directories.

baccccccc commented 1 year ago

The user should learn from his mistake an not download things with the same name to the same directory.

Look, it's not necessarily a mistake. Stop judging other people :) There are multiple valid scenarios when overwriting is the desired behavior.

Some people download a movie to watch it only once. Once done, they don't care if the file exists or not. Of course, you can say "then they should delete the files!" But my point is—that was not needed before, and now we're making life for those people harder.

Another example is what I'm doing. I know I downloaded a bunch of torrents long ago, but they are not added to my qBittorrent app right now. (Long story short, I downloaded them with a different client before.) Now I found a bunch of torrents, added to qBittorrent, and I just want them to be downloaded. (No, just having the files already is not good enough. I want to be able to run consistency checks every once a while, and to seed them. I.e., I need the torrent to be present in qBittorrent, and to be in "completed" state.)

Of course, I can just manually recheck every torrent. But that's too much work when you have thousands and thousands of torrents. (I have almost 37K at this moment.) So, if it's a large torrent—sure, I will make an effort to manually point to existing files and recheck instead of redownloading. But if it's a few MB? Meh, I would prefer to just download and overwrite whatever is there. (Because I know they're the same files.)

Of course, not everyone is in the same boat. But all I'm saying—the app behavior has changed. It might be good for some people but bad for others. So, at very least, it needs to be documented. Or better yet, it needs to be configurable. There are plenty of options for how to make it better. (See my previous post above for some ideas.)

glassez commented 1 year ago

so, are you saying that's by design?

Yes, the moving behavior was changed by design (at least if we are talking about a moving initiated behind the scenes, for example. about moving from a temporary folder). After all, accidentally overwriting an unrelated file looks like a more serious problem than if the torrent could not be moved to a final destination. But the above does not apply to adding torrents. Its behavior shouldn't have been changed (at least I just did a couple of quick tests to make sure), and it assumes the use of existing files (technically, files with matching names). So I don't understand why you're complaining about it.

baccccccc commented 1 year ago

So I don't understand why you're complaining about it.

  1. Add new torrent.
  2. Start downloading
  3. It downloads to temporary directory
  4. What happens next will surprise you! 4a. Expected: torrent moves to the permanent directory 4b. Actual: it's not getting moved.
  5. Moreover, a weird error message is logged

My problem is not with adding torrent (step 1.) It was mentioned for context.

The problem is step 4, more specifically, step 4b.

Additionally, step 5 is also confusing and makes troubleshooting unnecessarily difficult.

Does this make sense? Kindly see my previous post in this thread for the explanation why this is a concern for me.

glassez commented 1 year ago

My problem is not with adding torrent (step 1.) It was mentioned for context.

The problem is step 4, more specifically, step 4b.

After adding torrent it is supposed to use the files that are existing in final destination (and it does according to my test). So if there are all the files of torrent it should end up with checking only. If there are only some files of torrent then they are supposed to be moved in "incomplete" folder, torrent will download the remaining files and move all the files back to final destination. So why do you still have any files with matched names in final destination on step 4?

baccccccc commented 1 year ago

After adding torrent it is supposed to use the files that are existing in final destination (and it does according to my test).

Hmm. It never did this for me. Neither before this change (that is, prior to version 4.5) nor after the change (on 4.5.)

If I want to use existing files, I have to do the following.

  1. Add torrent.
  2. Open torrent properties.
  3. Uncheck "Automatic management"
  4. Uncheck "Use temporary folder for incomplete torrents."
  5. Manually initiate checking.

Only in this case it will recognize existing files and use them.

If I don't do that, it will only care for files that exist in temporary folder, but never check for files in the target folder, until the download concludes.

thalieht commented 1 year ago

After adding torrent it is supposed to use the files that are existing in final destination (and it does according to my test).

Hmm. It never did this for me.

According to your OP, you first add the torrent and after that you put a file to the final destination. qBt checks the final destination for existing files only during torrent addition (and on finish now, to prevent moving files in case of conflict),

baccccccc commented 1 year ago

I guess this is now being closed by #18516.

(that said, I still believe that the best solution is to provide an option to users.)