From d510e3cca3d5caae42121fa206b3decc981ae59e Mon Sep 17 00:00:00 2001 From: Simon Frei Date: Wed, 7 Nov 2018 11:04:41 +0100 Subject: [PATCH] all: Display errors while scanning in web UI (fixes #4480) (#5215) --- cmd/syncthing/gui.go | 14 +- cmd/syncthing/mocked_model_test.go | 2 +- lib/model/folder.go | 330 ++++++++++++++++++++++++++++- lib/model/folder_sendrecv.go | 82 +++---- lib/model/folder_sendrecv_test.go | 6 +- lib/model/folder_test.go | 150 +++++++++++++ lib/model/model.go | 300 +------------------------- lib/model/model_test.go | 135 ------------ lib/scanner/blockqueue.go | 6 +- lib/scanner/walk.go | 102 +++++---- lib/scanner/walk_test.go | 30 ++- 11 files changed, 620 insertions(+), 537 deletions(-) create mode 100644 lib/model/folder_test.go diff --git a/cmd/syncthing/gui.go b/cmd/syncthing/gui.go index d93b3d8e8..a0deca97f 100644 --- a/cmd/syncthing/gui.go +++ b/cmd/syncthing/gui.go @@ -115,7 +115,7 @@ type modelIntf interface { RemoteSequence(folder string) (int64, bool) State(folder string) (string, time.Time, error) UsageReportingStats(version int, preview bool) map[string]interface{} - PullErrors(folder string) ([]model.FileError, error) + FolderErrors(folder string) ([]model.FileError, error) WatchError(folder string) error } @@ -261,7 +261,8 @@ func (s *apiService) Serve() { getRestMux.HandleFunc("/rest/db/status", s.getDBStatus) // folder getRestMux.HandleFunc("/rest/db/browse", s.getDBBrowse) // folder [prefix] [dirsonly] [levels] getRestMux.HandleFunc("/rest/folder/versions", s.getFolderVersions) // folder - getRestMux.HandleFunc("/rest/folder/pullerrors", s.getPullErrors) // folder + getRestMux.HandleFunc("/rest/folder/errors", s.getFolderErrors) // folder + getRestMux.HandleFunc("/rest/folder/pullerrors", s.getFolderErrors) // folder (deprecated) getRestMux.HandleFunc("/rest/events", s.getIndexEvents) // [since] [limit] [timeout] [events] getRestMux.HandleFunc("/rest/events/disk", s.getDiskEvents) // [since] [limit] [timeout] getRestMux.HandleFunc("/rest/stats/device", s.getDeviceStats) // - @@ -695,12 +696,13 @@ func (s *apiService) getDBStatus(w http.ResponseWriter, r *http.Request) { func folderSummary(cfg configIntf, m modelIntf, folder string) (map[string]interface{}, error) { var res = make(map[string]interface{}) - pullErrors, err := m.PullErrors(folder) + errors, err := m.FolderErrors(folder) if err != nil && err != model.ErrFolderPaused { // Stats from the db can still be obtained if the folder is just paused return nil, err } - res["pullErrors"] = len(pullErrors) + res["errors"] = len(errors) + res["pullErrors"] = len(errors) // deprecated res["invalid"] = "" // Deprecated, retains external API for now @@ -1501,12 +1503,12 @@ func (s *apiService) postFolderVersionsRestore(w http.ResponseWriter, r *http.Re sendJSON(w, ferr) } -func (s *apiService) getPullErrors(w http.ResponseWriter, r *http.Request) { +func (s *apiService) getFolderErrors(w http.ResponseWriter, r *http.Request) { qs := r.URL.Query() folder := qs.Get("folder") page, perpage := getPagingParams(qs) - errors, err := s.model.PullErrors(folder) + errors, err := s.model.FolderErrors(folder) if err != nil { http.Error(w, err.Error(), http.StatusNotFound) diff --git a/cmd/syncthing/mocked_model_test.go b/cmd/syncthing/mocked_model_test.go index 04e854aa1..2df049bbe 100644 --- a/cmd/syncthing/mocked_model_test.go +++ b/cmd/syncthing/mocked_model_test.go @@ -139,7 +139,7 @@ func (m *mockedModel) UsageReportingStats(version int, preview bool) map[string] return nil } -func (m *mockedModel) PullErrors(folder string) ([]model.FileError, error) { +func (m *mockedModel) FolderErrors(folder string) ([]model.FileError, error) { return nil, nil } diff --git a/lib/model/folder.go b/lib/model/folder.go index 41db4b689..88ae5717a 100644 --- a/lib/model/folder.go +++ b/lib/model/folder.go @@ -11,14 +11,20 @@ import ( "errors" "fmt" "math/rand" + "path/filepath" + "sort" + "strings" "sync/atomic" "time" "github.com/syncthing/syncthing/lib/config" "github.com/syncthing/syncthing/lib/db" "github.com/syncthing/syncthing/lib/events" + "github.com/syncthing/syncthing/lib/fs" "github.com/syncthing/syncthing/lib/ignore" + "github.com/syncthing/syncthing/lib/osutil" "github.com/syncthing/syncthing/lib/protocol" + "github.com/syncthing/syncthing/lib/scanner" "github.com/syncthing/syncthing/lib/sync" "github.com/syncthing/syncthing/lib/watchaggregator" ) @@ -41,6 +47,8 @@ type folder struct { scanDelay chan time.Duration initialScanFinished chan struct{} stopped chan struct{} + scanErrors []FileError + scanErrorsMut sync.Mutex pullScheduled chan struct{} @@ -80,6 +88,7 @@ func newFolder(model *Model, cfg config.FolderConfiguration) folder { scanDelay: make(chan time.Duration), initialScanFinished: make(chan struct{}), stopped: make(chan struct{}), + scanErrorsMut: sync.NewMutex(), pullScheduled: make(chan struct{}, 1), // This needs to be 1-buffered so that we queue a pull if we're busy when it comes. @@ -266,14 +275,255 @@ func (f *folder) getHealthError() error { } func (f *folder) scanSubdirs(subDirs []string) error { - if err := f.model.internalScanFolderSubdirs(f.ctx, f.folderID, subDirs, f.localFlags); err != nil { - // Potentially sets the error twice, once in the scanner just - // by doing a check, and once here, if the error returned is - // the same one as returned by CheckHealth, though - // duplicate set is handled by setError. + if err := f.CheckHealth(); err != nil { + return err + } + + f.model.fmut.RLock() + fset := f.model.folderFiles[f.ID] + ignores := f.model.folderIgnores[f.ID] + f.model.fmut.RUnlock() + mtimefs := fset.MtimeFS() + + for i := range subDirs { + sub := osutil.NativeFilename(subDirs[i]) + + if sub == "" { + // A blank subdirs means to scan the entire folder. We can trim + // the subDirs list and go on our way. + subDirs = nil + break + } + + subDirs[i] = sub + } + + // Check if the ignore patterns changed as part of scanning this folder. + // If they did we should schedule a pull of the folder so that we + // request things we might have suddenly become unignored and so on. + oldHash := ignores.Hash() + defer func() { + if ignores.Hash() != oldHash { + l.Debugln("Folder", f.ID, "ignore patterns changed; triggering puller") + f.IgnoresUpdated() + } + }() + + if err := ignores.Load(".stignore"); err != nil && !fs.IsNotExist(err) { + err = fmt.Errorf("loading ignores: %v", err) f.setError(err) return err } + + // Clean the list of subitems to ensure that we start at a known + // directory, and don't scan subdirectories of things we've already + // scanned. + subDirs = unifySubs(subDirs, func(f string) bool { + _, ok := fset.Get(protocol.LocalDeviceID, f) + return ok + }) + + f.setState(FolderScanning) + + fchan := scanner.Walk(f.ctx, scanner.Config{ + Folder: f.ID, + Subs: subDirs, + Matcher: ignores, + TempLifetime: time.Duration(f.model.cfg.Options().KeepTemporariesH) * time.Hour, + CurrentFiler: cFiler{f.model, f.ID}, + Filesystem: mtimefs, + IgnorePerms: f.IgnorePerms, + AutoNormalize: f.AutoNormalize, + Hashers: f.model.numHashers(f.ID), + ShortID: f.model.shortID, + ProgressTickIntervalS: f.ScanProgressIntervalS, + UseLargeBlocks: f.UseLargeBlocks, + LocalFlags: f.localFlags, + }) + + batchFn := func(fs []protocol.FileInfo) error { + if err := f.CheckHealth(); err != nil { + l.Debugf("Stopping scan of folder %s due to: %s", f.Description(), err) + return err + } + f.model.updateLocalsFromScanning(f.ID, fs) + return nil + } + // Resolve items which are identical with the global state. + if f.localFlags&protocol.FlagLocalReceiveOnly != 0 { + oldBatchFn := batchFn // can't reference batchFn directly (recursion) + batchFn = func(fs []protocol.FileInfo) error { + for i := range fs { + switch gf, ok := fset.GetGlobal(fs[i].Name); { + case !ok: + continue + case gf.IsEquivalentOptional(fs[i], false, false, protocol.FlagLocalReceiveOnly): + // What we have locally is equivalent to the global file. + fs[i].Version = fs[i].Version.Merge(gf.Version) + fallthrough + case fs[i].IsDeleted() && gf.IsReceiveOnlyChanged(): + // Our item is deleted and the global item is our own + // receive only file. We can't delete file infos, so + // we just pretend it is a normal deleted file (nobody + // cares about that). + fs[i].LocalFlags &^= protocol.FlagLocalReceiveOnly + } + } + return oldBatchFn(fs) + } + } + batch := newFileInfoBatch(batchFn) + + // Schedule a pull after scanning, but only if we actually detected any + // changes. + changes := 0 + defer func() { + if changes > 0 { + f.SchedulePull() + } + }() + + f.clearScanErrors(subDirs) + for res := range fchan { + if res.Err != nil { + f.newScanError(res.Path, res.Err) + continue + } + if err := batch.flushIfFull(); err != nil { + return err + } + + batch.append(res.File) + changes++ + } + + if err := batch.flush(); err != nil { + return err + } + + if len(subDirs) == 0 { + // If we have no specific subdirectories to traverse, set it to one + // empty prefix so we traverse the entire folder contents once. + subDirs = []string{""} + } + + // Do a scan of the database for each prefix, to check for deleted and + // ignored files. + var toIgnore []db.FileInfoTruncated + ignoredParent := "" + pathSep := string(fs.PathSeparator) + for _, sub := range subDirs { + var iterError error + + fset.WithPrefixedHaveTruncated(protocol.LocalDeviceID, sub, func(fi db.FileIntf) bool { + file := fi.(db.FileInfoTruncated) + + if err := batch.flushIfFull(); err != nil { + iterError = err + return false + } + + if ignoredParent != "" && !strings.HasPrefix(file.Name, ignoredParent+pathSep) { + for _, file := range toIgnore { + l.Debugln("marking file as ignored", file) + nf := file.ConvertToIgnoredFileInfo(f.model.id.Short()) + batch.append(nf) + changes++ + if err := batch.flushIfFull(); err != nil { + iterError = err + return false + } + } + toIgnore = toIgnore[:0] + ignoredParent = "" + } + + switch ignored := ignores.Match(file.Name).IsIgnored(); { + case !file.IsIgnored() && ignored: + // File was not ignored at last pass but has been ignored. + if file.IsDirectory() { + // Delay ignoring as a child might be unignored. + toIgnore = append(toIgnore, file) + if ignoredParent == "" { + // If the parent wasn't ignored already, set + // this path as the "highest" ignored parent + ignoredParent = file.Name + } + return true + } + + l.Debugln("marking file as ignored", f) + nf := file.ConvertToIgnoredFileInfo(f.model.id.Short()) + batch.append(nf) + changes++ + + case file.IsIgnored() && !ignored: + // Successfully scanned items are already un-ignored during + // the scan, so check whether it is deleted. + fallthrough + case !file.IsIgnored() && !file.IsDeleted() && !file.IsUnsupported(): + // The file is not ignored, deleted or unsupported. Lets check if + // it's still here. Simply stat:ing it wont do as there are + // tons of corner cases (e.g. parent dir->symlink, missing + // permissions) + if !osutil.IsDeleted(mtimefs, file.Name) { + if ignoredParent != "" { + // Don't ignore parents of this not ignored item + toIgnore = toIgnore[:0] + ignoredParent = "" + } + return true + } + nf := protocol.FileInfo{ + Name: file.Name, + Type: file.Type, + Size: 0, + ModifiedS: file.ModifiedS, + ModifiedNs: file.ModifiedNs, + ModifiedBy: f.model.id.Short(), + Deleted: true, + Version: file.Version.Update(f.model.shortID), + LocalFlags: f.localFlags, + } + // We do not want to override the global version + // with the deleted file. Keeping only our local + // counter makes sure we are in conflict with any + // other existing versions, which will be resolved + // by the normal pulling mechanisms. + if file.ShouldConflict() { + nf.Version = nf.Version.DropOthers(f.model.shortID) + } + + batch.append(nf) + changes++ + } + return true + }) + + if iterError == nil && len(toIgnore) > 0 { + for _, file := range toIgnore { + l.Debugln("marking file as ignored", f) + nf := file.ConvertToIgnoredFileInfo(f.model.id.Short()) + batch.append(nf) + changes++ + if iterError = batch.flushIfFull(); iterError != nil { + break + } + } + toIgnore = toIgnore[:0] + } + + if iterError != nil { + return iterError + } + } + + if err := batch.flush(); err != nil { + return err + } + + f.model.folderStatRef(f.ID).ScanCompleted() + f.setState(FolderIdle) return nil } @@ -430,3 +680,73 @@ func (f *folder) basePause() time.Duration { func (f *folder) String() string { return fmt.Sprintf("%s/%s@%p", f.Type, f.folderID, f) } + +func (f *folder) newScanError(path string, err error) { + f.scanErrorsMut.Lock() + f.scanErrors = append(f.scanErrors, FileError{ + Err: err.Error(), + Path: path, + }) + f.scanErrorsMut.Unlock() +} + +func (f *folder) clearScanErrors(subDirs []string) { + f.scanErrorsMut.Lock() + defer f.scanErrorsMut.Unlock() + if len(subDirs) == 0 { + f.scanErrors = nil + return + } + filtered := f.scanErrors[:0] + pathSep := string(fs.PathSeparator) +outer: + for _, fe := range f.scanErrors { + for _, sub := range subDirs { + if strings.HasPrefix(fe.Path, sub+pathSep) { + continue outer + } + } + filtered = append(filtered, fe) + } + f.scanErrors = filtered +} + +func (f *folder) Errors() []FileError { + f.scanErrorsMut.Lock() + defer f.scanErrorsMut.Unlock() + return append([]FileError{}, f.scanErrors...) +} + +// The exists function is expected to return true for all known paths +// (excluding "" and ".") +func unifySubs(dirs []string, exists func(dir string) bool) []string { + if len(dirs) == 0 { + return nil + } + sort.Strings(dirs) + if dirs[0] == "" || dirs[0] == "." || dirs[0] == string(fs.PathSeparator) { + return nil + } + prev := "./" // Anything that can't be parent of a clean path + for i := 0; i < len(dirs); { + dir, err := fs.Canonicalize(dirs[i]) + if err != nil { + l.Debugf("Skipping %v for scan: %s", dirs[i], err) + dirs = append(dirs[:i], dirs[i+1:]...) + continue + } + if dir == prev || strings.HasPrefix(dir, prev+string(fs.PathSeparator)) { + dirs = append(dirs[:i], dirs[i+1:]...) + continue + } + parent := filepath.Dir(dir) + for parent != "." && parent != string(fs.PathSeparator) && !exists(parent) { + dir = parent + parent = filepath.Dir(dir) + } + dirs[i] = dir + prev = dir + i++ + } + return dirs +} diff --git a/lib/model/folder_sendrecv.go b/lib/model/folder_sendrecv.go index d37250730..fe6988691 100644 --- a/lib/model/folder_sendrecv.go +++ b/lib/model/folder_sendrecv.go @@ -102,17 +102,17 @@ type sendReceiveFolder struct { queue *jobQueue - errors map[string]string // path -> error string - errorsMut sync.Mutex + pullErrors map[string]string // path -> error string + pullErrorsMut sync.Mutex } func newSendReceiveFolder(model *Model, cfg config.FolderConfiguration, ver versioner.Versioner, fs fs.Filesystem) service { f := &sendReceiveFolder{ - folder: newFolder(model, cfg), - fs: fs, - versioner: ver, - queue: newJobQueue(), - errorsMut: sync.NewMutex(), + folder: newFolder(model, cfg), + fs: fs, + versioner: ver, + queue: newJobQueue(), + pullErrorsMut: sync.NewMutex(), } f.folder.puller = f @@ -167,7 +167,7 @@ func (f *sendReceiveFolder) pull() bool { l.Debugf("%v pulling (ignoresChanged=%v)", f, ignoresChanged) f.setState(FolderSyncing) - f.clearErrors() + f.clearPullErrors() scanChan := make(chan string) go f.pullScannerRoutine(scanChan) @@ -204,10 +204,10 @@ func (f *sendReceiveFolder) pull() bool { // we're not making it. Probably there are write // errors preventing us. Flag this with a warning and // wait a bit longer before retrying. - if folderErrors := f.PullErrors(); len(folderErrors) > 0 { + if errors := f.Errors(); len(errors) > 0 { events.Default.Log(events.FolderErrors, map[string]interface{}{ "folder": f.folderID, - "errors": folderErrors, + "errors": errors, }) } break @@ -327,7 +327,7 @@ func (f *sendReceiveFolder) processNeeded(ignores *ignore.Matcher, folderFiles * changed++ case runtime.GOOS == "windows" && fs.WindowsInvalidFilename(file.Name): - f.newError("pull", file.Name, fs.ErrInvalidFilename) + f.newPullError("pull", file.Name, fs.ErrInvalidFilename) case file.IsDeleted(): if file.IsDirectory() { @@ -497,7 +497,7 @@ nextFile: continue nextFile } } - f.newError("pull", fileName, errNotAvailable) + f.newPullError("pull", fileName, errNotAvailable) } return changed, fileDeletions, dirDeletions, nil @@ -513,7 +513,7 @@ func (f *sendReceiveFolder) processDeletions(ignores *ignore.Matcher, fileDeleti l.Debugln(f, "Deleting file", file.Name) if update, err := f.deleteFile(file, scanChan); err != nil { - f.newError("delete file", file.Name, err) + f.newPullError("delete file", file.Name, err) } else { dbUpdateChan <- update } @@ -573,7 +573,7 @@ func (f *sendReceiveFolder) handleDir(file protocol.FileInfo, dbUpdateChan chan< case err == nil && (!info.IsDir() || info.IsSymlink()): err = osutil.InWritableDir(f.fs.Remove, f.fs, file.Name) if err != nil { - f.newError("dir replace", file.Name, err) + f.newPullError("dir replace", file.Name, err) return } fallthrough @@ -603,13 +603,13 @@ func (f *sendReceiveFolder) handleDir(file protocol.FileInfo, dbUpdateChan chan< if err = osutil.InWritableDir(mkdir, f.fs, file.Name); err == nil { dbUpdateChan <- dbUpdateJob{file, dbUpdateHandleDir} } else { - f.newError("dir mkdir", file.Name, err) + f.newPullError("dir mkdir", file.Name, err) } return // Weird error when stat()'ing the dir. Probably won't work to do // anything else with it if we can't even stat() it. case err != nil: - f.newError("dir stat", file.Name, err) + f.newPullError("dir stat", file.Name, err) return } @@ -621,7 +621,7 @@ func (f *sendReceiveFolder) handleDir(file protocol.FileInfo, dbUpdateChan chan< } else if err := f.fs.Chmod(file.Name, mode|(fs.FileMode(info.Mode())&retainBits)); err == nil { dbUpdateChan <- dbUpdateJob{file, dbUpdateHandleDir} } else { - f.newError("dir chmod", file.Name, err) + f.newPullError("dir chmod", file.Name, err) } } @@ -631,7 +631,7 @@ func (f *sendReceiveFolder) checkParent(file string, scanChan chan<- string) boo parent := filepath.Dir(file) if err := osutil.TraversesSymlink(f.fs, parent); err != nil { - f.newError("traverses q", file, err) + f.newPullError("traverses q", file, err) return false } @@ -652,7 +652,7 @@ func (f *sendReceiveFolder) checkParent(file string, scanChan chan<- string) boo } l.Debugf("%v resurrecting parent directory of %v", f, file) if err := f.fs.MkdirAll(parent, 0755); err != nil { - f.newError("resurrecting parent dir", file, err) + f.newPullError("resurrecting parent dir", file, err) return false } scanChan <- parent @@ -691,7 +691,7 @@ func (f *sendReceiveFolder) handleSymlink(file protocol.FileInfo, dbUpdateChan c // Index entry from a Syncthing predating the support for including // the link target in the index entry. We log this as an error. err = errors.New("incompatible symlink entry; rescan with newer Syncthing on source") - f.newError("symlink", file.Name, err) + f.newPullError("symlink", file.Name, err) return } @@ -701,7 +701,7 @@ func (f *sendReceiveFolder) handleSymlink(file protocol.FileInfo, dbUpdateChan c // path. err = osutil.InWritableDir(f.fs.Remove, f.fs, file.Name) if err != nil { - f.newError("symlink remove", file.Name, err) + f.newPullError("symlink remove", file.Name, err) return } } @@ -715,7 +715,7 @@ func (f *sendReceiveFolder) handleSymlink(file protocol.FileInfo, dbUpdateChan c if err = osutil.InWritableDir(createLink, f.fs, file.Name); err == nil { dbUpdateChan <- dbUpdateJob{file, dbUpdateHandleSymlink} } else { - f.newError("symlink create", file.Name, err) + f.newPullError("symlink create", file.Name, err) } } @@ -743,7 +743,7 @@ func (f *sendReceiveFolder) handleDeleteDir(file protocol.FileInfo, ignores *ign }() if err = f.deleteDir(file.Name, ignores, scanChan); err != nil { - f.newError("delete dir", file.Name, err) + f.newPullError("delete dir", file.Name, err) return } @@ -1018,7 +1018,7 @@ func (f *sendReceiveFolder) handleFile(file protocol.FileInfo, copyChan chan<- c } if err := f.CheckAvailableSpace(blocksSize); err != nil { - f.newError("pulling file", file.Name, err) + f.newPullError("pulling file", file.Name, err) f.queue.Done(file.Name) return } @@ -1130,7 +1130,7 @@ func (f *sendReceiveFolder) shortcutFile(file, curFile protocol.FileInfo, dbUpda if !f.IgnorePerms && !file.NoPermissions { if err = f.fs.Chmod(file.Name, fs.FileMode(file.Permissions&0777)); err != nil { - f.newError("shortcut", file.Name, err) + f.newPullError("shortcut", file.Name, err) return } } @@ -1543,7 +1543,7 @@ func (f *sendReceiveFolder) finisherRoutine(ignores *ignore.Matcher, in <-chan * } if err != nil { - f.newError("finisher", state.file.Name, err) + f.newPullError("finisher", state.file.Name, err) } else { blockStatsMut.Lock() blockStats["total"] += state.reused + state.copyTotal + state.pullTotal @@ -1768,34 +1768,36 @@ func (f *sendReceiveFolder) moveForConflict(name string, lastModBy string) error return err } -func (f *sendReceiveFolder) newError(context, path string, err error) { - f.errorsMut.Lock() - defer f.errorsMut.Unlock() +func (f *sendReceiveFolder) newPullError(context, path string, err error) { + f.pullErrorsMut.Lock() + defer f.pullErrorsMut.Unlock() // We might get more than one error report for a file (i.e. error on // Write() followed by Close()); we keep the first error as that is // probably closer to the root cause. - if _, ok := f.errors[path]; ok { + if _, ok := f.pullErrors[path]; ok { return } l.Infof("Puller (folder %s, file %q): %s: %v", f.Description(), path, context, err) - f.errors[path] = fmt.Sprintf("%s: %s", context, err.Error()) + f.pullErrors[path] = fmt.Sprintf("%s: %s", context, err.Error()) } -func (f *sendReceiveFolder) clearErrors() { - f.errorsMut.Lock() - f.errors = make(map[string]string) - f.errorsMut.Unlock() +func (f *sendReceiveFolder) clearPullErrors() { + f.pullErrorsMut.Lock() + f.pullErrors = make(map[string]string) + f.pullErrorsMut.Unlock() } -func (f *sendReceiveFolder) PullErrors() []FileError { - f.errorsMut.Lock() - errors := make([]FileError, 0, len(f.errors)) - for path, err := range f.errors { +func (f *sendReceiveFolder) Errors() []FileError { + scanErrors := f.folder.Errors() + f.pullErrorsMut.Lock() + errors := make([]FileError, 0, len(f.pullErrors)+len(f.scanErrors)) + for path, err := range f.pullErrors { errors = append(errors, FileError{path, err}) } + f.pullErrorsMut.Unlock() + errors = append(errors, scanErrors...) sort.Sort(fileErrorList(errors)) - f.errorsMut.Unlock() return errors } diff --git a/lib/model/folder_sendrecv_test.go b/lib/model/folder_sendrecv_test.go index d074d50dd..95e041b16 100644 --- a/lib/model/folder_sendrecv_test.go +++ b/lib/model/folder_sendrecv_test.go @@ -97,9 +97,9 @@ func setUpSendReceiveFolder(model *Model) *sendReceiveFolder { }, }, - queue: newJobQueue(), - errors: make(map[string]string), - errorsMut: sync.NewMutex(), + queue: newJobQueue(), + pullErrors: make(map[string]string), + pullErrorsMut: sync.NewMutex(), } f.fs = fs.NewMtimeFS(f.Filesystem(), db.NewNamespacedKV(model.db, "mtime")) diff --git a/lib/model/folder_test.go b/lib/model/folder_test.go new file mode 100644 index 000000000..a13fd9eb6 --- /dev/null +++ b/lib/model/folder_test.go @@ -0,0 +1,150 @@ +// Copyright (C) 2018 The Syncthing Authors. +// +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this file, +// You can obtain one at https://mozilla.org/MPL/2.0/. + +package model + +import ( + "path/filepath" + "runtime" + "testing" + + "github.com/d4l3k/messagediff" + "github.com/syncthing/syncthing/lib/config" +) + +type unifySubsCase struct { + in []string // input to unifySubs + exists []string // paths that exist in the database + out []string // expected output +} + +func unifySubsCases() []unifySubsCase { + cases := []unifySubsCase{ + { + // 0. trailing slashes are cleaned, known paths are just passed on + []string{"foo/", "bar//"}, + []string{"foo", "bar"}, + []string{"bar", "foo"}, // the output is sorted + }, + { + // 1. "foo/bar" gets trimmed as it's covered by foo + []string{"foo", "bar/", "foo/bar/"}, + []string{"foo", "bar"}, + []string{"bar", "foo"}, + }, + { + // 2. "" gets simplified to the empty list; ie scan all + []string{"foo", ""}, + []string{"foo"}, + nil, + }, + { + // 3. "foo/bar" is unknown, but it's kept + // because its parent is known + []string{"foo/bar"}, + []string{"foo"}, + []string{"foo/bar"}, + }, + { + // 4. two independent known paths, both are kept + // "usr/lib" is not a prefix of "usr/libexec" + []string{"usr/lib", "usr/libexec"}, + []string{"usr", "usr/lib", "usr/libexec"}, + []string{"usr/lib", "usr/libexec"}, + }, + { + // 5. "usr/lib" is a prefix of "usr/lib/exec" + []string{"usr/lib", "usr/lib/exec"}, + []string{"usr", "usr/lib", "usr/libexec"}, + []string{"usr/lib"}, + }, + { + // 6. .stignore and .stfolder are special and are passed on + // verbatim even though they are unknown + []string{config.DefaultMarkerName, ".stignore"}, + []string{}, + []string{config.DefaultMarkerName, ".stignore"}, + }, + { + // 7. but the presence of something else unknown forces an actual + // scan + []string{config.DefaultMarkerName, ".stignore", "foo/bar"}, + []string{}, + []string{config.DefaultMarkerName, ".stignore", "foo"}, + }, + { + // 8. explicit request to scan all + nil, + []string{"foo"}, + nil, + }, + { + // 9. empty list of subs + []string{}, + []string{"foo"}, + nil, + }, + { + // 10. absolute path + []string{"/foo"}, + []string{"foo"}, + []string{"foo"}, + }, + } + + if runtime.GOOS == "windows" { + // Fixup path separators + for i := range cases { + for j, p := range cases[i].in { + cases[i].in[j] = filepath.FromSlash(p) + } + for j, p := range cases[i].exists { + cases[i].exists[j] = filepath.FromSlash(p) + } + for j, p := range cases[i].out { + cases[i].out[j] = filepath.FromSlash(p) + } + } + } + + return cases +} + +func unifyExists(f string, tc unifySubsCase) bool { + for _, e := range tc.exists { + if f == e { + return true + } + } + return false +} + +func TestUnifySubs(t *testing.T) { + cases := unifySubsCases() + for i, tc := range cases { + exists := func(f string) bool { + return unifyExists(f, tc) + } + out := unifySubs(tc.in, exists) + if diff, equal := messagediff.PrettyDiff(tc.out, out); !equal { + t.Errorf("Case %d failed; got %v, expected %v, diff:\n%s", i, out, tc.out, diff) + } + } +} + +func BenchmarkUnifySubs(b *testing.B) { + cases := unifySubsCases() + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + for _, tc := range cases { + exists := func(f string) bool { + return unifyExists(f, tc) + } + unifySubs(tc.in, exists) + } + } +} diff --git a/lib/model/model.go b/lib/model/model.go index 21c2fa349..a92d833f7 100644 --- a/lib/model/model.go +++ b/lib/model/model.go @@ -8,7 +8,6 @@ package model import ( "bytes" - "context" "crypto/tls" "encoding/json" "errors" @@ -18,7 +17,6 @@ import ( "path/filepath" "reflect" "runtime" - "sort" "strings" stdsync "sync" "time" @@ -67,7 +65,7 @@ type service interface { Serve() Stop() CheckHealth() error - PullErrors() []FileError + Errors() []FileError WatchError() error getState() (folderState, time.Time, error) @@ -1981,264 +1979,6 @@ func (m *Model) ScanFolderSubdirs(folder string, subs []string) error { return runner.Scan(subs) } -func (m *Model) internalScanFolderSubdirs(ctx context.Context, folder string, subDirs []string, localFlags uint32) error { - m.fmut.RLock() - if err := m.checkFolderRunningLocked(folder); err != nil { - m.fmut.RUnlock() - return err - } - fset := m.folderFiles[folder] - folderCfg := m.folderCfgs[folder] - ignores := m.folderIgnores[folder] - runner := m.folderRunners[folder] - m.fmut.RUnlock() - mtimefs := fset.MtimeFS() - - for i := range subDirs { - sub := osutil.NativeFilename(subDirs[i]) - - if sub == "" { - // A blank subdirs means to scan the entire folder. We can trim - // the subDirs list and go on our way. - subDirs = nil - break - } - - subDirs[i] = sub - } - - // Check if the ignore patterns changed as part of scanning this folder. - // If they did we should schedule a pull of the folder so that we - // request things we might have suddenly become unignored and so on. - oldHash := ignores.Hash() - defer func() { - if ignores.Hash() != oldHash { - l.Debugln("Folder", folder, "ignore patterns changed; triggering puller") - runner.IgnoresUpdated() - } - }() - - if err := runner.CheckHealth(); err != nil { - return err - } - - if err := ignores.Load(".stignore"); err != nil && !fs.IsNotExist(err) { - err = fmt.Errorf("loading ignores: %v", err) - runner.setError(err) - return err - } - - // Clean the list of subitems to ensure that we start at a known - // directory, and don't scan subdirectories of things we've already - // scanned. - subDirs = unifySubs(subDirs, func(f string) bool { - _, ok := fset.Get(protocol.LocalDeviceID, f) - return ok - }) - - runner.setState(FolderScanning) - - fchan := scanner.Walk(ctx, scanner.Config{ - Folder: folderCfg.ID, - Subs: subDirs, - Matcher: ignores, - TempLifetime: time.Duration(m.cfg.Options().KeepTemporariesH) * time.Hour, - CurrentFiler: cFiler{m, folder}, - Filesystem: mtimefs, - IgnorePerms: folderCfg.IgnorePerms, - AutoNormalize: folderCfg.AutoNormalize, - Hashers: m.numHashers(folder), - ShortID: m.shortID, - ProgressTickIntervalS: folderCfg.ScanProgressIntervalS, - UseLargeBlocks: folderCfg.UseLargeBlocks, - LocalFlags: localFlags, - }) - - if err := runner.CheckHealth(); err != nil { - return err - } - - batchFn := func(fs []protocol.FileInfo) error { - if err := runner.CheckHealth(); err != nil { - l.Debugf("Stopping scan of folder %s due to: %s", folderCfg.Description(), err) - return err - } - m.updateLocalsFromScanning(folder, fs) - return nil - } - // Resolve items which are identical with the global state. - if localFlags&protocol.FlagLocalReceiveOnly != 0 { - oldBatchFn := batchFn // can't reference batchFn directly (recursion) - batchFn = func(fs []protocol.FileInfo) error { - for i := range fs { - switch gf, ok := fset.GetGlobal(fs[i].Name); { - case !ok: - continue - case gf.IsEquivalentOptional(fs[i], false, false, protocol.FlagLocalReceiveOnly): - // What we have locally is equivalent to the global file. - fs[i].Version = fs[i].Version.Merge(gf.Version) - fallthrough - case fs[i].IsDeleted() && gf.IsReceiveOnlyChanged(): - // Our item is deleted and the global item is our own - // receive only file. We can't delete file infos, so - // we just pretend it is a normal deleted file (nobody - // cares about that). - fs[i].LocalFlags &^= protocol.FlagLocalReceiveOnly - } - } - return oldBatchFn(fs) - } - } - batch := newFileInfoBatch(batchFn) - - // Schedule a pull after scanning, but only if we actually detected any - // changes. - changes := 0 - defer func() { - if changes > 0 { - runner.SchedulePull() - } - }() - - for f := range fchan { - if err := batch.flushIfFull(); err != nil { - return err - } - - batch.append(f) - changes++ - } - - if err := batch.flush(); err != nil { - return err - } - - if len(subDirs) == 0 { - // If we have no specific subdirectories to traverse, set it to one - // empty prefix so we traverse the entire folder contents once. - subDirs = []string{""} - } - - // Do a scan of the database for each prefix, to check for deleted and - // ignored files. - var toIgnore []db.FileInfoTruncated - ignoredParent := "" - pathSep := string(fs.PathSeparator) - for _, sub := range subDirs { - var iterError error - - fset.WithPrefixedHaveTruncated(protocol.LocalDeviceID, sub, func(fi db.FileIntf) bool { - f := fi.(db.FileInfoTruncated) - - if err := batch.flushIfFull(); err != nil { - iterError = err - return false - } - - if ignoredParent != "" && !strings.HasPrefix(f.Name, ignoredParent+pathSep) { - for _, f := range toIgnore { - l.Debugln("marking file as ignored", f) - nf := f.ConvertToIgnoredFileInfo(m.id.Short()) - batch.append(nf) - changes++ - if err := batch.flushIfFull(); err != nil { - iterError = err - return false - } - } - toIgnore = toIgnore[:0] - ignoredParent = "" - } - - switch ignored := ignores.Match(f.Name).IsIgnored(); { - case !f.IsIgnored() && ignored: - // File was not ignored at last pass but has been ignored. - if f.IsDirectory() { - // Delay ignoring as a child might be unignored. - toIgnore = append(toIgnore, f) - if ignoredParent == "" { - // If the parent wasn't ignored already, set - // this path as the "highest" ignored parent - ignoredParent = f.Name - } - return true - } - - l.Debugln("marking file as ignored", f) - nf := f.ConvertToIgnoredFileInfo(m.id.Short()) - batch.append(nf) - changes++ - - case f.IsIgnored() && !ignored: - // Successfully scanned items are already un-ignored during - // the scan, so check whether it is deleted. - fallthrough - case !f.IsIgnored() && !f.IsDeleted() && !f.IsUnsupported(): - // The file is not ignored, deleted or unsupported. Lets check if - // it's still here. Simply stat:ing it wont do as there are - // tons of corner cases (e.g. parent dir->symlink, missing - // permissions) - if !osutil.IsDeleted(mtimefs, f.Name) { - if ignoredParent != "" { - // Don't ignore parents of this not ignored item - toIgnore = toIgnore[:0] - ignoredParent = "" - } - return true - } - nf := protocol.FileInfo{ - Name: f.Name, - Type: f.Type, - Size: 0, - ModifiedS: f.ModifiedS, - ModifiedNs: f.ModifiedNs, - ModifiedBy: m.id.Short(), - Deleted: true, - Version: f.Version.Update(m.shortID), - LocalFlags: localFlags, - } - // We do not want to override the global version - // with the deleted file. Keeping only our local - // counter makes sure we are in conflict with any - // other existing versions, which will be resolved - // by the normal pulling mechanisms. - if f.ShouldConflict() { - nf.Version = nf.Version.DropOthers(m.shortID) - } - - batch.append(nf) - changes++ - } - return true - }) - - if iterError == nil && len(toIgnore) > 0 { - for _, f := range toIgnore { - l.Debugln("marking file as ignored", f) - nf := f.ConvertToIgnoredFileInfo(m.id.Short()) - batch.append(nf) - changes++ - if iterError = batch.flushIfFull(); iterError != nil { - break - } - } - toIgnore = toIgnore[:0] - } - - if iterError != nil { - return iterError - } - } - - if err := batch.flush(); err != nil { - return err - } - - m.folderStatRef(folder).ScanCompleted() - runner.setState(FolderIdle) - return nil -} - func (m *Model) DelayScan(folder string, next time.Duration) { m.fmut.Lock() runner, ok := m.folderRunners[folder] @@ -2351,13 +2091,13 @@ func (m *Model) State(folder string) (string, time.Time, error) { return state.String(), changed, err } -func (m *Model) PullErrors(folder string) ([]FileError, error) { +func (m *Model) FolderErrors(folder string) ([]FileError, error) { m.fmut.RLock() defer m.fmut.RUnlock() if err := m.checkFolderRunningLocked(folder); err != nil { return nil, err } - return m.folderRunners[folder].PullErrors(), nil + return m.folderRunners[folder].Errors(), nil } func (m *Model) WatchError(folder string) error { @@ -2896,40 +2636,6 @@ func readOffsetIntoBuf(fs fs.Filesystem, file string, offset int64, buf []byte) return err } -// The exists function is expected to return true for all known paths -// (excluding "" and ".") -func unifySubs(dirs []string, exists func(dir string) bool) []string { - if len(dirs) == 0 { - return nil - } - sort.Strings(dirs) - if dirs[0] == "" || dirs[0] == "." || dirs[0] == string(fs.PathSeparator) { - return nil - } - prev := "./" // Anything that can't be parent of a clean path - for i := 0; i < len(dirs); { - dir, err := fs.Canonicalize(dirs[i]) - if err != nil { - l.Debugf("Skipping %v for scan: %s", dirs[i], err) - dirs = append(dirs[:i], dirs[i+1:]...) - continue - } - if dir == prev || strings.HasPrefix(dir, prev+string(fs.PathSeparator)) { - dirs = append(dirs[:i], dirs[i+1:]...) - continue - } - parent := filepath.Dir(dir) - for parent != "." && parent != string(fs.PathSeparator) && !exists(parent) { - dir = parent - parent = filepath.Dir(dir) - } - dirs[i] = dir - prev = dir - i++ - } - return dirs -} - // makeForgetUpdate takes an index update and constructs a download progress update // causing to forget any progress for files which we've just been sent. func makeForgetUpdate(files []protocol.FileInfo) []protocol.FileDownloadProgressUpdate { diff --git a/lib/model/model_test.go b/lib/model/model_test.go index f79b2e2e7..b5e381b74 100644 --- a/lib/model/model_test.go +++ b/lib/model/model_test.go @@ -24,7 +24,6 @@ import ( "testing" "time" - "github.com/d4l3k/messagediff" "github.com/syncthing/syncthing/lib/config" "github.com/syncthing/syncthing/lib/db" "github.com/syncthing/syncthing/lib/fs" @@ -2352,140 +2351,6 @@ func benchmarkTree(b *testing.B, n1, n2 int) { b.ReportAllocs() } -type unifySubsCase struct { - in []string // input to unifySubs - exists []string // paths that exist in the database - out []string // expected output -} - -func unifySubsCases() []unifySubsCase { - cases := []unifySubsCase{ - { - // 0. trailing slashes are cleaned, known paths are just passed on - []string{"foo/", "bar//"}, - []string{"foo", "bar"}, - []string{"bar", "foo"}, // the output is sorted - }, - { - // 1. "foo/bar" gets trimmed as it's covered by foo - []string{"foo", "bar/", "foo/bar/"}, - []string{"foo", "bar"}, - []string{"bar", "foo"}, - }, - { - // 2. "" gets simplified to the empty list; ie scan all - []string{"foo", ""}, - []string{"foo"}, - nil, - }, - { - // 3. "foo/bar" is unknown, but it's kept - // because its parent is known - []string{"foo/bar"}, - []string{"foo"}, - []string{"foo/bar"}, - }, - { - // 4. two independent known paths, both are kept - // "usr/lib" is not a prefix of "usr/libexec" - []string{"usr/lib", "usr/libexec"}, - []string{"usr", "usr/lib", "usr/libexec"}, - []string{"usr/lib", "usr/libexec"}, - }, - { - // 5. "usr/lib" is a prefix of "usr/lib/exec" - []string{"usr/lib", "usr/lib/exec"}, - []string{"usr", "usr/lib", "usr/libexec"}, - []string{"usr/lib"}, - }, - { - // 6. .stignore and .stfolder are special and are passed on - // verbatim even though they are unknown - []string{config.DefaultMarkerName, ".stignore"}, - []string{}, - []string{config.DefaultMarkerName, ".stignore"}, - }, - { - // 7. but the presence of something else unknown forces an actual - // scan - []string{config.DefaultMarkerName, ".stignore", "foo/bar"}, - []string{}, - []string{config.DefaultMarkerName, ".stignore", "foo"}, - }, - { - // 8. explicit request to scan all - nil, - []string{"foo"}, - nil, - }, - { - // 9. empty list of subs - []string{}, - []string{"foo"}, - nil, - }, - { - // 10. absolute path - []string{"/foo"}, - []string{"foo"}, - []string{"foo"}, - }, - } - - if runtime.GOOS == "windows" { - // Fixup path separators - for i := range cases { - for j, p := range cases[i].in { - cases[i].in[j] = filepath.FromSlash(p) - } - for j, p := range cases[i].exists { - cases[i].exists[j] = filepath.FromSlash(p) - } - for j, p := range cases[i].out { - cases[i].out[j] = filepath.FromSlash(p) - } - } - } - - return cases -} - -func unifyExists(f string, tc unifySubsCase) bool { - for _, e := range tc.exists { - if f == e { - return true - } - } - return false -} - -func TestUnifySubs(t *testing.T) { - cases := unifySubsCases() - for i, tc := range cases { - exists := func(f string) bool { - return unifyExists(f, tc) - } - out := unifySubs(tc.in, exists) - if diff, equal := messagediff.PrettyDiff(tc.out, out); !equal { - t.Errorf("Case %d failed; got %v, expected %v, diff:\n%s", i, out, tc.out, diff) - } - } -} - -func BenchmarkUnifySubs(b *testing.B) { - cases := unifySubsCases() - b.ReportAllocs() - b.ResetTimer() - for i := 0; i < b.N; i++ { - for _, tc := range cases { - exists := func(f string) bool { - return unifyExists(f, tc) - } - unifySubs(tc.in, exists) - } - } -} - func TestIssue3028(t *testing.T) { // Create two files that we'll delete, one with a name that is a prefix of the other. diff --git a/lib/scanner/blockqueue.go b/lib/scanner/blockqueue.go index 6e91352ed..e17078aba 100644 --- a/lib/scanner/blockqueue.go +++ b/lib/scanner/blockqueue.go @@ -64,14 +64,14 @@ func HashFile(ctx context.Context, fs fs.Filesystem, path string, blockSize int, type parallelHasher struct { fs fs.Filesystem workers int - outbox chan<- protocol.FileInfo + outbox chan<- ScanResult inbox <-chan protocol.FileInfo counter Counter done chan<- struct{} wg sync.WaitGroup } -func newParallelHasher(ctx context.Context, fs fs.Filesystem, workers int, outbox chan<- protocol.FileInfo, inbox <-chan protocol.FileInfo, counter Counter, done chan<- struct{}) { +func newParallelHasher(ctx context.Context, fs fs.Filesystem, workers int, outbox chan<- ScanResult, inbox <-chan protocol.FileInfo, counter Counter, done chan<- struct{}) { ph := ¶llelHasher{ fs: fs, workers: workers, @@ -122,7 +122,7 @@ func (ph *parallelHasher) hashFiles(ctx context.Context) { } select { - case ph.outbox <- f: + case ph.outbox <- ScanResult{File: f}: case <-ctx.Done(): return } diff --git a/lib/scanner/walk.go b/lib/scanner/walk.go index 0877443d9..3b842c70f 100644 --- a/lib/scanner/walk.go +++ b/lib/scanner/walk.go @@ -8,6 +8,8 @@ package scanner import ( "context" + "errors" + "fmt" "path/filepath" "runtime" "strings" @@ -61,7 +63,13 @@ type CurrentFiler interface { CurrentFile(name string) (protocol.FileInfo, bool) } -func Walk(ctx context.Context, cfg Config) chan protocol.FileInfo { +type ScanResult struct { + File protocol.FileInfo + Err error + Path string // to be set in case Err != nil and File == nil +} + +func Walk(ctx context.Context, cfg Config) chan ScanResult { w := walker{cfg} if w.CurrentFiler == nil { @@ -77,17 +85,23 @@ func Walk(ctx context.Context, cfg Config) chan protocol.FileInfo { return w.walk(ctx) } +var ( + errUTF8Invalid = errors.New("item is not in UTF8 encoding") + errUTF8Normalization = errors.New("item is not in the correct UTF8 normalization form") + errUTF8Conflict = errors.New("item has UTF8 encoding conflict with another item") +) + type walker struct { Config } // Walk returns the list of files found in the local folder by scanning the // file system. Files are blockwise hashed. -func (w *walker) walk(ctx context.Context) chan protocol.FileInfo { +func (w *walker) walk(ctx context.Context) chan ScanResult { l.Debugln("Walk", w.Subs, w.Matcher) toHashChan := make(chan protocol.FileInfo) - finishedChan := make(chan protocol.FileInfo) + finishedChan := make(chan ScanResult) // A routine which walks the filesystem tree, and sends files which have // been modified to the counter routine. @@ -182,7 +196,7 @@ func (w *walker) walk(ctx context.Context) chan protocol.FileInfo { return finishedChan } -func (w *walker) walkAndHashFiles(ctx context.Context, fchan, dchan chan protocol.FileInfo) fs.WalkFunc { +func (w *walker) walkAndHashFiles(ctx context.Context, toHashChan chan<- protocol.FileInfo, finishedChan chan<- ScanResult) fs.WalkFunc { now := time.Now() ignoredParent := "" @@ -202,7 +216,7 @@ func (w *walker) walkAndHashFiles(ctx context.Context, fchan, dchan chan protoco } if err != nil { - l.Debugln("error:", path, info, err) + w.handleError(ctx, "scan", path, err, finishedChan) return skip } @@ -225,7 +239,7 @@ func (w *walker) walkAndHashFiles(ctx context.Context, fchan, dchan chan protoco } if !utf8.ValidString(path) { - l.Warnf("File name %q is not in UTF8 encoding; skipping.", path) + w.handleError(ctx, "scan", path, errUTF8Invalid, finishedChan) return skip } @@ -244,7 +258,7 @@ func (w *walker) walkAndHashFiles(ctx context.Context, fchan, dchan chan protoco if ignoredParent == "" { // parent isn't ignored, nothing special - return w.handleItem(ctx, path, fchan, dchan, skip) + return w.handleItem(ctx, path, toHashChan, finishedChan, skip) } // Part of current path below the ignored (potential) parent @@ -253,17 +267,17 @@ func (w *walker) walkAndHashFiles(ctx context.Context, fchan, dchan chan protoco // ignored path isn't actually a parent of the current path if rel == path { ignoredParent = "" - return w.handleItem(ctx, path, fchan, dchan, skip) + return w.handleItem(ctx, path, toHashChan, finishedChan, skip) } // The previously ignored parent directories of the current, not // ignored path need to be handled as well. - if err = w.handleItem(ctx, ignoredParent, fchan, dchan, skip); err != nil { + if err = w.handleItem(ctx, ignoredParent, toHashChan, finishedChan, skip); err != nil { return err } for _, name := range strings.Split(rel, string(fs.PathSeparator)) { ignoredParent = filepath.Join(ignoredParent, name) - if err = w.handleItem(ctx, ignoredParent, fchan, dchan, skip); err != nil { + if err = w.handleItem(ctx, ignoredParent, toHashChan, finishedChan, skip); err != nil { return err } } @@ -273,21 +287,23 @@ func (w *walker) walkAndHashFiles(ctx context.Context, fchan, dchan chan protoco } } -func (w *walker) handleItem(ctx context.Context, path string, fchan, dchan chan protocol.FileInfo, skip error) error { +func (w *walker) handleItem(ctx context.Context, path string, toHashChan chan<- protocol.FileInfo, finishedChan chan<- ScanResult, skip error) error { info, err := w.Filesystem.Lstat(path) // An error here would be weird as we've already gotten to this point, but act on it nonetheless if err != nil { + w.handleError(ctx, "scan", path, err, finishedChan) return skip } - path, shouldSkip := w.normalizePath(path, info) - if shouldSkip { + path, err = w.normalizePath(path, info) + if err != nil { + w.handleError(ctx, "normalizing path", path, err, finishedChan) return skip } switch { case info.IsSymlink(): - if err := w.walkSymlink(ctx, path, dchan); err != nil { + if err := w.walkSymlink(ctx, path, finishedChan); err != nil { return err } if info.IsDir() { @@ -297,16 +313,16 @@ func (w *walker) handleItem(ctx context.Context, path string, fchan, dchan chan return nil case info.IsDir(): - err = w.walkDir(ctx, path, info, dchan) + err = w.walkDir(ctx, path, info, finishedChan) case info.IsRegular(): - err = w.walkRegular(ctx, path, info, fchan) + err = w.walkRegular(ctx, path, info, toHashChan) } return err } -func (w *walker) walkRegular(ctx context.Context, relPath string, info fs.FileInfo, fchan chan protocol.FileInfo) error { +func (w *walker) walkRegular(ctx context.Context, relPath string, info fs.FileInfo, toHashChan chan<- protocol.FileInfo) error { curFile, hasCurFile := w.CurrentFiler.CurrentFile(relPath) blockSize := protocol.MinBlockSize @@ -352,7 +368,7 @@ func (w *walker) walkRegular(ctx context.Context, relPath string, info fs.FileIn l.Debugln("to hash:", relPath, f) select { - case fchan <- f: + case toHashChan <- f: case <-ctx.Done(): return ctx.Err() } @@ -360,7 +376,7 @@ func (w *walker) walkRegular(ctx context.Context, relPath string, info fs.FileIn return nil } -func (w *walker) walkDir(ctx context.Context, relPath string, info fs.FileInfo, dchan chan protocol.FileInfo) error { +func (w *walker) walkDir(ctx context.Context, relPath string, info fs.FileInfo, finishedChan chan<- ScanResult) error { curFile, hasCurFile := w.CurrentFiler.CurrentFile(relPath) f, _ := CreateFileInfo(info, relPath, nil) @@ -384,7 +400,7 @@ func (w *walker) walkDir(ctx context.Context, relPath string, info fs.FileInfo, l.Debugln("dir:", relPath, f) select { - case dchan <- f: + case finishedChan <- ScanResult{File: f}: case <-ctx.Done(): return ctx.Err() } @@ -394,7 +410,7 @@ func (w *walker) walkDir(ctx context.Context, relPath string, info fs.FileInfo, // walkSymlink returns nil or an error, if the error is of the nature that // it should stop the entire walk. -func (w *walker) walkSymlink(ctx context.Context, relPath string, dchan chan protocol.FileInfo) error { +func (w *walker) walkSymlink(ctx context.Context, relPath string, finishedChan chan<- ScanResult) error { // Symlinks are not supported on Windows. We ignore instead of returning // an error. if runtime.GOOS == "windows" { @@ -408,7 +424,7 @@ func (w *walker) walkSymlink(ctx context.Context, relPath string, dchan chan pro target, err := w.Filesystem.ReadSymlink(relPath) if err != nil { - l.Debugln("readlink error:", relPath, err) + w.handleError(ctx, "reading link:", relPath, err, finishedChan) return nil } @@ -439,7 +455,7 @@ func (w *walker) walkSymlink(ctx context.Context, relPath string, dchan chan pro l.Debugln("symlink changedb:", relPath, f) select { - case dchan <- f: + case finishedChan <- ScanResult{File: f}: case <-ctx.Done(): return ctx.Err() } @@ -449,7 +465,7 @@ func (w *walker) walkSymlink(ctx context.Context, relPath string, dchan chan pro // normalizePath returns the normalized relative path (possibly after fixing // it on disk), or skip is true. -func (w *walker) normalizePath(path string, info fs.FileInfo) (normPath string, skip bool) { +func (w *walker) normalizePath(path string, info fs.FileInfo) (normPath string, err error) { if runtime.GOOS == "darwin" { // Mac OS X file names should always be NFD normalized. normPath = norm.NFD.String(path) @@ -462,14 +478,13 @@ func (w *walker) normalizePath(path string, info fs.FileInfo) (normPath string, if path == normPath { // The file name is already normalized: nothing to do - return path, false + return path, nil } if !w.AutoNormalize { // We're not authorized to do anything about it, so complain and skip. - l.Warnf("File name %q is not in the correct UTF8 normalization form; skipping.", path) - return "", true + return "", errUTF8Normalization } // We will attempt to normalize it. @@ -477,11 +492,12 @@ func (w *walker) normalizePath(path string, info fs.FileInfo) (normPath string, if fs.IsNotExist(err) { // Nothing exists with the normalized filename. Good. if err = w.Filesystem.Rename(path, normPath); err != nil { - l.Infof(`Error normalizing UTF8 encoding of file "%s": %v`, path, err) - return "", true + return "", err } l.Infof(`Normalized UTF8 encoding of file name "%s".`, path) - } else if w.Filesystem.SameFile(info, normInfo) { + return normPath, nil + } + if w.Filesystem.SameFile(info, normInfo) { // With some filesystems (ZFS), if there is an un-normalized path and you ask whether the normalized // version exists, it responds with true. Therefore we need to check fs.SameFile as well. // In this case, a call to Rename won't do anything, so we have to rename via a temp file. @@ -491,23 +507,19 @@ func (w *walker) normalizePath(path string, info fs.FileInfo) (normPath string, tempPath := fs.TempNameWithPrefix(normPath, "") if err = w.Filesystem.Rename(path, tempPath); err != nil { - l.Infof(`Error during normalizing UTF8 encoding of file "%s" (renamed to "%s"): %v`, path, tempPath, err) - return "", true + return "", err } if err = w.Filesystem.Rename(tempPath, normPath); err != nil { // I don't ever expect this to happen, but if it does, we should probably tell our caller that the normalized // path is the temp path: that way at least the user's data still gets synced. l.Warnf(`Error renaming "%s" to "%s" while normalizating UTF8 encoding: %v. You will want to rename this file back manually`, tempPath, normPath, err) - return tempPath, false + return tempPath, nil } - } else { - // There is something already in the way at the normalized - // file name. - l.Infof(`File "%s" path has UTF8 encoding conflict with another file; ignoring.`, path) - return "", true + return normPath, nil } - - return normPath, false + // There is something already in the way at the normalized + // file name. + return "", errUTF8Conflict } // updateFileInfo updates walker specific members of protocol.FileInfo that do not depend on type @@ -522,6 +534,16 @@ func (w *walker) updateFileInfo(file, curFile protocol.FileInfo) protocol.FileIn file.LocalFlags = w.LocalFlags return file } +func (w *walker) handleError(ctx context.Context, context, path string, err error, finishedChan chan<- ScanResult) { + l.Infof("Scanner (folder %s, file %q): %s: %v", w.Folder, path, context, err) + select { + case finishedChan <- ScanResult{ + Err: fmt.Errorf("%s: %s", context, err.Error()), + Path: path, + }: + case <-ctx.Done(): + } +} // A byteCounter gets bytes added to it via Update() and then provides the // Total() and one minute moving average Rate() in bytes per second. diff --git a/lib/scanner/walk_test.go b/lib/scanner/walk_test.go index 447d73655..c845fe6ee 100644 --- a/lib/scanner/walk_test.go +++ b/lib/scanner/walk_test.go @@ -74,7 +74,10 @@ func TestWalkSub(t *testing.T) { }) var files []protocol.FileInfo for f := range fchan { - files = append(files, f) + if f.Err != nil { + t.Errorf("Error while scanning %v: %v", f.Err, f.Path) + } + files = append(files, f.File) } // The directory contains two files, where one is ignored from a higher @@ -107,7 +110,10 @@ func TestWalk(t *testing.T) { var tmp []protocol.FileInfo for f := range fchan { - tmp = append(tmp, f) + if f.Err != nil { + t.Errorf("Error while scanning %v: %v", f.Err, f.Path) + } + tmp = append(tmp, f.File) } sort.Sort(fileList(tmp)) files := fileList(tmp).testfiles() @@ -246,8 +252,9 @@ func TestNormalization(t *testing.T) { func TestIssue1507(t *testing.T) { w := &walker{} - c := make(chan protocol.FileInfo, 100) - fn := w.walkAndHashFiles(context.TODO(), c, c) + h := make(chan protocol.FileInfo, 100) + f := make(chan ScanResult, 100) + fn := w.walkAndHashFiles(context.TODO(), h, f) fn("", nil, protocol.ErrClosed) } @@ -471,7 +478,9 @@ func walkDir(fs fs.Filesystem, dir string, cfiler CurrentFiler, matcher *ignore. var tmp []protocol.FileInfo for f := range fchan { - tmp = append(tmp, f) + if f.Err == nil { + tmp = append(tmp, f.File) + } } sort.Sort(fileList(tmp)) @@ -580,7 +589,11 @@ func TestStopWalk(t *testing.T) { dirs := 0 files := 0 for { - f := <-fchan + res := <-fchan + if res.Err != nil { + t.Errorf("Error while scanning %v: %v", res.Err, res.Path) + } + f := res.File t.Log("Scanned", f) if f.IsDirectory() { if len(f.Name) == 0 || f.Permissions == 0 { @@ -710,7 +723,10 @@ func TestIssue4841(t *testing.T) { var files []protocol.FileInfo for f := range fchan { - files = append(files, f) + if f.Err != nil { + t.Errorf("Error while scanning %v: %v", f.Err, f.Path) + } + files = append(files, f.File) } sort.Sort(fileList(files))