Skip to content

Commit 72c39fa

Browse files
nielashnielash
committed
bisync: fix listings ignoring a modtime less than 1s older
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.
1 parent 0b8e9c4 commit 72c39fa

3 files changed

Lines changed: 117 additions & 6 deletions

File tree

‎cmd/bisync/listing.go‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -129,12 +129,6 @@ func (ls *fileList) put(file string, size int64, modtime time.Time, hash, id str
129129
fi := ls.get(file)
130130
if fi != nil {
131131
fi.size = size
132-
// if already have higher precision of same time, avoid overwriting it
133-
if fi.time != modtime {
134-
if modtime.Before(fi.time) && fi.time.Sub(modtime) < time.Second {
135-
modtime = fi.time
136-
}
137-
}
138132
fi.time = modtime
139133
fi.hash = hash
140134
fi.id = id

‎cmd/bisync/listing_internal_test.go‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,114 @@
1+
package bisync
2+
3+
import (
4+
"context"
5+
"os"
6+
"path/filepath"
7+
"strings"
8+
"testing"
9+
"time"
10+
11+
"github.com/rclone/rclone/fs"
12+
"github.com/rclone/rclone/fstest"
13+
"github.com/stretchr/testify/assert"
14+
"github.com/stretchr/testify/require"
15+
)
16+
17+
func TestFileListPut(t *testing.T) {
18+
old := time.Date(2026, 9, 29, 12, 0, 1, 555555555, time.UTC)
19+
for _, test := range []struct {
20+
name string
21+
modtime time.Time
22+
}{
23+
{"slightly older", old.Add(-124 * time.Microsecond)},
24+
{"slightly newer", old.Add(124 * time.Microsecond)},
25+
{"same second, truncated", old.Truncate(time.Second)},
26+
{"same second, rounded up", old.Round(time.Second)},
27+
} {
28+
t.Run(test.name, func(t *testing.T) {
29+
ls := newFileList()
30+
ls.put("file", 1, old, "", "-", "-")
31+
ls.put("file", 2, test.modtime, "", "-", "-")
32+
assert.True(t, test.modtime.Equal(ls.getTime("file")), "got %v, want %v", ls.getTime("file"), test.modtime)
33+
assert.EqualValues(t, 2, ls.getSize("file"))
34+
})
35+
}
36+
}
37+
38+
// Test a file whose new version is less than a second older than the old one
39+
func TestBisyncCloseModtimes(t *testing.T) {
40+
if *fstest.RemoteName != "" {
41+
t.Skip("Skipping test on non local remote")
42+
}
43+
ctx := context.Background()
44+
unchanged := time.Date(2026, 9, 1, 0, 0, 0, 0, time.UTC)
45+
newRun := func(t *testing.T) (path1, path2 string, run func(resync bool) error) {
46+
// t.TempDir includes the test name, which makes the listing names too long
47+
dir, err := os.MkdirTemp("", "bisync")
48+
require.NoError(t, err)
49+
t.Cleanup(func() { _ = os.RemoveAll(dir) })
50+
path1, path2 = filepath.Join(dir, "path1"), filepath.Join(dir, "path2")
51+
workdir := filepath.Join(dir, "workdir")
52+
for _, path := range []string{path1, path2} {
53+
require.NoError(t, os.Mkdir(path, 0o755))
54+
require.NoError(t, os.WriteFile(filepath.Join(path, "file1.txt"), []byte("unchanged"), 0o644))
55+
require.NoError(t, os.Chtimes(filepath.Join(path, "file1.txt"), unchanged, unchanged))
56+
}
57+
run = func(resync bool) error {
58+
f1, err := fs.NewFs(ctx, path1)
59+
require.NoError(t, err)
60+
f2, err := fs.NewFs(ctx, path2)
61+
require.NoError(t, err)
62+
return Bisync(ctx, f1, f2, &Options{Workdir: workdir, Resync: resync, CheckSync: CheckSyncTrue, MaxDelete: DefaultMaxDelete})
63+
}
64+
return path1, path2, run
65+
}
66+
writeFile := func(t *testing.T, path, content string, modtime time.Time) {
67+
file := filepath.Join(path, "file0.txt")
68+
require.NoError(t, os.WriteFile(file, []byte(content), 0o644))
69+
require.NoError(t, os.Chtimes(file, modtime, modtime))
70+
}
71+
// listingTime returns the modtime recorded for file0.txt in the listing for path (1 or 2)
72+
listingTime := func(t *testing.T, path1 string, path int) time.Time {
73+
listings, err := filepath.Glob(filepath.Join(filepath.Dir(path1), "workdir", "*.path"+string(rune('0'+path))+".lst"))
74+
require.NoError(t, err)
75+
require.Len(t, listings, 1)
76+
data, err := os.ReadFile(listings[0])
77+
require.NoError(t, err)
78+
for line := range strings.SplitSeq(string(data), "\n") {
79+
if strings.HasSuffix(line, `"file0.txt"`) {
80+
fields := strings.Fields(line)
81+
modtime, err := time.Parse(timeFormat, fields[4])
82+
require.NoError(t, err)
83+
return modtime
84+
}
85+
}
86+
t.Fatal("file0.txt not in listing")
87+
return time.Time{}
88+
}
89+
// fileTime returns file0.txt's modtime as the filesystem stored it (Windows keeps 100ns)
90+
fileTime := func(t *testing.T, path string) time.Time {
91+
info, err := os.Stat(filepath.Join(path, "file0.txt"))
92+
require.NoError(t, err)
93+
return info.ModTime()
94+
}
95+
modtime := time.Date(2026, 9, 29, 12, 0, 1, 555555555, time.UTC)
96+
97+
t.Run("resync", func(t *testing.T) {
98+
path1, path2, run := newRun(t)
99+
writeFile(t, path1, "path1", modtime)
100+
writeFile(t, path2, "path2, newer", modtime.Add(124*time.Microsecond))
101+
require.NoError(t, run(true))
102+
assert.True(t, fileTime(t, path2).Equal(listingTime(t, path1, 2)), "Path2 listing has %v, file has %v", listingTime(t, path1, 2), fileTime(t, path2))
103+
})
104+
105+
t.Run("older edit", func(t *testing.T) {
106+
path1, path2, run := newRun(t)
107+
writeFile(t, path1, "original", modtime)
108+
require.NoError(t, run(true))
109+
writeFile(t, path1, "edited", modtime.Add(-200*time.Microsecond))
110+
require.NoError(t, run(false))
111+
assert.True(t, fileTime(t, path1).Equal(listingTime(t, path1, 1)), "Path1 listing has %v, file has %v", listingTime(t, path1, 1), fileTime(t, path1))
112+
assert.True(t, fileTime(t, path2).Equal(listingTime(t, path1, 2)), "Path2 listing has %v, file has %v", listingTime(t, path1, 2), fileTime(t, path2))
113+
})
114+
}

‎docs/content/bisync.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1937,6 +1937,9 @@ about *Unison* and synchronization in general.
19371937

19381938
- Added `--max-delete-renames-aware` to exclude guaranteed tracked renames from
19391939
the `--max-delete` safety check when used with `--track-renames`.
1940+
- Fixed an issue causing a `--resync` to fail with "path1 and path2 are out of
1941+
sync", or later runs to report changes that didn't happen, when a file's new
1942+
modtime was less than a second earlier than the one in the listing.
19401943

19411944
### `v1.74.2`
19421945

0 commit comments

Comments
 (0)