`operations.DeleteFile` does not use `--backup-dir`, despite comment
#7566 aperta il 14 gen 2024
Metriche repository
- Star
- (57.185 stelle)
- Metriche merge PR
- (Merge medio 32g 20h) (23 PR mergiate in 30 g)
Descrizione
The associated forum post URL from https://forum.rclone.org
https://forum.rclone.org/t/combine-remote-decrypts-files-when-moving-to-backup-dir-expected/43986
What is the problem you are having with rclone?
The comment for operations.DeleteFile is incorrect. It states that --backup-dir is respected, but in fact, the only possible outcome is for backupDir to be nil.
Some of its callers should probably be using backupDir, and so should probably be calling operations.DeleteFileWithBackupDir instead.
cmd/bisync/bisync_test.go
491: return operations.DeleteFile(ctx, obj)
cmd/deletefile/deletefile.go
41: return operations.DeleteFile(context.Background(), fileObj)
cmd/ncdu/ncdu.go
581: err := operations.DeleteFile(ctx, obj)
636: err = operations.DeleteFile(ctx, obj)
fs/operations/dedupe.go
66: err := DeleteFile(ctx, o)
130: err := DeleteFile(ctx, o)
fs/operations/operations.go
374: err = DeleteFile(ctx, dst)
410: return newDst, DeleteFile(ctx, src)
1812: err = DeleteFile(ctx, srcObj)
fs/operations/rc.go
276: return nil, DeleteFile(ctx, o)
fs/sync/sync.go
397: s.processError(operations.DeleteFile(s.ctx, src))
443: err = operations.DeleteFile(ctx, src)
operations.DeleteFileWithBackupDir requires callers to have already looked up the backupDir with operations.BackupDir which is a relatively expensive operation, so it should be done outside the loop when it is necessary.
ncw's suggested fix, which LGTM:
- leave
operations.DeleteFilealone but fix the comment to say callDeleteFileWithBackupDirif--backup-dirsupport is required - add a note to
DeleteFileWithBackupDirnoting that you useBackupDirto find the--backup-dirand that it is relatively expensive so don't put it in a loop - audit the existing call sides for
operations.DeleteFileand try to figure out whether they should be obeying--backup-diror not.
How to use GitHub
- Please use the 👍 reaction to show that you are affected by the same issue.
- Please don't comment if you have no relevant information to add. It's just extra noise for everyone subscribed to this issue.
- Subscribe to receive notifications on status change and new comments.