diff --git a/lib/ignore/cache.go b/lib/ignore/cache.go index 1d1af9939..64ee92a8c 100644 --- a/lib/ignore/cache.go +++ b/lib/ignore/cache.go @@ -8,6 +8,12 @@ package ignore import "time" +type nower interface { + Now() time.Time +} + +var clock = nower(defaultClock{}) + type cache struct { patterns []Pattern entries map[string]cacheEntry @@ -27,7 +33,7 @@ func newCache(patterns []Pattern) *cache { func (c *cache) clean(d time.Duration) { for k, v := range c.entries { - if time.Since(v.access) > d { + if clock.Now().Sub(v.access) > d { delete(c.entries, k) } } @@ -36,7 +42,7 @@ func (c *cache) clean(d time.Duration) { func (c *cache) get(key string) (Result, bool) { entry, ok := c.entries[key] if ok { - entry.access = time.Now() + entry.access = clock.Now() c.entries[key] = entry } return entry.result, ok @@ -50,3 +56,9 @@ func (c *cache) len() int { l := len(c.entries) return l } + +type defaultClock struct{} + +func (defaultClock) Now() time.Time { + return time.Now() +} diff --git a/lib/ignore/cache_test.go b/lib/ignore/cache_test.go index 2c5366d96..68513203d 100644 --- a/lib/ignore/cache_test.go +++ b/lib/ignore/cache_test.go @@ -12,6 +12,13 @@ import ( ) func TestCache(t *testing.T) { + fc := new(fakeClock) + oldClock := clock + clock = fc + defer func() { + clock = oldClock + }() + c := newCache(nil) res, ok := c.get("nonexistent") @@ -52,11 +59,11 @@ func TestCache(t *testing.T) { // Sleep and access, to get some data for clean - time.Sleep(500 * time.Millisecond) + *fc += 500 // milliseconds c.get("true") - time.Sleep(100 * time.Millisecond) + *fc += 100 // milliseconds // "false" was accessed ~600 ms ago, "true" was accessed ~100 ms ago. // This should clean out "false" but not "true" @@ -75,3 +82,11 @@ func TestCache(t *testing.T) { t.Errorf("item should have been cleaned") } } + +type fakeClock int64 // milliseconds + +func (f *fakeClock) Now() time.Time { + t := time.Unix(int64(*f)/1000, (int64(*f)%1000)*int64(time.Millisecond)) + *f++ + return t +} diff --git a/lib/ignore/ignore.go b/lib/ignore/ignore.go index e92cd7b20..7dd6c6031 100644 --- a/lib/ignore/ignore.go +++ b/lib/ignore/ignore.go @@ -64,24 +64,59 @@ func (r Result) IsCaseFolded() bool { return r&resultFoldCase == resultFoldCase } -type Matcher struct { - lines []string - patterns []Pattern - withCache bool - matches *cache - curHash string - stop chan struct{} - modtimes map[string]time.Time - mut sync.Mutex +// The ChangeDetector is responsible for determining if files have changed +// on disk. It gets told to Remember() files (name and modtime) and will +// then get asked if a file has been Seen() (i.e., Remember() has been +// called on it) and if any of the files have Changed(). To forget all +// files, call Reset(). +type ChangeDetector interface { + Remember(name string, modtime time.Time) + Seen(name string) bool + Changed() bool + Reset() } -func New(withCache bool) *Matcher { - m := &Matcher{ - withCache: withCache, - stop: make(chan struct{}), - mut: sync.NewMutex(), +type Matcher struct { + lines []string + patterns []Pattern + withCache bool + matches *cache + curHash string + stop chan struct{} + changeDetector ChangeDetector + mut sync.Mutex +} + +// An Option can be passed to New() +type Option func(*Matcher) + +// WithCache enables or disables lookup caching. The default is disabled. +func WithCache(v bool) Option { + return func(m *Matcher) { + m.withCache = v } - if withCache { +} + +// WithChangeDetector sets a custom ChangeDetector. The default is to simply +// use the on disk modtime for comparison. +func WithChangeDetector(cd ChangeDetector) Option { + return func(m *Matcher) { + m.changeDetector = cd + } +} + +func New(opts ...Option) *Matcher { + m := &Matcher{ + stop: make(chan struct{}), + mut: sync.NewMutex(), + } + for _, opt := range opts { + opt(m) + } + if m.changeDetector == nil { + m.changeDetector = newModtimeChecker() + } + if m.withCache { go m.clean(2 * time.Hour) } return m @@ -91,7 +126,7 @@ func (m *Matcher) Load(file string) error { m.mut.Lock() defer m.mut.Unlock() - if m.patternsUnchanged(file) { + if m.changeDetector.Seen(file) && !m.changeDetector.Changed() { return nil } @@ -108,9 +143,8 @@ func (m *Matcher) Load(file string) error { return err } - m.modtimes = map[string]time.Time{ - file: info.ModTime(), - } + m.changeDetector.Reset() + m.changeDetector.Remember(file, info.ModTime()) return m.parseLocked(fd, file) } @@ -122,7 +156,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) + lines, patterns, err := parseIgnoreFile(r, file, m.changeDetector) // Error is saved and returned at the end. We process the patterns // (possibly blank) anyway. @@ -142,26 +176,6 @@ func (m *Matcher) parseLocked(r io.Reader, file string) error { return err } -// patternsUnchanged returns true if none of the files making up the loaded -// patterns have changed since last check. -func (m *Matcher) patternsUnchanged(file string) bool { - if _, ok := m.modtimes[file]; !ok { - return false - } - - for filename, modtime := range m.modtimes { - info, err := os.Stat(filename) - if err != nil { - return false - } - if !info.ModTime().Equal(modtime) { - return false - } - } - - return true -} - func (m *Matcher) Match(file string) (result Result) { if m == nil || file == "." { return resultNotMatched @@ -284,8 +298,8 @@ func hashPatterns(patterns []Pattern) string { return fmt.Sprintf("%x", h.Sum(nil)) } -func loadIgnoreFile(file string, modtimes map[string]time.Time) ([]string, []Pattern, error) { - if _, ok := modtimes[file]; ok { +func loadIgnoreFile(file string, cd ChangeDetector) ([]string, []Pattern, error) { + if cd.Seen(file) { return nil, nil, fmt.Errorf("multiple include of ignore file %q", file) } @@ -299,12 +313,13 @@ func loadIgnoreFile(file string, modtimes map[string]time.Time) ([]string, []Pat if err != nil { return nil, nil, err } - modtimes[file] = info.ModTime() - return parseIgnoreFile(fd, file, modtimes) + cd.Remember(file, info.ModTime()) + + return parseIgnoreFile(fd, file, cd) } -func parseIgnoreFile(fd io.Reader, currentFile string, modtimes map[string]time.Time) ([]string, []Pattern, error) { +func parseIgnoreFile(fd io.Reader, currentFile string, cd ChangeDetector) ([]string, []Pattern, error) { var lines []string var patterns []Pattern @@ -371,7 +386,7 @@ 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) + includeLines, includePatterns, err := loadIgnoreFile(includeFile, cd) if err != nil { return fmt.Errorf("include of %q: %v", includeRel, err) } @@ -466,3 +481,41 @@ func WriteIgnores(path string, content []string) error { return nil } + +// modtimeChecker is the default implementation of ChangeDetector +type modtimeChecker struct { + modtimes map[string]time.Time +} + +func newModtimeChecker() *modtimeChecker { + return &modtimeChecker{ + modtimes: map[string]time.Time{}, + } +} + +func (c *modtimeChecker) Remember(name string, modtime time.Time) { + c.modtimes[name] = modtime +} + +func (c *modtimeChecker) Seen(name string) bool { + _, ok := c.modtimes[name] + return ok +} + +func (c *modtimeChecker) Reset() { + c.modtimes = map[string]time.Time{} +} + +func (c *modtimeChecker) Changed() bool { + for name, modtime := range c.modtimes { + info, err := os.Stat(name) + if err != nil { + return true + } + if !info.ModTime().Equal(modtime) { + return true + } + } + + return false +} diff --git a/lib/ignore/ignore_test.go b/lib/ignore/ignore_test.go index e744e849f..50cf6d06d 100644 --- a/lib/ignore/ignore_test.go +++ b/lib/ignore/ignore_test.go @@ -18,7 +18,7 @@ import ( ) func TestIgnore(t *testing.T) { - pats := New(true) + pats := New(WithCache(true)) err := pats.Load("testdata/.stignore") if err != nil { t.Fatal(err) @@ -68,7 +68,7 @@ func TestExcludes(t *testing.T) { i*2 !ign2 ` - pats := New(true) + pats := New(WithCache(true)) err := pats.Parse(bytes.NewBufferString(stignore), ".stignore") if err != nil { t.Fatal(err) @@ -113,7 +113,7 @@ func TestFlagOrder(t *testing.T) { (?i)(?d)(?d)!ign9 (?d)(?d)!ign10 ` - pats := New(true) + pats := New(WithCache(true)) err := pats.Parse(bytes.NewBufferString(stignore), ".stignore") if err != nil { t.Fatal(err) @@ -148,7 +148,7 @@ func TestDeletables(t *testing.T) { ign7 (?i)ign8 ` - pats := New(true) + pats := New(WithCache(true)) err := pats.Parse(bytes.NewBufferString(stignore), ".stignore") if err != nil { t.Fatal(err) @@ -187,7 +187,7 @@ func TestBadPatterns(t *testing.T) { } for _, pat := range badPatterns { - err := New(true).Parse(bytes.NewBufferString(pat), ".stignore") + err := New(WithCache(true)).Parse(bytes.NewBufferString(pat), ".stignore") if err == nil { t.Errorf("No error for pattern %q", pat) } @@ -195,7 +195,7 @@ func TestBadPatterns(t *testing.T) { } func TestCaseSensitivity(t *testing.T) { - ign := New(true) + ign := New(WithCache(true)) err := ign.Parse(bytes.NewBufferString("test"), ".stignore") if err != nil { t.Error(err) @@ -247,7 +247,7 @@ func TestCaching(t *testing.T) { fd2.WriteString("/y/\n") - pats := New(true) + pats := New(WithCache(true)) err = pats.Load(fd1.Name()) if err != nil { t.Fatal(err) @@ -354,7 +354,7 @@ func TestCommentsAndBlankLines(t *testing.T) { ` - pats := New(true) + pats := New(WithCache(true)) err := pats.Parse(bytes.NewBufferString(stignore), ".stignore") if err != nil { t.Error(err) @@ -382,7 +382,7 @@ flamingo *.crow *.crow ` - pats := New(false) + pats := New() err := pats.Parse(bytes.NewBufferString(stignore), ".stignore") if err != nil { b.Error(err) @@ -424,7 +424,7 @@ flamingo } // Load the patterns - pats := New(true) + pats := New(WithCache(true)) err = pats.Load(fd.Name()) if err != nil { b.Fatal(err) @@ -460,7 +460,7 @@ func TestCacheReload(t *testing.T) { t.Fatal(err) } - pats := New(true) + pats := New(WithCache(true)) err = pats.Load(fd.Name()) if err != nil { t.Fatal(err) @@ -515,7 +515,7 @@ func TestCacheReload(t *testing.T) { } func TestHash(t *testing.T) { - p1 := New(true) + p1 := New(WithCache(true)) err := p1.Load("testdata/.stignore") if err != nil { t.Fatal(err) @@ -531,7 +531,7 @@ func TestHash(t *testing.T) { /ffile lost+found ` - p2 := New(true) + p2 := New(WithCache(true)) err = p2.Parse(bytes.NewBufferString(stignore), ".stignore") if err != nil { t.Fatal(err) @@ -546,7 +546,7 @@ func TestHash(t *testing.T) { /ffile lost+found ` - p3 := New(true) + p3 := New(WithCache(true)) err = p3.Parse(bytes.NewBufferString(stignore), ".stignore") if err != nil { t.Fatal(err) @@ -570,7 +570,7 @@ func TestHash(t *testing.T) { } func TestHashOfEmpty(t *testing.T) { - p1 := New(true) + p1 := New(WithCache(true)) err := p1.Load("testdata/.stignore") if err != nil { t.Fatal(err) @@ -608,7 +608,7 @@ func TestWindowsPatterns(t *testing.T) { a/b c\d ` - pats := New(true) + pats := New(WithCache(true)) err := pats.Parse(bytes.NewBufferString(stignore), ".stignore") if err != nil { t.Fatal(err) @@ -633,7 +633,7 @@ func TestAutomaticCaseInsensitivity(t *testing.T) { A/B c/d ` - pats := New(true) + pats := New(WithCache(true)) err := pats.Parse(bytes.NewBufferString(stignore), ".stignore") if err != nil { t.Fatal(err) @@ -652,7 +652,7 @@ func TestCommas(t *testing.T) { foo,bar.txt {baz,quux}.txt ` - pats := New(true) + pats := New(WithCache(true)) err := pats.Parse(bytes.NewBufferString(stignore), ".stignore") if err != nil { t.Fatal(err) @@ -683,7 +683,7 @@ func TestIssue3164(t *testing.T) { (?d)(?i)/foo (?d)(?i)**/bar ` - pats := New(true) + pats := New(WithCache(true)) err := pats.Parse(bytes.NewBufferString(stignore), ".stignore") if err != nil { t.Fatal(err) @@ -719,7 +719,7 @@ func TestIssue3174(t *testing.T) { stignore := ` *รค* ` - pats := New(true) + pats := New(WithCache(true)) err := pats.Parse(bytes.NewBufferString(stignore), ".stignore") if err != nil { t.Fatal(err) @@ -734,7 +734,7 @@ func TestIssue3639(t *testing.T) { stignore := ` foo/ ` - pats := New(true) + pats := New(WithCache(true)) err := pats.Parse(bytes.NewBufferString(stignore), ".stignore") if err != nil { t.Fatal(err) @@ -767,7 +767,7 @@ func TestIssue3674(t *testing.T) { {"as/dc", true}, } - pats := New(true) + pats := New(WithCache(true)) err := pats.Parse(bytes.NewBufferString(stignore), ".stignore") if err != nil { t.Fatal(err) @@ -799,7 +799,7 @@ func TestGobwasGlobIssue18(t *testing.T) { {"bbaa", false}, } - pats := New(true) + pats := New(WithCache(true)) err := pats.Parse(bytes.NewBufferString(stignore), ".stignore") if err != nil { t.Fatal(err) @@ -859,7 +859,7 @@ func TestRoot(t *testing.T) { {"b", true}, } - pats := New(true) + pats := New(WithCache(true)) err := pats.Parse(bytes.NewBufferString(stignore), ".stignore") if err != nil { t.Fatal(err) diff --git a/lib/model/model.go b/lib/model/model.go index 73823dbd4..c20f1f1e4 100644 --- a/lib/model/model.go +++ b/lib/model/model.go @@ -314,7 +314,7 @@ func (m *Model) addFolderLocked(cfg config.FolderConfiguration) { m.deviceFolders[device.DeviceID] = append(m.deviceFolders[device.DeviceID], cfg.ID) } - ignores := ignore.New(m.cacheIgnoredFiles) + ignores := ignore.New(ignore.WithCache(m.cacheIgnoredFiles)) if err := ignores.Load(filepath.Join(cfg.Path(), ".stignore")); err != nil && !os.IsNotExist(err) { l.Warnln("Loading ignores:", err) } @@ -1260,7 +1260,7 @@ func (m *Model) GetIgnores(folder string) ([]string, []string, error) { } if cfg, ok := m.cfg.Folders()[folder]; ok { - matcher := ignore.New(false) + matcher := ignore.New() path := filepath.Join(cfg.Path(), ".stignore") if err := matcher.Load(path); err != nil { return nil, nil, err diff --git a/lib/model/model_test.go b/lib/model/model_test.go index 1c44ff172..ca2aee237 100644 --- a/lib/model/model_test.go +++ b/lib/model/model_test.go @@ -953,14 +953,6 @@ func changeIgnores(t *testing.T, m *Model, expected []string) { ignores = append(ignores, "pox") - if runtime.GOOS == "darwin" { - // Mac has seconds-only timestamp precision, which tricks the ignore - // system into thinking the file has not changed. Work around it in - // an ugly way... - time.Sleep(time.Second) - } else { - time.Sleep(time.Millisecond) - } err = m.SetIgnores("default", ignores) if err != nil { t.Error(err) @@ -1011,6 +1003,14 @@ func TestIgnores(t *testing.T) { m.AddFolder(defaultFolderConfig) m.StartFolder("default") + // Reach in and update the ignore matcher to one that always does + // reloads when asked to, instead of checking file mtimes. This is + // because we will be changing the files on disk often enough that the + // mtimes will be unreliable to determine change status. + m.fmut.Lock() + m.folderIgnores["default"] = ignore.New(ignore.WithCache(true), ignore.WithChangeDetector(newAlwaysChanged())) + m.fmut.Unlock() + // Make sure the initial scan has finished (ScanFolders is blocking) m.ScanFolders() @@ -1843,7 +1843,7 @@ func TestIssue3164(t *testing.T) { f := protocol.FileInfo{ Name: "issue3164", } - m := ignore.New(false) + m := ignore.New() if err := m.Parse(bytes.NewBufferString("(?d)oktodelete"), ""); err != nil { t.Fatal(err) } @@ -2490,3 +2490,31 @@ func (fakeAddr) Network() string { func (fakeAddr) String() string { return "address" } + +// alwaysChanges is an ignore.ChangeDetector that always returns true on Changed() +type alwaysChanged struct { + seen map[string]struct{} +} + +func newAlwaysChanged() *alwaysChanged { + return &alwaysChanged{ + seen: make(map[string]struct{}), + } +} + +func (c *alwaysChanged) Remember(name string, _ time.Time) { + c.seen[name] = struct{}{} +} + +func (c *alwaysChanged) Reset() { + c.seen = make(map[string]struct{}) +} + +func (c *alwaysChanged) Seen(name string) bool { + _, ok := c.seen[name] + return ok +} + +func (c *alwaysChanged) Changed() bool { + return true +} diff --git a/lib/scanner/walk_test.go b/lib/scanner/walk_test.go index 725113ee5..852d29615 100644 --- a/lib/scanner/walk_test.go +++ b/lib/scanner/walk_test.go @@ -54,7 +54,7 @@ func init() { } func TestWalkSub(t *testing.T) { - ignores := ignore.New(false) + ignores := ignore.New() err := ignores.Load("testdata/.stignore") if err != nil { t.Fatal(err) @@ -90,7 +90,7 @@ func TestWalkSub(t *testing.T) { } func TestWalk(t *testing.T) { - ignores := ignore.New(false) + ignores := ignore.New() err := ignores.Load("testdata/.stignore") if err != nil { t.Fatal(err)