diff --git a/lib/fs/basicfs_watch.go b/lib/fs/basicfs_watch.go index b6d2c2a1b..31a76c537 100644 --- a/lib/fs/basicfs_watch.go +++ b/lib/fs/basicfs_watch.go @@ -29,10 +29,6 @@ func (f *BasicFilesystem) Watch(name string, ignore Matcher, ctx context.Context return nil, err } - absShouldIgnore := func(absPath string) bool { - return ignore.ShouldIgnore(f.unrootedChecked(absPath)) - } - outChan := make(chan Event) backendChan := make(chan notify.EventInfo, backendBuffer) @@ -41,7 +37,15 @@ func (f *BasicFilesystem) Watch(name string, ignore Matcher, ctx context.Context eventMask |= permEventMask } - if err := notify.WatchWithFilter(filepath.Join(absName, "..."), backendChan, absShouldIgnore, eventMask); err != nil { + if ignore.SkipIgnoredDirs() { + absShouldIgnore := func(absPath string) bool { + return ignore.ShouldIgnore(f.unrootedChecked(absPath)) + } + err = notify.WatchWithFilter(filepath.Join(absName, "..."), backendChan, absShouldIgnore, eventMask) + } else { + err = notify.Watch(filepath.Join(absName, "..."), backendChan, eventMask) + } + if err != nil { notify.Stop(backendChan) if reachedMaxUserWatches(err) { err = errors.New("failed to setup inotify handler. Please increase inotify limits, see https://docs.syncthing.net/users/faq.html#inotify-limits") diff --git a/lib/fs/basicfs_watch_test.go b/lib/fs/basicfs_watch_test.go index 7919b679a..41bbf10af 100644 --- a/lib/fs/basicfs_watch_test.go +++ b/lib/fs/basicfs_watch_test.go @@ -41,15 +41,17 @@ func TestMain(m *testing.M) { testFs = NewFilesystem(FilesystemTypeBasic, testDirAbs) backendBuffer = 10 - defer func() { - backendBuffer = 500 - os.RemoveAll(testDir) - }() - os.Exit(m.Run()) + + exitCode := m.Run() + + backendBuffer = 500 + os.RemoveAll(testDir) + + os.Exit(exitCode) } const ( - testDir = "temporary_test_root" + testDir = "testdata" ) var ( @@ -75,7 +77,31 @@ func TestWatchIgnore(t *testing.T) { {name, NonRemove}, } - testScenario(t, name, testCase, expectedEvents, allowedEvents, ignored) + testScenario(t, name, testCase, expectedEvents, allowedEvents, fakeMatcher{ignore: filepath.Join(name, ignored), skipIgnoredDirs: true}) +} + +func TestWatchInclude(t *testing.T) { + name := "include" + + file := "file" + ignored := "ignored" + testFs.MkdirAll(filepath.Join(name, ignored), 0777) + included := filepath.Join(ignored, "included") + + testCase := func() { + createTestFile(name, file) + createTestFile(name, included) + } + + expectedEvents := []Event{ + {file, NonRemove}, + {included, NonRemove}, + } + allowedEvents := []Event{ + {name, NonRemove}, + } + + testScenario(t, name, testCase, expectedEvents, allowedEvents, fakeMatcher{ignore: filepath.Join(name, ignored), include: filepath.Join(name, included)}) } func TestWatchRename(t *testing.T) { @@ -104,7 +130,7 @@ func TestWatchRename(t *testing.T) { // set the "allow others" flag because we might get the create of // "oldfile" initially - testScenario(t, name, testCase, expectedEvents, allowedEvents, "") + testScenario(t, name, testCase, expectedEvents, allowedEvents, fakeMatcher{}) } // TestWatchOutside checks that no changes from outside the folder make it in @@ -120,7 +146,11 @@ func TestWatchOutside(t *testing.T) { go func() { defer func() { if recover() == nil { - t.Fatalf("Watch did not panic on receiving event outside of folder") + select { + case <-ctx.Done(): // timed out + default: + t.Fatalf("Watch did not panic on receiving event outside of folder") + } } cancel() }() @@ -128,6 +158,13 @@ func TestWatchOutside(t *testing.T) { }() backendChan <- fakeEventInfo(filepath.Join(filepath.Dir(testDirAbs), "outside")) + + select { + case <-time.After(10 * time.Second): + cancel() + t.Errorf("Timed out before panicing") + case <-ctx.Done(): + } } func TestWatchSubpath(t *testing.T) { @@ -178,7 +215,7 @@ func TestWatchOverflow(t *testing.T) { } } - testScenario(t, name, testCase, expectedEvents, allowedEvents, "") + testScenario(t, name, testCase, expectedEvents, allowedEvents, fakeMatcher{}) } func TestWatchErrorLinuxInterpretation(t *testing.T) { @@ -283,7 +320,7 @@ func TestWatchIssue4877(t *testing.T) { testFs = origTestFs }() - testScenario(t, name, testCase, expectedEvents, allowedEvents, "") + testScenario(t, name, testCase, expectedEvents, allowedEvents, fakeMatcher{}) } // path relative to folder root, also creates parent dirs if necessary @@ -312,7 +349,7 @@ func sleepMs(ms int) { time.Sleep(time.Duration(ms) * time.Millisecond) } -func testScenario(t *testing.T, name string, testCase func(), expectedEvents, allowedEvents []Event, ignored string) { +func testScenario(t *testing.T, name string, testCase func(), expectedEvents, allowedEvents []Event, fm fakeMatcher) { if err := testFs.MkdirAll(name, 0755); err != nil { panic(fmt.Sprintf("Failed to create directory %s: %s", name, err)) } @@ -321,11 +358,7 @@ func testScenario(t *testing.T, name string, testCase func(), expectedEvents, al ctx, cancel := context.WithCancel(context.Background()) defer cancel() - if ignored != "" { - ignored = filepath.Join(name, ignored) - } - - eventChan, err := testFs.Watch(name, fakeMatcher{ignored}, ctx, false) + eventChan, err := testFs.Watch(name, fm, ctx, false) if err != nil { panic(err) } @@ -335,7 +368,7 @@ func testScenario(t *testing.T, name string, testCase func(), expectedEvents, al testCase() select { - case <-time.NewTimer(time.Minute).C: + case <-time.After(time.Minute): t.Errorf("Timed out before receiving all expected events") case <-ctx.Done(): @@ -382,10 +415,19 @@ func testWatchOutput(t *testing.T, name string, in <-chan Event, expectedEvents, } } -type fakeMatcher struct{ match string } +// Matches are done via direct comparison against both ignore and include +type fakeMatcher struct { + ignore string + include string + skipIgnoredDirs bool +} func (fm fakeMatcher) ShouldIgnore(name string) bool { - return name == fm.match + return name != fm.include && name == fm.ignore +} + +func (fm fakeMatcher) SkipIgnoredDirs() bool { + return fm.skipIgnoredDirs } type fakeEventInfo string diff --git a/lib/fs/filesystem.go b/lib/fs/filesystem.go index 9731755dc..7e5e30d2a 100644 --- a/lib/fs/filesystem.go +++ b/lib/fs/filesystem.go @@ -87,6 +87,7 @@ type Usage struct { type Matcher interface { ShouldIgnore(name string) bool + SkipIgnoredDirs() bool } type MatchResult interface { diff --git a/lib/ignore/ignore.go b/lib/ignore/ignore.go index 2d44c1c4e..59c9be9a4 100644 --- a/lib/ignore/ignore.go +++ b/lib/ignore/ignore.go @@ -77,15 +77,16 @@ type ChangeDetector interface { } type Matcher struct { - fs fs.Filesystem - lines []string // exact lines read from .stignore - patterns []Pattern // patterns including those from included files - withCache bool - matches *cache - curHash string - stop chan struct{} - changeDetector ChangeDetector - mut sync.Mutex + fs fs.Filesystem + lines []string // exact lines read from .stignore + patterns []Pattern // patterns including those from included files + withCache bool + matches *cache + curHash string + stop chan struct{} + changeDetector ChangeDetector + skipIgnoredDirs bool + mut sync.Mutex } // An Option can be passed to New() @@ -108,9 +109,10 @@ func WithChangeDetector(cd ChangeDetector) Option { func New(fs fs.Filesystem, opts ...Option) *Matcher { m := &Matcher{ - fs: fs, - stop: make(chan struct{}), - mut: sync.NewMutex(), + fs: fs, + stop: make(chan struct{}), + mut: sync.NewMutex(), + skipIgnoredDirs: true, } for _, opt := range opts { opt(m) @@ -169,6 +171,14 @@ func (m *Matcher) parseLocked(r io.Reader, file string) error { return err } + m.skipIgnoredDirs = true + for _, p := range patterns { + if p.result&resultInclude == resultInclude { + m.skipIgnoredDirs = false + break + } + } + m.curHash = newHash m.patterns = patterns if m.withCache { @@ -291,6 +301,12 @@ func (m *Matcher) ShouldIgnore(filename string) bool { return false } +func (m *Matcher) SkipIgnoredDirs() bool { + m.mut.Lock() + defer m.mut.Unlock() + return m.skipIgnoredDirs +} + func hashPatterns(patterns []Pattern) string { h := md5.New() for _, pat := range patterns { diff --git a/lib/scanner/testdata/.stignore b/lib/scanner/testdata/.stignore index f9656bfd9..71df1a444 100644 --- a/lib/scanner/testdata/.stignore +++ b/lib/scanner/testdata/.stignore @@ -2,3 +2,4 @@ bfile dir1/cfile +/dir2/dir21 diff --git a/lib/scanner/testdata/dir2/dir21/dir22/dir23/efile b/lib/scanner/testdata/dir2/dir21/dir22/dir23/efile new file mode 100644 index 000000000..e69de29bb diff --git a/lib/scanner/testdata/dir2/dir21/dir22/efile/efile b/lib/scanner/testdata/dir2/dir21/dir22/efile/efile new file mode 100644 index 000000000..e69de29bb diff --git a/lib/scanner/testdata/dir2/dir21/dira/efile b/lib/scanner/testdata/dir2/dir21/dira/efile new file mode 100644 index 000000000..e69de29bb diff --git a/lib/scanner/testdata/dir2/dir21/dira/ffile b/lib/scanner/testdata/dir2/dir21/dira/ffile new file mode 100644 index 000000000..e69de29bb diff --git a/lib/scanner/testdata/dir2/dir21/efile/ign/efile b/lib/scanner/testdata/dir2/dir21/efile/ign/efile new file mode 100644 index 000000000..e69de29bb diff --git a/lib/scanner/walk.go b/lib/scanner/walk.go index 3f1f22905..c77f243c4 100644 --- a/lib/scanner/walk.go +++ b/lib/scanner/walk.go @@ -8,7 +8,9 @@ package scanner import ( "context" + "path/filepath" "runtime" + "strings" "sync/atomic" "time" "unicode/utf8" @@ -196,6 +198,8 @@ func (w *walker) walk(ctx context.Context) chan protocol.FileInfo { func (w *walker) walkAndHashFiles(ctx context.Context, fchan, dchan chan protocol.FileInfo) fs.WalkFunc { now := time.Now() + ignoredParent := "" + return func(path string, info fs.FileInfo, err error) error { select { case <-ctx.Done(): @@ -220,12 +224,6 @@ func (w *walker) walkAndHashFiles(ctx context.Context, fchan, dchan chan protoco return nil } - 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 { - return skip - } - if fs.IsTemporary(path) { l.Debugln("temporary:", path) if info.IsRegular() && info.ModTime().Add(w.TempLifetime).Before(now) { @@ -240,43 +238,88 @@ func (w *walker) walkAndHashFiles(ctx context.Context, fchan, dchan chan protoco return skip } - if w.Matcher.Match(path).IsIgnored() { - l.Debugln("ignored (patterns):", path) - return skip - } - if !utf8.ValidString(path) { l.Warnf("File name %q is not in UTF8 encoding; skipping.", path) return skip } - path, shouldSkip := w.normalizePath(path, info) - if shouldSkip { - return skip - } - - switch { - case info.IsSymlink(): - if err := w.walkSymlink(ctx, path, dchan); err != nil { - return err - } - if info.IsDir() { - // under no circumstances shall we descend into a symlink + if w.Matcher.Match(path).IsIgnored() { + l.Debugln("ignored (patterns):", path) + // Only descend if matcher says so and the current file is not a symlink. + if w.Matcher.SkipIgnoredDirs() || (info.IsSymlink() && info.IsDir()) { return fs.SkipDir } + // If the parent wasn't ignored already, set this path as the "highest" ignored parent + if info.IsDir() && (ignoredParent == "" || !strings.HasPrefix(path, ignoredParent+string(fs.PathSeparator))) { + ignoredParent = path + } return nil - - case info.IsDir(): - err = w.walkDir(ctx, path, info, dchan) - - case info.IsRegular(): - err = w.walkRegular(ctx, path, info, fchan) } - return err + if ignoredParent == "" { + // parent isn't ignored, nothing special + return w.handleItem(ctx, path, fchan, dchan, skip) + } + + // Part of current path below the ignored (potential) parent + rel := strings.TrimPrefix(path, ignoredParent+string(fs.PathSeparator)) + + // ignored path isn't actually a parent of the current path + if rel == path { + ignoredParent = "" + return w.handleItem(ctx, path, fchan, dchan, 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 { + 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 { + return err + } + } + ignoredParent = "" + + return nil } } +func (w *walker) handleItem(ctx context.Context, path string, fchan, dchan chan protocol.FileInfo, 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 { + return skip + } + + path, shouldSkip := w.normalizePath(path, info) + if shouldSkip { + return skip + } + + switch { + case info.IsSymlink(): + if err := w.walkSymlink(ctx, path, dchan); err != nil { + return err + } + if info.IsDir() { + // under no circumstances shall we descend into a symlink + return fs.SkipDir + } + return nil + + case info.IsDir(): + err = w.walkDir(ctx, path, info, dchan) + + case info.IsRegular(): + err = w.walkRegular(ctx, path, info, fchan) + } + + return err +} + func (w *walker) walkRegular(ctx context.Context, relPath string, info fs.FileInfo, fchan chan protocol.FileInfo) error { curMode := uint32(info.Mode()) if runtime.GOOS == "windows" && osutil.IsWindowsExecutable(relPath) { diff --git a/lib/scanner/walk_test.go b/lib/scanner/walk_test.go index 80ad3f5d6..6e7ce6a39 100644 --- a/lib/scanner/walk_test.go +++ b/lib/scanner/walk_test.go @@ -38,6 +38,8 @@ type testfile struct { type testfileList []testfile +var testFs fs.Filesystem + var testdata = testfileList{ {"afile", 4, "b5bb9d8014a0f9b1d61e21e796d78dccdf1352f23cd32812f4850b878ae4944c"}, {"dir1", 128, ""}, @@ -53,17 +55,19 @@ func init() { // Limit the stack size to 10 megs to crash early in that case instead of // potentially taking down the box... rdebug.SetMaxStack(10 * 1 << 20) + + testFs = fs.NewFilesystem(fs.FilesystemTypeBasic, "testdata") } func TestWalkSub(t *testing.T) { - ignores := ignore.New(fs.NewFilesystem(fs.FilesystemTypeBasic, ".")) - err := ignores.Load("testdata/.stignore") + ignores := ignore.New(testFs) + err := ignores.Load(".stignore") if err != nil { t.Fatal(err) } fchan := Walk(context.TODO(), Config{ - Filesystem: fs.NewFilesystem(fs.FilesystemTypeBasic, "testdata"), + Filesystem: testFs, Subs: []string{"dir2"}, Matcher: ignores, Hashers: 2, @@ -88,15 +92,15 @@ func TestWalkSub(t *testing.T) { } func TestWalk(t *testing.T) { - ignores := ignore.New(fs.NewFilesystem(fs.FilesystemTypeBasic, ".")) - err := ignores.Load("testdata/.stignore") + ignores := ignore.New(testFs) + err := ignores.Load(".stignore") if err != nil { t.Fatal(err) } t.Log(ignores) fchan := Walk(context.TODO(), Config{ - Filesystem: fs.NewFilesystem(fs.FilesystemTypeBasic, "testdata"), + Filesystem: testFs, Matcher: ignores, Hashers: 2, }) @@ -192,11 +196,9 @@ func TestNormalization(t *testing.T) { numValid := len(tests) - numInvalid - fs := fs.NewFilesystem(fs.FilesystemTypeBasic, ".") - for _, s1 := range tests { // Create a directory for each of the interesting strings above - if err := fs.MkdirAll(filepath.Join("testdata/normalization", s1), 0755); err != nil { + if err := testFs.MkdirAll(filepath.Join("normalization", s1), 0755); err != nil { t.Fatal(err) } @@ -205,7 +207,7 @@ func TestNormalization(t *testing.T) { // file names. Ensure that the file doesn't exist when it's // created. This detects and fails if there's file name // normalization stuff at the filesystem level. - if fd, err := fs.OpenFile(filepath.Join("testdata/normalization", s1, s2), os.O_CREATE|os.O_EXCL, 0644); err != nil { + if fd, err := testFs.OpenFile(filepath.Join("normalization", s1, s2), os.O_CREATE|os.O_EXCL, 0644); err != nil { t.Fatal(err) } else { fd.Write([]byte("test")) @@ -219,14 +221,8 @@ func TestNormalization(t *testing.T) { // make sure it all gets done. In production, things will be correct // eventually... - _, err := walkDir(fs, "testdata/normalization", nil) - if err != nil { - t.Fatal(err) - } - tmp, err := walkDir(fs, "testdata/normalization", nil) - if err != nil { - t.Fatal(err) - } + walkDir(testFs, "normalization", nil, nil) + tmp := walkDir(testFs, "normalization", nil, nil) files := fileList(tmp).testfiles() @@ -268,9 +264,10 @@ func TestWalkSymlinkUnix(t *testing.T) { defer os.RemoveAll("_symlinks") os.Symlink("../testdata", "_symlinks/link") + fs := fs.NewFilesystem(fs.FilesystemTypeBasic, "_symlinks") for _, path := range []string{".", "link"} { // Scan it - files, _ := walkDir(fs.NewFilesystem(fs.FilesystemTypeBasic, "_symlinks"), path, nil) + files := walkDir(fs, path, nil, nil) // Verify that we got one symlink and with the correct attributes if len(files) != 1 { @@ -291,9 +288,11 @@ func TestWalkSymlinkWindows(t *testing.T) { } // Create a folder with a symlink in it - os.RemoveAll("_symlinks") - os.Mkdir("_symlinks", 0755) - defer os.RemoveAll("_symlinks") + name := "_symlinks-win" + os.RemoveAll(name) + os.Mkdir(name, 0755) + defer os.RemoveAll(name) + fs := fs.NewFilesystem(fs.FilesystemTypeBasic, name) if err := osutil.DebugSymlinkForTestsOnly("../testdata", "_symlinks/link"); err != nil { // Probably we require permissions we don't have. t.Skip(err) @@ -301,7 +300,7 @@ func TestWalkSymlinkWindows(t *testing.T) { for _, path := range []string{".", "link"} { // Scan it - files, _ := walkDir(fs.NewFilesystem(fs.FilesystemTypeBasic, "_symlinks"), path, nil) + files := walkDir(fs, path, nil, nil) // Verify that we got zero symlinks if len(files) != 0 { @@ -330,10 +329,8 @@ func TestWalkRootSymlink(t *testing.T) { } // Scan it - files, err := walkDir(fs.NewFilesystem(fs.FilesystemTypeBasic, link), ".", nil) - if err != nil { - t.Fatal("Expected no error when root folder path is provided via a symlink: " + err.Error()) - } + files := walkDir(fs.NewFilesystem(fs.FilesystemTypeBasic, link), ".", nil, nil) + // Verify that we got two files if len(files) != 2 { t.Errorf("expected two files, not %d", len(files)) @@ -352,10 +349,7 @@ func TestBlocksizeHysteresis(t *testing.T) { current := make(fakeCurrentFiler) runTest := func(expectedBlockSize int) { - files, err := walkDir(sf, ".", current) - if err != nil { - t.Fatal(err) - } + files := walkDir(sf, ".", current, nil) if len(files) != 1 { t.Fatalf("expected one file, not %d", len(files)) } @@ -409,7 +403,7 @@ func TestBlocksizeHysteresis(t *testing.T) { runTest(512 << 10) } -func walkDir(fs fs.Filesystem, dir string, cfiler CurrentFiler) ([]protocol.FileInfo, error) { +func walkDir(fs fs.Filesystem, dir string, cfiler CurrentFiler, matcher *ignore.Matcher) []protocol.FileInfo { fchan := Walk(context.TODO(), Config{ Filesystem: fs, Subs: []string{dir}, @@ -417,6 +411,7 @@ func walkDir(fs fs.Filesystem, dir string, cfiler CurrentFiler) ([]protocol.File Hashers: 2, UseLargeBlocks: true, CurrentFiler: cfiler, + Matcher: matcher, }) var tmp []protocol.FileInfo @@ -425,7 +420,7 @@ func walkDir(fs fs.Filesystem, dir string, cfiler CurrentFiler) ([]protocol.File } sort.Sort(fileList(tmp)) - return tmp, nil + return tmp } type fileList []protocol.FileInfo @@ -580,15 +575,53 @@ func TestIssue4799(t *testing.T) { } fd.Close() - files, err := walkDir(fs, "/foo", nil) - if err != nil { - t.Fatal(err) - } + files := walkDir(fs, "/foo", nil, nil) if len(files) != 1 || files[0].Name != "foo" { t.Error(`Received unexpected file infos when walking "/foo"`, files) } } +func TestRecurseInclude(t *testing.T) { + stignore := ` + !/dir1/cfile + !efile + !ffile + * + ` + ignores := ignore.New(testFs, ignore.WithCache(true)) + if err := ignores.Parse(bytes.NewBufferString(stignore), ".stignore"); err != nil { + t.Fatal(err) + } + + files := walkDir(testFs, ".", nil, ignores) + + expected := []string{ + filepath.Join("dir1"), + filepath.Join("dir1", "cfile"), + filepath.Join("dir2"), + filepath.Join("dir2", "dir21"), + filepath.Join("dir2", "dir21", "dir22"), + filepath.Join("dir2", "dir21", "dir22", "dir23"), + filepath.Join("dir2", "dir21", "dir22", "dir23", "efile"), + filepath.Join("dir2", "dir21", "dir22", "efile"), + filepath.Join("dir2", "dir21", "dir22", "efile", "efile"), + filepath.Join("dir2", "dir21", "dira"), + filepath.Join("dir2", "dir21", "dira", "efile"), + filepath.Join("dir2", "dir21", "dira", "ffile"), + filepath.Join("dir2", "dir21", "efile"), + filepath.Join("dir2", "dir21", "efile", "ign"), + filepath.Join("dir2", "dir21", "efile", "ign", "efile"), + } + if len(files) != len(expected) { + t.Fatalf("Got %d files %v, expected %d files at %v", len(files), files, len(expected), expected) + } + for i := range files { + if files[i].Name != expected[i] { + t.Errorf("Got %v, expected file at %v", files[i], expected[i]) + } + } +} + func TestIssue4841(t *testing.T) { tmp, err := ioutil.TempDir("", "") if err != nil {