lib/ignore: Implement deletable ignores using (?d) prefix (fixes #1362)

lib/ignore/cache.go

@@ -14,7 +14,7 @@ type cache struct {
 }
 
 type cacheEntry struct {
-	value  bool
+	result Result
 	access time.Time
 }
 
@@ -33,17 +33,17 @@ func (c *cache) clean(d time.Duration) {
 	}
 }
 
-func (c *cache) get(key string) (result, ok bool) {
+func (c *cache) get(key string) (Result, bool) {
-	res, ok := c.entries[key]
+	entry, ok := c.entries[key]
 	if ok {
-		res.access = time.Now()
+		entry.access = time.Now()
-		c.entries[key] = res
+		c.entries[key] = entry
 	}
-	return res.value, ok
+	return entry.result, ok
 }
 
-func (c *cache) set(key string, val bool) {
+func (c *cache) set(key string, result Result) {
-	c.entries[key] = cacheEntry{val, time.Now()}
+	c.entries[key] = cacheEntry{result, time.Now()}
 }
 
 func (c *cache) len() int {

lib/ignore/cache_test.go

@@ -15,22 +15,22 @@ func TestCache(t *testing.T) {
 	c := newCache(nil)
 
 	res, ok := c.get("nonexistent")
-	if res != false || ok != false {
+	if res.IsIgnored() || res.IsDeletable() || ok != false {
 		t.Errorf("res %v, ok %v for nonexistent item", res, ok)
 	}
 
 	// Set and check some items
 
-	c.set("true", true)
+	c.set("true", Result{true, true})
-	c.set("false", false)
+	c.set("false", Result{false, false})
 
 	res, ok = c.get("true")
-	if res != true || ok != true {
+	if !res.IsIgnored() || !res.IsDeletable() || ok != true {
 		t.Errorf("res %v, ok %v for true item", res, ok)
 	}
 
 	res, ok = c.get("false")
-	if res != false || ok != true {
+	if res.IsIgnored() || res.IsDeletable() || ok != true {
 		t.Errorf("res %v, ok %v for false item", res, ok)
 	}
 
@@ -41,12 +41,12 @@ func TestCache(t *testing.T) {
 	// Same values should exist
 
 	res, ok = c.get("true")
-	if res != true || ok != true {
+	if !res.IsIgnored() || !res.IsDeletable() || ok != true {
 		t.Errorf("res %v, ok %v for true item", res, ok)
 	}
 
 	res, ok = c.get("false")
-	if res != false || ok != true {
+	if res.IsIgnored() || res.IsDeletable() || ok != true {
 		t.Errorf("res %v, ok %v for false item", res, ok)
 	}
 

lib/ignore/ignore.go

@@ -22,11 +22,17 @@ import (
 	"github.com/syncthing/syncthing/lib/sync"
 )
 
+var notMatched = Result{
+	include:   false,
+	deletable: false,
+}
+
 type Pattern struct {
-	pattern  string
+	pattern   string
-	match    glob.Glob
+	match     glob.Glob
-	include  bool
+	include   bool
-	foldCase bool
+	foldCase  bool
+	deletable bool
 }
 
 func (p Pattern) String() string {
@@ -37,9 +43,25 @@ func (p Pattern) String() string {
 	if p.foldCase {
 		ret = "(?i)" + ret
 	}
+	if p.deletable {
+		ret = "(?d)" + ret
+	}
 	return ret
 }
 
+type Result struct {
+	include   bool
+	deletable bool
+}
+
+func (r Result) IsIgnored() bool {
+	return r.include
+}
+
+func (r Result) IsDeletable() bool {
+	return r.include && r.deletable
+}
+
 type Matcher struct {
 	patterns  []Pattern
 	withCache bool
@@ -99,16 +121,16 @@ func (m *Matcher) Parse(r io.Reader, file string) error {
 	return err
 }
 
-func (m *Matcher) Match(file string) (result bool) {
+func (m *Matcher) Match(file string) (result Result) {
 	if m == nil {
-		return false
+		return notMatched
 	}
 
 	m.mut.Lock()
 	defer m.mut.Unlock()
 
 	if len(m.patterns) == 0 {
-		return false
+		return notMatched
 	}
 
 	if m.matches != nil {
@@ -133,17 +155,23 @@ func (m *Matcher) Match(file string) (result bool) {
 				lowercaseFile = strings.ToLower(file)
 			}
 			if pattern.match.Match(lowercaseFile) {
-				return pattern.include
+				return Result{
+					pattern.include,
+					pattern.deletable,
+				}
 			}
 		} else {
 			if pattern.match.Match(file) {
-				return pattern.include
+				return Result{
+					pattern.include,
+					pattern.deletable,
+				}
 			}
 		}
 	}
 
 	// Default to false.
-	return false
+	return notMatched
 }
 
 // Patterns return a list of the loaded patterns, as they've been parsed
@@ -223,14 +251,25 @@ func parseIgnoreFile(fd io.Reader, currentFile string, seen map[string]bool) ([]
 			foldCase: runtime.GOOS == "darwin" || runtime.GOOS == "windows",
 		}
 
-		if strings.HasPrefix(line, "!") {
+		// Allow prefixes to be specified in any order, but only once.
-			line = line[1:]
+		var seenPrefix [3]bool
-			pattern.include = false
-		}
 
-		if strings.HasPrefix(line, "(?i)") {
+		for {
-			pattern.foldCase = true
+			if strings.HasPrefix(line, "!") && !seenPrefix[0] {
-			line = line[4:]
+				seenPrefix[0] = true
+				line = line[1:]
+				pattern.include = false
+			} else if strings.HasPrefix(line, "(?i)") && !seenPrefix[1] {
+				seenPrefix[1] = true
+				pattern.foldCase = true
+				line = line[4:]
+			} else if strings.HasPrefix(line, "(?d)") && !seenPrefix[2] {
+				seenPrefix[2] = true
+				pattern.deletable = true
+				line = line[4:]
+			} else {
+				break
+			}
 		}
 
 		if pattern.foldCase {

lib/ignore/ignore_test.go

@@ -8,6 +8,7 @@ package ignore
 
 import (
 	"bytes"
+	"fmt"
 	"io/ioutil"
 	"os"
 	"path/filepath"
@@ -52,7 +53,7 @@ func TestIgnore(t *testing.T) {
 	}
 
 	for i, tc := range tests {
-		if r := pats.Match(tc.f); r != tc.r {
+		if r := pats.Match(tc.f); r.IsIgnored() != tc.r {
 			t.Errorf("Incorrect ignoreFile() #%d (%s); E: %v, A: %v", i, tc.f, tc.r, r)
 		}
 	}
@@ -90,12 +91,90 @@ func TestExcludes(t *testing.T) {
 	}
 
 	for _, tc := range tests {
-		if r := pats.Match(tc.f); r != tc.r {
+		if r := pats.Match(tc.f); r.IsIgnored() != tc.r {
 			t.Errorf("Incorrect match for %s: %v != %v", tc.f, r, tc.r)
 		}
 	}
 }
 
+func TestFlagOrder(t *testing.T) {
+	stignore := `
+	## Ok cases
+	(?i)(?d)!ign1
+	(?d)(?i)!ign2
+	(?i)!(?d)ign3
+	(?d)!(?i)ign4
+	!(?i)(?d)ign5
+	!(?d)(?i)ign6
+	## Bad cases
+	!!(?i)(?d)ign7
+	(?i)(?i)(?d)ign8
+	(?i)(?d)(?d)!ign9
+	(?d)(?d)!ign10
+	`
+	pats := New(true)
+	err := pats.Parse(bytes.NewBufferString(stignore), ".stignore")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	for i := 1; i < 7; i++ {
+		pat := fmt.Sprintf("ign%d", i)
+		if r := pats.Match(pat); r.IsIgnored() || r.IsDeletable() {
+			t.Errorf("incorrect %s", pat)
+		}
+	}
+	for i := 7; i < 10; i++ {
+		pat := fmt.Sprintf("ign%d", i)
+		if r := pats.Match(pat); r.IsDeletable() {
+			t.Errorf("incorrect %s", pat)
+		}
+	}
+
+	if r := pats.Match("(?d)!ign10"); !r.IsIgnored() {
+		t.Errorf("incorrect")
+	}
+}
+
+func TestDeletables(t *testing.T) {
+	stignore := `
+	(?d)ign1
+	(?d)(?i)ign2
+	(?i)(?d)ign3
+	!(?d)ign4
+	!ign5
+	!(?i)(?d)ign6
+	ign7
+	(?i)ign8
+	`
+	pats := New(true)
+	err := pats.Parse(bytes.NewBufferString(stignore), ".stignore")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var tests = []struct {
+		f string
+		i bool
+		d bool
+	}{
+		{"ign1", true, true},
+		{"ign2", true, true},
+		{"ign3", true, true},
+		{"ign4", false, false},
+		{"ign5", false, false},
+		{"ign6", false, false},
+		{"ign7", true, false},
+		{"ign8", true, false},
+	}
+
+	for _, tc := range tests {
+		if r := pats.Match(tc.f); r.IsIgnored() != tc.i || r.IsDeletable() != tc.d {
+			t.Errorf("Incorrect match for %s: %v != Result{%t, %t}", tc.f, r, tc.i, tc.d)
+		}
+	}
+}
+
 func TestBadPatterns(t *testing.T) {
 	var badPatterns = []string{
 		"[",
@@ -132,13 +211,13 @@ func TestCaseSensitivity(t *testing.T) {
 	}
 
 	for _, tc := range match {
-		if !ign.Match(tc) {
+		if !ign.Match(tc).IsIgnored() {
 			t.Errorf("Incorrect match for %q: should be matched", tc)
 		}
 	}
 
 	for _, tc := range dontMatch {
-		if ign.Match(tc) {
+		if ign.Match(tc).IsIgnored() {
 			t.Errorf("Incorrect match for %q: should not be matched", tc)
 		}
 	}
@@ -277,7 +356,7 @@ func TestCommentsAndBlankLines(t *testing.T) {
 	}
 }
 
-var result bool
+var result Result
 
 func BenchmarkMatch(b *testing.B) {
 	stignore := `
@@ -381,13 +460,13 @@ func TestCacheReload(t *testing.T) {
 
 	// Verify that both are ignored
 
-	if !pats.Match("f1") {
+	if !pats.Match("f1").IsIgnored() {
 		t.Error("Unexpected non-match for f1")
 	}
-	if !pats.Match("f2") {
+	if !pats.Match("f2").IsIgnored() {
 		t.Error("Unexpected non-match for f2")
 	}
-	if pats.Match("f3") {
+	if pats.Match("f3").IsIgnored() {
 		t.Error("Unexpected match for f3")
 	}
 
@@ -413,13 +492,13 @@ func TestCacheReload(t *testing.T) {
 
 	// Verify that the new patterns are in effect
 
-	if !pats.Match("f1") {
+	if !pats.Match("f1").IsIgnored() {
 		t.Error("Unexpected non-match for f1")
 	}
-	if pats.Match("f2") {
+	if pats.Match("f2").IsIgnored() {
 		t.Error("Unexpected match for f2")
 	}
-	if !pats.Match("f3") {
+	if !pats.Match("f3").IsIgnored() {
 		t.Error("Unexpected non-match for f3")
 	}
 }
@@ -526,7 +605,7 @@ func TestWindowsPatterns(t *testing.T) {
 
 	tests := []string{`a\b`, `c\d`}
 	for _, pat := range tests {
-		if !pats.Match(pat) {
+		if !pats.Match(pat).IsIgnored() {
 			t.Errorf("Should match %s", pat)
 		}
 	}
@@ -551,7 +630,7 @@ func TestAutomaticCaseInsensitivity(t *testing.T) {
 
 	tests := []string{`a/B`, `C/d`}
 	for _, pat := range tests {
-		if !pats.Match(pat) {
+		if !pats.Match(pat).IsIgnored() {
 			t.Errorf("Should match %s", pat)
 		}
 	}

lib/model/model.go

@@ -209,7 +209,7 @@ func (m *Model) warnAboutOverwritingProtectedFiles(folder string) {
 		}
 
 		// check if file is ignored
-		if ignores.Match(protectedFilePath) {
+		if ignores.Match(protectedFilePath).IsIgnored() {
 			continue
 		}
 
@@ -800,7 +800,7 @@ func (m *Model) Request(deviceID protocol.DeviceID, folder, name string, offset
 		// cleaned from any possible funny business.
 		if rn, err := filepath.Rel(folderPath, fn); err != nil {
 			return err
-		} else if folderIgnores.Match(rn) {
+		} else if folderIgnores.Match(rn).IsIgnored() {
 			l.Debugf("%v REQ(in) for ignored file: %s: %q / %q o=%d s=%d", m, deviceID, folder, name, offset, len(buf))
 			return protocol.ErrNoSuchFile
 		}
@@ -1149,7 +1149,7 @@ func sendIndexTo(initial bool, minLocalVer int64, conn protocol.Connection, fold
 			maxLocalVer = f.LocalVersion
 		}
 
-		if ignores.Match(f.Name) || symlinkInvalid(folder, f) {
+		if ignores.Match(f.Name).IsIgnored() || symlinkInvalid(folder, f) {
 			l.Debugln("not sending update for ignored/unsupported symlink", f)
 			return true
 		}
@@ -1441,7 +1441,7 @@ func (m *Model) internalScanFolderSubs(folder string, subs []string) error {
 					batch = batch[:0]
 				}
 
-				if ignores.Match(f.Name) || symlinkInvalid(folder, f) {
+				if ignores.Match(f.Name).IsIgnored() || symlinkInvalid(folder, f) {
 					// File has been ignored or an unsupported symlink. Set invalid bit.
 					l.Debugln("setting invalid bit on ignored", f)
 					nf := protocol.FileInfo{

lib/model/rwfolder.go

@@ -470,7 +470,7 @@ func (p *rwFolder) pullerIteration(ignores *ignore.Matcher) int {
 
 		file := intf.(protocol.FileInfo)
 
-		if ignores.Match(file.Name) {
+		if ignores.Match(file.Name).IsIgnored() {
 			// This is an ignored file. Skip it, continue iteration.
 			return true
 		}
@@ -583,7 +583,7 @@ nextFile:
 	for i := range dirDeletions {
 		dir := dirDeletions[len(dirDeletions)-i-1]
 		l.Debugln("Deleting dir", dir.Name)
-		p.deleteDir(dir)
+		p.deleteDir(dir, ignores)
 	}
 
 	// Wait for db updates to complete
@@ -689,7 +689,7 @@ func (p *rwFolder) handleDir(file protocol.FileInfo) {
 }
 
 // deleteDir attempts to delete the given directory
-func (p *rwFolder) deleteDir(file protocol.FileInfo) {
+func (p *rwFolder) deleteDir(file protocol.FileInfo, matcher *ignore.Matcher) {
 	var err error
 	events.Default.Log(events.ItemStarted, map[string]string{
 		"folder": p.folder,
@@ -712,9 +712,9 @@ func (p *rwFolder) deleteDir(file protocol.FileInfo) {
 	dir, _ := os.Open(realName)
 	if dir != nil {
 		files, _ := dir.Readdirnames(-1)
-		for _, file := range files {
+		for _, dirFile := range files {
-			if defTempNamer.IsTemporary(file) {
+			if defTempNamer.IsTemporary(dirFile) || (matcher != nil && matcher.Match(filepath.Join(file.Name, dirFile)).IsDeletable()) {
-				osutil.InWritableDir(osutil.Remove, filepath.Join(realName, file))
+				osutil.InWritableDir(osutil.Remove, filepath.Join(realName, dirFile))
 			}
 		}
 		dir.Close()

lib/scanner/walk.go

@@ -19,6 +19,7 @@ import (
 	"github.com/rcrowley/go-metrics"
 	"github.com/syncthing/syncthing/lib/db"
 	"github.com/syncthing/syncthing/lib/events"
+	"github.com/syncthing/syncthing/lib/ignore"
 	"github.com/syncthing/syncthing/lib/osutil"
 	"github.com/syncthing/syncthing/lib/protocol"
 	"github.com/syncthing/syncthing/lib/symlinks"
@@ -50,7 +51,7 @@ type Walker struct {
 	// BlockSize controls the size of the block used when hashing.
 	BlockSize int
 	// If Matcher is not nil, it is used to identify files to ignore which were specified by the user.
-	Matcher IgnoreMatcher
+	Matcher *ignore.Matcher
 	// If TempNamer is not nil, it is used to ignore temporary files when walking.
 	TempNamer TempNamer
 	// Number of hours to keep temporary files for
@@ -89,11 +90,6 @@ type CurrentFiler interface {
 	CurrentFile(name string) (protocol.FileInfo, bool)
 }
 
-type IgnoreMatcher interface {
-	// Match returns true if the file should be ignored.
-	Match(filename string) bool
-}
-
 // Walk returns the list of files found in the local folder by scanning the
 // file system. Files are blockwise hashed.
 func (w *Walker) Walk() (chan protocol.FileInfo, error) {
@@ -241,7 +237,7 @@ func (w *Walker) walkAndHashFiles(fchan, dchan chan protocol.FileInfo) filepath.
 		}
 
 		if sn := filepath.Base(relPath); sn == ".stignore" || sn == ".stfolder" ||
-			strings.HasPrefix(relPath, ".stversions") || (w.Matcher != nil && w.Matcher.Match(relPath)) {
+			strings.HasPrefix(relPath, ".stversions") || (w.Matcher != nil && w.Matcher.Match(relPath).IsIgnored()) {
 			// An ignored file
 			l.Debugln("ignored:", relPath)
 			return skip