operations.DeleteFile does not use --backup-dir, despite comment

[nielash]

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.

rclone/fs/operations/operations.go

Lines 489 to 495 in 519fe98

	// DeleteFile deletes a single file respecting --dry-run and accumulating stats and errors.
	//
	// If useBackupDir is set and --backup-dir is in effect then it moves
	// the file to there instead of deleting
	func DeleteFile(ctx context.Context, dst fs.Object) (err error) {
	return DeleteFileWithBackupDir(ctx, dst, 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:

1. leave operations.DeleteFile alone but fix the comment to say call DeleteFileWithBackupDir if --backup-dir support is required
2. add a note to DeleteFileWithBackupDir noting that you use BackupDir to find the --backup-dir and that it is relatively expensive so don't put it in a loop
3. audit the existing call sides for operations.DeleteFile and try to figure out whether they should be obeying --backup-dir or 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.