Conversation
nielash
force-pushed
the
fix-bisync-listing-modtime
branch
from
September 30, 2026 07:46
1be15fd to
8ef3bd2
Compare
nielash
marked this pull request as ready for review
September 30, 2026 08:01
Before this change, when bisync updated a listing entry, it kept the old modtime if the new one was less than a second earlier. This dated back to when bisync truncated modtimes to the destination's precision before recording them (`ConvertPrecision`), so that a truncated time would not overwrite the more precise original. `ConvertPrecision` was removed in 956c296, and since then, bisync has kept the original precision and relied on the modify window when comparing. But the rule in `ls.put` was left behind, and it then fired only when a file really had an earlier modtime, recording a time the file did not have. This could cause: - a `--resync` to abort with "path1 and path2 are out of sync" when Path1's version was slightly older than Path2's - later runs to report changes that didn't happen, for a file changed to a version less than a second older, or copied from a remote with lower modtime precision within the same second as the old version (for example, from SFTP to local) After this change, bisync records the modtime it is given, like any other update. Comparisons already account for differences in precision through the modify window.
nielash
force-pushed
the
fix-bisync-listing-modtime
branch
from
September 30, 2026 18:12
8ef3bd2 to
72c39fa
Compare
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
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 does this change do?
Before this change,
ls.putkept a listing entry's old modtime whenever the new one was less than a second earlier. That rule was a companion toConvertPrecision, which truncated modtimes before they were recorded, so a truncated time wouldn't overwrite the precise original. 956c296 removedConvertPrecision(#8025) but left the rule behind, so now it only fires when a file really does have an earlier modtime -- and records a time the file doesn't have.That shows up two ways:
--resyncaborts with "path1 and path2 are out of sync" when Path1's version is slightly older than Path2's (default--resync-mode path1), and running--resyncagain fails the same way.After this change, bisync records the modtime it's given. Comparisons already account for differences in precision through the modify window. It's just a 6-line deletion + unit tests.
I also tried the opposite approach -- keeping the more precise of two times within the modify window -- but testing showed that's exactly what causes the phantom change on the higher-precision side. The listing there needs to match what the file actually has.
This also pairs with #10001: the identical files that #10001 records go through
ls.puttoo, so without this fix a file with a slightly older modtime can keep coming back as "changed on both paths" on every run.Linked issue
No issue -- I found this while adding test coverage for bisync. Related: #8025, #10001
Checklist
test_allpasses for this backend and if submitting a new backend can provide a test account for the integration tester - see CONTRIBUTING.md.