From c3b5eba205071f774886e82a961e72a15a2fb1ee Mon Sep 17 00:00:00 2001 From: Simon Frei Date: Mon, 25 May 2020 08:46:24 +0200 Subject: [PATCH] lib/model: Fix checking children when trying to delete a dir (fixes #6646) (#6647) --- lib/model/folder_sendrecv.go | 85 ++++++++++++++++++++----------- lib/model/folder_sendrecv_test.go | 17 +++++-- 2 files changed, 68 insertions(+), 34 deletions(-) diff --git a/lib/model/folder_sendrecv.go b/lib/model/folder_sendrecv.go index add5d1dd6..a657a71bc 100644 --- a/lib/model/folder_sendrecv.go +++ b/lib/model/folder_sendrecv.go @@ -61,17 +61,18 @@ type copyBlocksState struct { const retainBits = fs.ModeSetgid | fs.ModeSetuid | fs.ModeSticky var ( - activity = newDeviceActivity() - errNoDevice = errors.New("peers who had this file went away, or the file has changed while syncing. will retry later") - errDirPrefix = "directory has been deleted on a remote device but " - errDirHasToBeScanned = errors.New(errDirPrefix + "contains unexpected files, scheduling scan") - errDirHasIgnored = errors.New(errDirPrefix + "contains ignored files (see ignore documentation for (?d) prefix)") - errDirNotEmpty = errors.New(errDirPrefix + "is not empty; the contents are probably ignored on that remote device, but not locally") - errNotAvailable = errors.New("no connected device has the required version of this file") - errModified = errors.New("file modified but not rescanned; will try again later") - errUnexpectedDirOnFileDel = errors.New("encountered directory when trying to remove file/symlink") - errIncompatibleSymlink = errors.New("incompatible symlink entry; rescan with newer Syncthing on source") - contextRemovingOldItem = "removing item to be replaced" + activity = newDeviceActivity() + errNoDevice = errors.New("peers who had this file went away, or the file has changed while syncing. will retry later") + errDirPrefix = "directory has been deleted on a remote device but " + errDirHasToBeScanned = errors.New(errDirPrefix + "contains changed files, scheduling scan") + errDirHasIgnored = errors.New(errDirPrefix + "contains ignored files (see ignore documentation for (?d) prefix)") + errDirHasReceiveOnlyChanged = errors.New(errDirPrefix + "contains locally changed files") + errDirNotEmpty = errors.New(errDirPrefix + "is not empty; the contents are probably ignored on that remote device, but not locally") + errNotAvailable = errors.New("no connected device has the required version of this file") + errModified = errors.New("file modified but not rescanned; will try again later") + errUnexpectedDirOnFileDel = errors.New("encountered directory when trying to remove file/symlink") + errIncompatibleSymlink = errors.New("incompatible symlink entry; rescan with newer Syncthing on source") + contextRemovingOldItem = "removing item to be replaced" ) const ( @@ -1819,31 +1820,52 @@ func (f *sendReceiveFolder) deleteDirOnDisk(dir string, snap *db.Snapshot, scanC toBeDeleted := make([]string, 0, len(files)) - hasIgnored := false - hasKnown := false - hasToBeScanned := false + var hasIgnored, hasKnown, hasToBeScanned, hasReceiveOnlyChanged bool for _, dirFile := range files { fullDirFile := filepath.Join(dir, dirFile) - if fs.IsTemporary(dirFile) || f.ignores.Match(fullDirFile).IsDeletable() { + switch { + case fs.IsTemporary(dirFile) || f.ignores.Match(fullDirFile).IsDeletable(): toBeDeleted = append(toBeDeleted, fullDirFile) - } else if f.ignores != nil && f.ignores.Match(fullDirFile).IsIgnored() { + continue + case f.ignores != nil && f.ignores.Match(fullDirFile).IsIgnored(): hasIgnored = true - } else if cf, ok := snap.Get(protocol.LocalDeviceID, fullDirFile); !ok || cf.IsDeleted() || cf.IsInvalid() { - // Something appeared in the dir that we either are not aware of - // at all, that we think should be deleted or that is invalid, - // but not currently ignored -> schedule scan. The scanChan - // might be nil, in which case we trust the scanning to be - // handled later as a result of our error return. - if scanChan != nil { - scanChan <- fullDirFile - } - hasToBeScanned = true - } else { - // Dir contains file that is valid according to db and - // not ignored -> something weird is going on - hasKnown = true + continue } + cf, ok := snap.Get(protocol.LocalDeviceID, fullDirFile) + switch { + case !ok || cf.IsDeleted(): + // Something appeared in the dir that we either are not + // aware of at all or that we think should be deleted + // -> schedule scan. + scanChan <- fullDirFile + hasToBeScanned = true + continue + case ok && f.Type == config.FolderTypeReceiveOnly && cf.IsReceiveOnlyChanged(): + hasReceiveOnlyChanged = true + continue + } + info, err := f.fs.Lstat(fullDirFile) + var diskFile protocol.FileInfo + if err == nil { + diskFile, err = scanner.CreateFileInfo(info, fullDirFile, f.fs) + } + if err != nil { + // Lets just assume the file has changed. + scanChan <- fullDirFile + hasToBeScanned = true + continue + } + if !cf.IsEquivalentOptional(diskFile, f.ModTimeWindow(), f.IgnorePerms, true, protocol.LocalAllFlags) { + // File on disk changed compared to what we have in db + // -> schedule scan. + scanChan <- fullDirFile + hasToBeScanned = true + continue + } + // Dir contains file that is valid according to db and + // not ignored -> something weird is going on + hasKnown = true } if hasToBeScanned { @@ -1852,6 +1874,9 @@ func (f *sendReceiveFolder) deleteDirOnDisk(dir string, snap *db.Snapshot, scanC if hasIgnored { return errDirHasIgnored } + if hasReceiveOnlyChanged { + return errDirHasReceiveOnlyChanged + } if hasKnown { return errDirNotEmpty } diff --git a/lib/model/folder_sendrecv_test.go b/lib/model/folder_sendrecv_test.go index 874fe570a..3bdaf1db8 100644 --- a/lib/model/folder_sendrecv_test.go +++ b/lib/model/folder_sendrecv_test.go @@ -818,9 +818,14 @@ func TestCopyOwner(t *testing.T) { // owner/group. dbUpdateChan := make(chan dbUpdateJob, 1) + scanChan := make(chan string) defer close(dbUpdateChan) - f.handleDir(dir, f.fset.Snapshot(), dbUpdateChan, nil) - <-dbUpdateChan // empty the channel for later + f.handleDir(dir, f.fset.Snapshot(), dbUpdateChan, scanChan) + select { + case <-dbUpdateChan: // empty the channel for later + case toScan := <-scanChan: + t.Fatal("Unexpected receive on scanChan:", toScan) + } info, err := f.fs.Lstat("foo/bar") if err != nil { @@ -874,8 +879,12 @@ func TestCopyOwner(t *testing.T) { SymlinkTarget: "over the rainbow", } - f.handleSymlink(symlink, snap, dbUpdateChan, nil) - <-dbUpdateChan + f.handleSymlink(symlink, snap, dbUpdateChan, scanChan) + select { + case <-dbUpdateChan: + case toScan := <-scanChan: + t.Fatal("Unexpected receive on scanChan:", toScan) + } info, err = f.fs.Lstat("foo/bar/sym") if err != nil {