`operations.DeleteFile` does not use `--backup-dir`, despite comment
#7,566 opened on Jan 14, 2024
Repository metrics
- Stars
- (57,185 stars)
- PR merge metrics
- (Avg merge 32d 20h) (23 merged PRs in 30d)
Description
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.