mirror of
https://github.com/octoleo/syncthing.git
synced 2024-12-22 02:48:59 +00:00
lib/scanner: Prevent sync-conflict for receive-only local modifications (#9323)
### Purpose This PR changes behaviour of syncthing related to `receive-only` folders, which I believe to be a bug since I wouldn't expect the current behaviour. With the current syncthing codebase, a file of a `receive-only` folder that is only modified locally can cause the creation of a `.sync-conflict` file. ### Testing Consider this szenario: Setup two paired clients that sync a folder with a given file (e.g. `Test.txt`). One of the clients configures the folder to be `receive-only`. Now, change the contents of the file for the receive-only client **_twice_**. With the current syncthing codebase, this leads to the creation of a `.sync-conflict` file that contains the modified contents, while the regular `Test.txt` file is reset to the cluster's provided contents. This is due to a `protocol.FileInfo#ShouldConflict` check, that is succeeding on the locally modified file. This PR changes this behaviour to not reset the file and not cause the creation of a `.sync-conflict`. Instead, the second content update is treated the same as the first content update. This PR also contains a test that fails on the current codebase and succeeds with the changes introduced in this PR. ### Screenshots This is not a GUI change ### Documentation This is not a user visible change. ## Authorship Your name and email will be added automatically to the AUTHORS file based on the commit metadata. #### Thanks to all the syncthing folks for this awesome piece of software!
This commit is contained in:
parent
e829a63295
commit
8edd67a569
@ -480,6 +480,74 @@ func TestRecvOnlyRevertOwnID(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestRecvOnlyLocalChangeDoesNotCauseConflict(t *testing.T) {
|
||||
// Get us a model up and running
|
||||
|
||||
m, f, wcfgCancel := setupROFolder(t)
|
||||
defer wcfgCancel()
|
||||
ffs := f.Filesystem(nil)
|
||||
defer cleanupModel(m)
|
||||
conn := addFakeConn(m, device1, f.ID)
|
||||
|
||||
// Create some test data
|
||||
|
||||
must(t, ffs.MkdirAll(".stfolder", 0o755))
|
||||
oldData := []byte("hello\n")
|
||||
knownFiles := setupKnownFiles(t, ffs, oldData)
|
||||
|
||||
// Send an index update for the known stuff
|
||||
|
||||
must(t, m.Index(conn, "ro", knownFiles))
|
||||
f.updateLocalsFromScanning(knownFiles)
|
||||
|
||||
// Scan the folder.
|
||||
|
||||
must(t, m.ScanFolder("ro"))
|
||||
|
||||
// Everything should be in sync.
|
||||
|
||||
size := globalSize(t, m, "ro")
|
||||
if size.Files != 1 || size.Directories != 1 {
|
||||
t.Fatalf("Global: expected 1 file and 1 directory: %+v", size)
|
||||
}
|
||||
size = localSize(t, m, "ro")
|
||||
if size.Files != 1 || size.Directories != 1 {
|
||||
t.Fatalf("Local: expected 1 file and 1 directory: %+v", size)
|
||||
}
|
||||
size = needSizeLocal(t, m, "ro")
|
||||
if size.Files+size.Directories > 0 {
|
||||
t.Fatalf("Need: expected nothing: %+v", size)
|
||||
}
|
||||
size = receiveOnlyChangedSize(t, m, "ro")
|
||||
if size.Files+size.Directories > 0 {
|
||||
t.Fatalf("ROChanged: expected nothing: %+v", size)
|
||||
}
|
||||
|
||||
// Modify the file
|
||||
|
||||
writeFilePerm(t, ffs, "knownDir/knownFile", []byte("change1\n"), 0o644)
|
||||
|
||||
must(t, m.ScanFolder("ro"))
|
||||
|
||||
size = receiveOnlyChangedSize(t, m, "ro")
|
||||
if size.Files != 1 {
|
||||
t.Fatalf("Receive only: expected 1 file: %+v", size)
|
||||
}
|
||||
|
||||
// Perform another modification. This should not cause the file to be needed.
|
||||
// This is a regression test: Previously on scan the file version was changed to conflict with the global
|
||||
// version, thus being needed and creating a conflict copy on next pull.
|
||||
|
||||
writeFilePerm(t, ffs, "knownDir/knownFile", []byte("change2\n"), 0o644)
|
||||
|
||||
must(t, m.ScanFolder("ro"))
|
||||
|
||||
size = needSizeLocal(t, m, "ro")
|
||||
if size.Files != 0 {
|
||||
t.Fatalf("Need: expected nothing: %+v", size)
|
||||
}
|
||||
}
|
||||
|
||||
func setupKnownFiles(t *testing.T, ffs fs.Filesystem, data []byte) []protocol.FileInfo {
|
||||
t.Helper()
|
||||
|
||||
|
@ -426,12 +426,14 @@ func (w *walker) walkRegular(ctx context.Context, relPath string, info fs.FileIn
|
||||
l.Debugln(w, "unchanged:", curFile)
|
||||
return nil
|
||||
}
|
||||
if curFile.ShouldConflict() {
|
||||
if curFile.ShouldConflict() && !f.ShouldConflict() {
|
||||
// The old file was invalid for whatever reason and probably not
|
||||
// up to date with what was out there in the cluster. Drop all
|
||||
// others from the version vector to indicate that we haven't
|
||||
// taken their version into account, and possibly cause a
|
||||
// conflict.
|
||||
// conflict. However, only do this if the new file is not also
|
||||
// invalid. This would indicate that the new file is not part
|
||||
// of the cluster, but e.g. a local change.
|
||||
f.Version = f.Version.DropOthers(w.ShortID)
|
||||
}
|
||||
l.Debugln(w, "rescan:", curFile)
|
||||
@ -471,12 +473,14 @@ func (w *walker) walkDir(ctx context.Context, relPath string, info fs.FileInfo,
|
||||
l.Debugln(w, "unchanged:", curFile)
|
||||
return nil
|
||||
}
|
||||
if curFile.ShouldConflict() {
|
||||
if curFile.ShouldConflict() && !f.ShouldConflict() {
|
||||
// The old file was invalid for whatever reason and probably not
|
||||
// up to date with what was out there in the cluster. Drop all
|
||||
// others from the version vector to indicate that we haven't
|
||||
// taken their version into account, and possibly cause a
|
||||
// conflict.
|
||||
// conflict. However, only do this if the new file is not also
|
||||
// invalid. This would indicate that the new file is not part
|
||||
// of the cluster, but e.g. a local change.
|
||||
f.Version = f.Version.DropOthers(w.ShortID)
|
||||
}
|
||||
l.Debugln(w, "rescan:", curFile)
|
||||
@ -524,12 +528,14 @@ func (w *walker) walkSymlink(ctx context.Context, relPath string, info fs.FileIn
|
||||
l.Debugln(w, "unchanged:", curFile, info.ModTime().Unix(), info.Mode()&fs.ModePerm)
|
||||
return nil
|
||||
}
|
||||
if curFile.ShouldConflict() {
|
||||
if curFile.ShouldConflict() && !f.ShouldConflict() {
|
||||
// The old file was invalid for whatever reason and probably not
|
||||
// up to date with what was out there in the cluster. Drop all
|
||||
// others from the version vector to indicate that we haven't
|
||||
// taken their version into account, and possibly cause a
|
||||
// conflict.
|
||||
// conflict. However, only do this if the new file is not also
|
||||
// invalid. This would indicate that the new file is not part
|
||||
// of the cluster, but e.g. a local change.
|
||||
f.Version = f.Version.DropOthers(w.ShortID)
|
||||
}
|
||||
l.Debugln(w, "rescan:", curFile)
|
||||
|
Loading…
Reference in New Issue
Block a user