bisync: fix koofr and chunker integration tests #8796
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Before this change, koofr failed certain bisync tests because it can't set mod time without deleting and re-uploading. This caused the "nothing to transfer" log to not get printed where expected (as it is only printed when there are 0 transfers, but koofr requires extra transfers to set modtime.) This change fixes the issue by ignoring the absence of the "nothing to transfer" log line on backends that return `fs.ErrorCantSetModTimeWithoutDelete` for `obj.SetModTime`.
Before this change, TestChunkerS3: tests were failing because our use of obj.Remove (for "modtime_write_test") created an unexpected extra transfer. This is because chunker calls operations.Move for removes, which (per its function comment) is supposed to be only accounted as a check. But because S3 can Copy but not Move, the move falls back to copy and ends up getting counted as a transfer anyway. https://github.com/rclone/rclone/blob/99e8a63df2528e4fd470821095cbeb32e8a827a3/fs/operations/operations.go#L506 https://github.com/rclone/rclone/blob/99e8a63df2528e4fd470821095cbeb32e8a827a3/fs/operations/copy.go#L381 This is probably a bug that should get a more proper fix in operations. But in the meantime, we can get around it by doing our "modtime_write_test" with its own unique stats group.
ncw
approved these changes
Sep 4, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is the purpose of this change?
Two fixes for bisync integration tests on
koofrandchunker.@ncw For the
chunkerissue (which I think I tracked down), it should be noted that there is an underlying bug inoperationsthat this PR does not fix, but just side-steps.The bug is that
operations.Move(per its function comment) is supposed to be only accounted as a check. But on backends likeS3which canCopybut notMove, the move falls back to server-side copy and ends up getting counted as a transfer anyway, even whenisTransfer == false.rclone/fs/operations/operations.go
Lines 413 to 416 in 99e8a63
rclone/fs/operations/operations.go
Line 506 in 99e8a63
rclone/fs/operations/copy.go
Line 381 in 99e8a63
This could theoretically be fixed by making a similar
Copy/CopyTransferscheme and/or passing anisTransferparam toCopy. But asCopyhas lots of callers, that's a potentially invasive change that I deemed out-of-scope for this PR 😅See the 68a9019 commit message for further explanation about why this affects
chunkerdifferently from other backends.Was the change discussed in an issue or in the forum before?
Checklist