From bcfd18ceb14641e8fcef973d6499bcad9d03effb Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Sat, 15 Apr 2017 09:38:23 +0200 Subject: [PATCH] Revert "lib/model, gui: Allow creating and editing ignores of paused folders (fixes #3608)" This reverts commit 25b314f5f1448355dc9779ad260dcaa843dcd40c. --- cmd/syncthing/gui.go | 4 +- gui/default/assets/lang/lang-en.json | 2 - .../syncthing/core/syncthingController.js | 86 +++++++++---------- .../syncthing/folder/editFolderModalView.html | 2 +- .../folder/editIgnoresModalView.html | 7 +- lib/ignore/ignore.go | 52 +++-------- lib/model/folder.go | 9 +- lib/model/model.go | 78 ++++++++++------- lib/model/model_test.go | 65 ++++++-------- lib/model/rofolder.go | 16 ++-- lib/model/rwfolder.go | 15 ++-- lib/model/rwfolder_test.go | 14 +-- 12 files changed, 147 insertions(+), 203 deletions(-) diff --git a/cmd/syncthing/gui.go b/cmd/syncthing/gui.go index ca431e6be..b276fe79e 100644 --- a/cmd/syncthing/gui.go +++ b/cmd/syncthing/gui.go @@ -969,9 +969,7 @@ func (s *apiService) getRandomString(w http.ResponseWriter, r *http.Request) { func (s *apiService) getDBIgnores(w http.ResponseWriter, r *http.Request) { qs := r.URL.Query() - folder := qs.Get("folder") - - ignores, patterns, err := s.model.GetIgnores(folder) + ignores, patterns, err := s.model.GetIgnores(qs.Get("folder")) if err != nil { http.Error(w, err.Error(), 500) return diff --git a/gui/default/assets/lang/lang-en.json b/gui/default/assets/lang/lang-en.json index 24d541120..26db4838a 100644 --- a/gui/default/assets/lang/lang-en.json +++ b/gui/default/assets/lang/lang-en.json @@ -43,7 +43,6 @@ "Copied from original": "Copied from original", "Copyright © 2014-2016 the following Contributors:": "Copyright © 2014-2016 the following Contributors:", "Copyright © 2014-2017 the following Contributors:": "Copyright © 2014-2017 the following Contributors:", - "Creating ignore patterns, overwriting an existing file at {%path%}.": "Creating ignore patterns, overwriting an existing file at {{path}}.", "Danger!": "Danger!", "Deleted": "Deleted", "Device": "Device", @@ -64,7 +63,6 @@ "Edit Device": "Edit Device", "Edit Folder": "Edit Folder", "Editing": "Editing", - "Editing {%path%}.": "Editing {{path}}.", "Enable NAT traversal": "Enable NAT traversal", "Enable Relaying": "Enable Relaying", "Enter comma separated (\"tcp://ip:port\", \"tcp://host:port\") addresses or \"dynamic\" to perform automatic discovery of the address.": "Enter comma separated (\"tcp://ip:port\", \"tcp://host:port\") addresses or \"dynamic\" to perform automatic discovery of the address.", diff --git a/gui/default/syncthing/core/syncthingController.js b/gui/default/syncthing/core/syncthingController.js index a418e2d0c..f1717681e 100755 --- a/gui/default/syncthing/core/syncthingController.js +++ b/gui/default/syncthing/core/syncthingController.js @@ -58,24 +58,6 @@ angular.module('syncthing.core') $scope.metricRates = (window.localStorage["metricRates"] == "true"); } catch (exception) { } - $scope.folderDefaults = { - selectedDevices: {}, - type: "readwrite", - rescanIntervalS: 60, - minDiskFreePct: 1, - maxConflicts: 10, - fsync: true, - order: "random", - fileVersioningSelector: "none", - trashcanClean: 0, - simpleKeep: 5, - staggeredMaxAge: 365, - staggeredCleanInterval: 3600, - staggeredVersionsPath: "", - externalCommand: "", - autoNormalize: true - }; - $scope.localStateTotal = { bytes: 0, files: 0 @@ -1411,9 +1393,24 @@ angular.module('syncthing.core') }; $scope.addFolder = function () { - $scope.currentFolder = angular.copy($scope.folderDefaults); + $scope.currentFolder = { + selectedDevices: {}, + type: "readwrite", + rescanIntervalS: 60, + minDiskFreePct: 1, + maxConflicts: 10, + fsync: true, + order: "random", + fileVersioningSelector: "none", + trashcanClean: 0, + simpleKeep: 5, + staggeredMaxAge: 365, + staggeredCleanInterval: 3600, + staggeredVersionsPath: "", + externalCommand: "", + autoNormalize: true + }; $scope.editingExisting = false; - $('#editIgnores textarea').val(""); $scope.folderEditor.$setPristine(); $http.get(urlbase + '/svc/random/string?length=10').success(function (data) { $scope.currentFolder.id = (data.random.substr(0, 5) + '-' + data.random.substr(5, 5)).toLowerCase(); @@ -1423,11 +1420,26 @@ angular.module('syncthing.core') $scope.addFolderAndShare = function (folder, folderLabel, device) { $scope.dismissFolderRejection(folder, device); - $scope.currentFolder = angular.copy($scope.folderDefaults); - $scope.currentFolder.id = folder; - $scope.currentFolder.label = folderLabel; - $scope.currentFolder.viewFlags = { - importFromOtherDevice: true + $scope.currentFolder = { + id: folder, + label: folderLabel, + selectedDevices: {}, + rescanIntervalS: 60, + minDiskFreePct: 1, + maxConflicts: 10, + fsync: true, + order: "random", + fileVersioningSelector: "none", + trashcanClean: 0, + simpleKeep: 5, + staggeredMaxAge: 365, + staggeredCleanInterval: 3600, + staggeredVersionsPath: "", + externalCommand: "", + autoNormalize: true, + viewFlags: { + importFromOtherDevice: true + } }; $scope.currentFolder.selectedDevices[device] = true; @@ -1504,20 +1516,10 @@ angular.module('syncthing.core') delete folderCfg.versioning; } - var ignores = $('#editIgnores textarea').val().trim(); - if (!$scope.editingExisting && ignores) { - folderCfg.paused = true; - }; - $scope.folders[folderCfg.id] = folderCfg; $scope.config.folders = folderList($scope.folders); $scope.saveConfig(); - - if (!$scope.editingExisting && ignores) { - $scope.saveIgnores(); - $scope.setFolderPause(folderCfg.id, false); - }; }; $scope.dismissFolderRejection = function (folder, device) { @@ -1591,21 +1593,11 @@ angular.module('syncthing.core') }); }; - $scope.editIgnoresOnAddingFolder = function () { - if ($scope.editingExisting) { + $scope.saveIgnores = function () { + if (!$scope.editingExisting) { return; } - if ($scope.currentFolder.path.endsWith($scope.system.pathSeparator)) { - $scope.currentFolder.path = $scope.currentFolder.path.slice(0, -1); - }; - $('#editIgnores').modal().one('shown.bs.modal', function () { - textArea.focus(); - }); - }; - - - $scope.saveIgnores = function () { $http.post(urlbase + '/db/ignores?folder=' + encodeURIComponent($scope.currentFolder.id), { ignore: $('#editIgnores textarea').val().split('\n') }); diff --git a/gui/default/syncthing/folder/editFolderModalView.html b/gui/default/syncthing/folder/editFolderModalView.html index 359f90e17..2eb1faee2 100644 --- a/gui/default/syncthing/folder/editFolderModalView.html +++ b/gui/default/syncthing/folder/editFolderModalView.html @@ -184,7 +184,7 @@ - - + \ No newline at end of file diff --git a/lib/ignore/ignore.go b/lib/ignore/ignore.go index 20338ea9a..81bc1a880 100644 --- a/lib/ignore/ignore.go +++ b/lib/ignore/ignore.go @@ -19,7 +19,6 @@ import ( "time" "github.com/gobwas/glob" - "github.com/syncthing/syncthing/lib/osutil" "github.com/syncthing/syncthing/lib/sync" ) @@ -65,7 +64,6 @@ func (r Result) IsCaseFolded() bool { } type Matcher struct { - lines []string patterns []Pattern withCache bool matches *cache @@ -122,7 +120,7 @@ func (m *Matcher) Parse(r io.Reader, file string) error { } func (m *Matcher) parseLocked(r io.Reader, file string) error { - lines, patterns, err := parseIgnoreFile(r, file, m.modtimes) + patterns, err := parseIgnoreFile(r, file, m.modtimes) // Error is saved and returned at the end. We process the patterns // (possibly blank) anyway. @@ -133,7 +131,6 @@ func (m *Matcher) parseLocked(r io.Reader, file string) error { } m.curHash = newHash - m.lines = lines m.patterns = patterns if m.withCache { m.matches = newCache(patterns) @@ -209,13 +206,6 @@ func (m *Matcher) Match(file string) (result Result) { return resultNotMatched } -// Lines return a list of the unprocessed lines in .stignore at last load -func (m *Matcher) Lines() []string { - m.mut.Lock() - defer m.mut.Unlock() - return m.lines -} - // Patterns return a list of the loaded patterns, as they've been parsed func (m *Matcher) Patterns() []string { if m == nil { @@ -284,28 +274,27 @@ func hashPatterns(patterns []Pattern) string { return fmt.Sprintf("%x", h.Sum(nil)) } -func loadIgnoreFile(file string, modtimes map[string]time.Time) ([]string, []Pattern, error) { +func loadIgnoreFile(file string, modtimes map[string]time.Time) ([]Pattern, error) { if _, ok := modtimes[file]; ok { - return nil, nil, fmt.Errorf("multiple include of ignore file %q", file) + return nil, fmt.Errorf("Multiple include of ignore file %q", file) } fd, err := os.Open(file) if err != nil { - return nil, nil, err + return nil, err } defer fd.Close() info, err := fd.Stat() if err != nil { - return nil, nil, err + return nil, err } modtimes[file] = info.ModTime() return parseIgnoreFile(fd, file, modtimes) } -func parseIgnoreFile(fd io.Reader, currentFile string, modtimes map[string]time.Time) ([]string, []Pattern, error) { - var lines []string +func parseIgnoreFile(fd io.Reader, currentFile string, modtimes map[string]time.Time) ([]Pattern, error) { var patterns []Pattern defaultResult := resultInclude @@ -371,12 +360,11 @@ func parseIgnoreFile(fd io.Reader, currentFile string, modtimes map[string]time. } else if strings.HasPrefix(line, "#include ") { includeRel := line[len("#include "):] includeFile := filepath.Join(filepath.Dir(currentFile), includeRel) - includeLines, includePatterns, err := loadIgnoreFile(includeFile, modtimes) + includes, err := loadIgnoreFile(includeFile, modtimes) if err != nil { return fmt.Errorf("include of %q: %v", includeRel, err) } - lines = append(lines, includeLines...) - patterns = append(patterns, includePatterns...) + patterns = append(patterns, includes...) } else { // Path name or pattern, add it so it matches files both in // current directory and subdirs. @@ -401,7 +389,6 @@ func parseIgnoreFile(fd io.Reader, currentFile string, modtimes map[string]time. var err error for scanner.Scan() { line := strings.TrimSpace(scanner.Text()) - lines = append(lines, line) switch { case line == "": continue @@ -424,11 +411,11 @@ func parseIgnoreFile(fd io.Reader, currentFile string, modtimes map[string]time. } } if err != nil { - return nil, nil, err + return nil, err } } - return lines, patterns, nil + return patterns, nil } // IsInternal returns true if the file, as a path relative to the folder @@ -447,22 +434,3 @@ func IsInternal(file string) bool { } return false } - -// WriteIgnores is a convenience function to avoid code duplication -func WriteIgnores(path string, content []string) error { - fd, err := osutil.CreateAtomic(path) - if err != nil { - return err - } - - for _, line := range content { - fmt.Fprintln(fd, line) - } - - if err := fd.Close(); err != nil { - return err - } - osutil.HideFile(path) - - return nil -} diff --git a/lib/model/folder.go b/lib/model/folder.go index 49e20b873..927519f46 100644 --- a/lib/model/folder.go +++ b/lib/model/folder.go @@ -10,11 +10,9 @@ import "time" type folder struct { stateTracker - - scan folderScanner - model *Model - stop chan struct{} - initialScanCompleted chan struct{} + scan folderScanner + model *Model + stop chan struct{} } func (f *folder) IndexUpdated() { @@ -25,7 +23,6 @@ func (f *folder) DelayScan(next time.Duration) { } func (f *folder) Scan(subdirs []string) error { - <-f.initialScanCompleted return f.scan.Scan(subdirs) } func (f *folder) Stop() { diff --git a/lib/model/model.go b/lib/model/model.go index f09401f9e..e38c04791 100644 --- a/lib/model/model.go +++ b/lib/model/model.go @@ -7,6 +7,7 @@ package model import ( + "bufio" "crypto/tls" "encoding/json" "errors" @@ -1251,51 +1252,66 @@ func (m *Model) ConnectedTo(deviceID protocol.DeviceID) bool { } func (m *Model) GetIgnores(folder string) ([]string, []string, error) { + var lines []string + m.fmut.RLock() cfg, ok := m.folderCfgs[folder] m.fmut.RUnlock() - if ok { - if !cfg.HasMarker() { - return nil, nil, fmt.Errorf("Folder %s stopped", folder) - } - - m.fmut.RLock() - ignores := m.folderIgnores[folder] - m.fmut.RUnlock() - - return ignores.Lines(), ignores.Patterns(), nil + if !ok { + return lines, nil, fmt.Errorf("Folder %s does not exist", folder) } - if cfg, ok := m.cfg.Folders()[folder]; ok { - matcher := ignore.New(false) - path := filepath.Join(cfg.Path(), ".stignore") - if err := matcher.Load(path); err != nil { - return nil, nil, err - } - return matcher.Lines(), matcher.Patterns(), nil + if !cfg.HasMarker() { + return lines, nil, fmt.Errorf("Folder %s stopped", folder) } - return nil, nil, fmt.Errorf("Folder %s does not exist", folder) + fd, err := os.Open(filepath.Join(cfg.Path(), ".stignore")) + if err != nil { + if os.IsNotExist(err) { + return lines, nil, nil + } + l.Warnln("Loading .stignore:", err) + return lines, nil, err + } + defer fd.Close() + + scanner := bufio.NewScanner(fd) + for scanner.Scan() { + lines = append(lines, strings.TrimSpace(scanner.Text())) + } + + m.fmut.RLock() + patterns := m.folderIgnores[folder].Patterns() + m.fmut.RUnlock() + + return lines, patterns, nil } func (m *Model) SetIgnores(folder string, content []string) error { - cfg, ok := m.cfg.Folders()[folder] + cfg, ok := m.folderCfgs[folder] if !ok { return fmt.Errorf("Folder %s does not exist", folder) } - if err := ignore.WriteIgnores(filepath.Join(cfg.Path(), ".stignore"), content); err != nil { + path := filepath.Join(cfg.Path(), ".stignore") + + fd, err := osutil.CreateAtomic(path) + if err != nil { l.Warnln("Saving .stignore:", err) return err } - m.fmut.RLock() - runner, ok := m.folderRunners[folder] - m.fmut.RUnlock() - if ok { - return runner.Scan(nil) + for _, line := range content { + fmt.Fprintln(fd, line) } - return nil + + if err := fd.Close(); err != nil { + l.Warnln("Saving .stignore:", err) + return err + } + osutil.HideFile(path) + + return m.ScanFolder(folder) } // OnHello is called when an device connects to us. @@ -2379,13 +2395,9 @@ func (m *Model) CommitConfiguration(from, to config.Configuration) bool { for folderID, cfg := range toFolders { if _, ok := fromFolders[folderID]; !ok { // A folder was added. - if cfg.Paused { - l.Infoln(m, "Paused folder", cfg.Description()) - } else { - l.Infoln(m, "Adding folder", cfg.Description()) - m.AddFolder(cfg) - m.StartFolder(folderID) - } + l.Debugln(m, "adding folder", folderID) + m.AddFolder(cfg) + m.StartFolder(folderID) } } diff --git a/lib/model/model_test.go b/lib/model/model_test.go index 839ab44f4..1a4739f02 100644 --- a/lib/model/model_test.go +++ b/lib/model/model_test.go @@ -927,7 +927,7 @@ func TestIntroducer(t *testing.T) { } } -func changeIgnores(t *testing.T, m *Model, expected []string) { +func TestIgnores(t *testing.T) { arrEqual := func(a, b []string) bool { if len(a) != len(b) { return false @@ -941,6 +941,22 @@ func changeIgnores(t *testing.T, m *Model, expected []string) { return true } + // Assure a clean start state + ioutil.WriteFile("testdata/.stfolder", nil, 0644) + ioutil.WriteFile("testdata/.stignore", []byte(".*\nquux\n"), 0644) + + db := db.OpenMemory() + m := NewModel(defaultConfig, protocol.LocalDeviceID, "device", "syncthing", "dev", db, nil) + m.AddFolder(defaultFolderConfig) + m.StartFolder("default") + m.ServeBackground() + defer m.Stop() + + expected := []string{ + ".*", + "quux", + } + ignores, _, err := m.GetIgnores("default") if err != nil { t.Error(err) @@ -989,34 +1005,8 @@ func changeIgnores(t *testing.T, m *Model, expected []string) { if !arrEqual(ignores, expected) { t.Errorf("Incorrect ignores: %v != %v", ignores, expected) } -} -func TestIgnores(t *testing.T) { - // Assure a clean start state - ioutil.WriteFile("testdata/.stfolder", nil, 0644) - ioutil.WriteFile("testdata/.stignore", []byte(".*\nquux\n"), 0644) - - db := db.OpenMemory() - m := NewModel(defaultConfig, protocol.LocalDeviceID, "device", "syncthing", "dev", db, nil) - m.ServeBackground() - defer m.Stop() - - // m.cfg.SetFolder is not usable as it is non-blocking, and there is no - // way to know when the folder is actually added. - m.AddFolder(defaultFolderConfig) - m.StartFolder("default") - - // Make sure the initial scan has finished (ScanFolders is blocking) - m.ScanFolders() - - expected := []string{ - ".*", - "quux", - } - - changeIgnores(t, m, expected) - - _, _, err := m.GetIgnores("doesnotexist") + _, _, err = m.GetIgnores("doesnotexist") if err == nil { t.Error("No error") } @@ -1032,16 +1022,6 @@ func TestIgnores(t *testing.T) { if err == nil { t.Error("No error") } - - // Repeat tests with paused folder - pausedDefaultFolderConfig := defaultFolderConfig - pausedDefaultFolderConfig.Paused = true - - m.RestartFolder(pausedDefaultFolderConfig) - // Here folder initialization is not an issue as a paused folder isn't - // added to the model and thus there is no initial scan happening. - - changeIgnores(t, m, expected) } func TestROScanRecovery(t *testing.T) { @@ -1789,8 +1769,13 @@ func TestIssue3028(t *testing.T) { m.StartFolder("default") m.ServeBackground() - // Make sure the initial scan has finished (ScanFolders is blocking) - m.ScanFolders() + // Ugly hack for testing: reach into the model for the SendReceiveFolder and wait + // for it to complete the initial scan. The risk is that it otherwise + // runs during our modifications and screws up the test. + m.fmut.RLock() + folder := m.folderRunners["default"].(*sendReceiveFolder) + m.fmut.RUnlock() + <-folder.initialScanCompleted // Get a count of how many files are there now diff --git a/lib/model/rofolder.go b/lib/model/rofolder.go index 2cbb99381..9f50ece35 100644 --- a/lib/model/rofolder.go +++ b/lib/model/rofolder.go @@ -26,11 +26,10 @@ type sendOnlyFolder struct { func newSendOnlyFolder(model *Model, cfg config.FolderConfiguration, _ versioner.Versioner, _ *fs.MtimeFS) service { return &sendOnlyFolder{ folder: folder{ - stateTracker: newStateTracker(cfg.ID), - scan: newFolderScanner(cfg), - stop: make(chan struct{}), - model: model, - initialScanCompleted: make(chan struct{}), + stateTracker: newStateTracker(cfg.ID), + scan: newFolderScanner(cfg), + stop: make(chan struct{}), + model: model, }, FolderConfiguration: cfg, } @@ -44,6 +43,7 @@ func (f *sendOnlyFolder) Serve() { f.scan.timer.Stop() }() + initialScanCompleted := false for { select { case <-f.stop: @@ -68,11 +68,9 @@ func (f *sendOnlyFolder) Serve() { continue } - select { - case <-f.initialScanCompleted: - default: + if !initialScanCompleted { l.Infoln("Completed initial scan (ro) of", f.Description()) - close(f.initialScanCompleted) + initialScanCompleted = true } if f.scan.HasNoInterval() { diff --git a/lib/model/rwfolder.go b/lib/model/rwfolder.go index 6fb073dcc..b6c29a5ca 100644 --- a/lib/model/rwfolder.go +++ b/lib/model/rwfolder.go @@ -96,16 +96,17 @@ type sendReceiveFolder struct { errors map[string]string // path -> error string errorsMut sync.Mutex + + initialScanCompleted chan (struct{}) // exposed for testing } func newSendReceiveFolder(model *Model, cfg config.FolderConfiguration, ver versioner.Versioner, mtimeFS *fs.MtimeFS) service { f := &sendReceiveFolder{ folder: folder{ - stateTracker: newStateTracker(cfg.ID), - scan: newFolderScanner(cfg), - stop: make(chan struct{}), - model: model, - initialScanCompleted: make(chan struct{}), + stateTracker: newStateTracker(cfg.ID), + scan: newFolderScanner(cfg), + stop: make(chan struct{}), + model: model, }, FolderConfiguration: cfg, @@ -118,6 +119,8 @@ func newSendReceiveFolder(model *Model, cfg config.FolderConfiguration, ver vers remoteIndex: make(chan struct{}, 1), // This needs to be 1-buffered so that we queue a notification if we're busy doing a pull when it comes. errorsMut: sync.NewMutex(), + + initialScanCompleted: make(chan struct{}), } f.configureCopiersAndPullers() @@ -1060,7 +1063,7 @@ func (f *sendReceiveFolder) handleFile(file protocol.FileInfo, copyChan chan<- c // sweep is complete. As we do retries, we'll queue the scan // for this file up to ten times, but the last nine of those // scans will be cheap... - go f.Scan([]string{file.Name}) + go f.scan.Scan([]string{file.Name}) return } } diff --git a/lib/model/rwfolder_test.go b/lib/model/rwfolder_test.go index 13a2fa7c4..a3ea49c6d 100644 --- a/lib/model/rwfolder_test.go +++ b/lib/model/rwfolder_test.go @@ -77,12 +77,11 @@ func setUpModel(file protocol.FileInfo) *Model { return model } -func setUpSendReceiveFolder(model *Model) *sendReceiveFolder { - f := &sendReceiveFolder{ +func setUpSendReceiveFolder(model *Model) sendReceiveFolder { + return sendReceiveFolder{ folder: folder{ - stateTracker: newStateTracker("default"), - model: model, - initialScanCompleted: make(chan struct{}), + stateTracker: newStateTracker("default"), + model: model, }, mtimeFS: fs.NewMtimeFS(fs.DefaultFilesystem, db.NewNamespacedKV(model.db, "mtime")), @@ -91,11 +90,6 @@ func setUpSendReceiveFolder(model *Model) *sendReceiveFolder { errors: make(map[string]string), errorsMut: sync.NewMutex(), } - - // Folders are never actually started, so no initial scan will be done - close(f.initialScanCompleted) - - return f } // Layout of the files: (indexes from the above array)