Merge pull request #1134 from syncthing/fix-816

Don't ignore ignored items forever (fixes #816)

internal/ignore/ignore_test.go

@@ -353,3 +353,69 @@ flamingo
 		result = pats.Match("filename")
 	}
 }
+
+func TestCacheReload(t *testing.T) {
+	fd, err := ioutil.TempFile("", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	defer fd.Close()
+	defer os.Remove(fd.Name())
+
+	// Ignore file matches f1 and f2
+
+	_, err = fd.WriteString("f1\nf2\n")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	pats, err := Load(fd.Name(), true)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Verify that both are ignored
+
+	if !pats.Match("f1") {
+		t.Error("Unexpected non-match for f1")
+	}
+	if !pats.Match("f2") {
+		t.Error("Unexpected non-match for f2")
+	}
+	if pats.Match("f3") {
+		t.Error("Unexpected match for f3")
+	}
+
+	// Rewrite file to match f1 and f3
+
+	err = fd.Truncate(0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = fd.Seek(0, os.SEEK_SET)
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = fd.WriteString("f1\nf3\n")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	pats, err = Load(fd.Name(), true)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Verify that the new patterns are in effect
+
+	if !pats.Match("f1") {
+		t.Error("Unexpected non-match for f1")
+	}
+	if pats.Match("f2") {
+		t.Error("Unexpected match for f2")
+	}
+	if !pats.Match("f3") {
+		t.Error("Unexpected non-match for f3")
+	}
+}

internal/scanner/walk.go

@@ -143,26 +143,27 @@ func (w *Walker) walkAndHashFiles(fchan chan protocol.FileInfo) filepath.WalkFun
 
 		// Index wise symlinks are always files, regardless of what the target
 		// is, because symlinks carry their target path as their content.
-		if info.Mode()&os.ModeSymlink != 0 {
+		if info.Mode()&os.ModeSymlink == os.ModeSymlink {
 			var rval error
-			// If the target is a directory, do NOT descend down there.
-			// This will cause files to get tracked, and removing the symlink
-			// will as a result remove files in their real location.
-			// But do not SkipDir if the target is not a directory, as it will
-			// stop scanning the current directory.
+			// If the target is a directory, do NOT descend down there. This
+			// will cause files to get tracked, and removing the symlink will
+			// as a result remove files in their real location. But do not
+			// SkipDir if the target is not a directory, as it will stop
+			// scanning the current directory.
 			if info.IsDir() {
 				rval = filepath.SkipDir
 			}
 
-			// We always rehash symlinks as they have no modtime or
-			// permissions.
-			// We check if they point to the old target by checking that
-			// their existing blocks match with the blocks in the index.
-			// If we don't have a filer or don't support symlinks, skip.
-			if w.CurrentFiler == nil || !symlinks.Supported {
+			// If we don't support symlinks, skip.
+			if !symlinks.Supported {
 				return rval
 			}
 
+			// We always rehash symlinks as they have no modtime or
+			// permissions. We check if they point to the old target by
+			// checking that their existing blocks match with the blocks in
+			// the index.
+
 			target, flags, err := symlinks.Read(p)
 			flags = flags & protocol.SymlinkTypeMask
 			if err != nil {
@@ -180,9 +181,17 @@ func (w *Walker) walkAndHashFiles(fchan chan protocol.FileInfo) filepath.WalkFun
 				return rval
 			}
 
-			cf := w.CurrentFiler.CurrentFile(rn)
-			if !cf.IsDeleted() && cf.IsSymlink() && SymlinkTypeEqual(flags, cf.Flags) && BlocksEqual(cf.Blocks, blocks) {
-				return rval
+			if w.CurrentFiler != nil {
+				// A symlink is "unchanged", if
+				//  - it wasn't deleted (because it isn't now)
+				//  - it was a symlink
+				//  - it wasn't invalid
+				//  - the symlink type (file/dir) was the same
+				//  - the block list (i.e. hash of target) was the same
+				cf := w.CurrentFiler.CurrentFile(rn)
+				if !cf.IsDeleted() && cf.IsSymlink() && !cf.IsInvalid() && SymlinkTypeEqual(flags, cf.Flags) && BlocksEqual(cf.Blocks, blocks) {
+					return rval
+				}
 			}
 
 			f := protocol.FileInfo{
@@ -204,9 +213,15 @@ func (w *Walker) walkAndHashFiles(fchan chan protocol.FileInfo) filepath.WalkFun
 
 		if info.Mode().IsDir() {
 			if w.CurrentFiler != nil {
+				// A directory is "unchanged", if it
+				//  - has the same permissions as previously, unless we are ignoring permissions
+				//  - was not marked deleted (since it apparently exists now)
+				//  - was a directory previously (not a file or something else)
+				//  - was not a symlink (since it's a directory now)
+				//  - was not invalid (since it looks valid now)
 				cf := w.CurrentFiler.CurrentFile(rn)
 				permUnchanged := w.IgnorePerms || !cf.HasPermissionBits() || PermsEqual(cf.Flags, uint32(info.Mode()))
-				if !cf.IsDeleted() && cf.IsDirectory() && permUnchanged && !cf.IsSymlink() {
+				if permUnchanged && !cf.IsDeleted() && cf.IsDirectory() && !cf.IsSymlink() && !cf.IsInvalid() {
 					return nil
 				}
 			}
@@ -232,9 +247,16 @@ func (w *Walker) walkAndHashFiles(fchan chan protocol.FileInfo) filepath.WalkFun
 
 		if info.Mode().IsRegular() {
 			if w.CurrentFiler != nil {
+				// A file is "unchanged", if it
+				//  - has the same permissions as previously, unless we are ignoring permissions
+				//  - was not marked deleted (since it apparently exists now)
+				//  - had the same modification time as it has now
+				//  - was not a directory previously (since it's a file now)
+				//  - was not a symlink (since it's a file now)
+				//  - was not invalid (since it looks valid now)
 				cf := w.CurrentFiler.CurrentFile(rn)
 				permUnchanged := w.IgnorePerms || !cf.HasPermissionBits() || PermsEqual(cf.Flags, uint32(info.Mode()))
-				if !cf.IsDeleted() && cf.Modified == info.ModTime().Unix() && permUnchanged {
+				if permUnchanged && !cf.IsDeleted() && cf.Modified == info.ModTime().Unix() && !cf.IsDirectory() && !cf.IsSymlink() && !cf.IsInvalid() {
 					return nil
 				}
 

test/ignore_test.go

@@ -0,0 +1,146 @@
+// Copyright (C) 2014 The Syncthing Authors.
+//
+// This program is free software: you can redistribute it and/or modify it
+// under the terms of the GNU General Public License as published by the Free
+// Software Foundation, either version 3 of the License, or (at your option)
+// any later version.
+//
+// This program is distributed in the hope that it will be useful, but WITHOUT
+// ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+// FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+// more details.
+//
+// You should have received a copy of the GNU General Public License along
+// with this program. If not, see <http://www.gnu.org/licenses/>.
+
+// +build integration
+
+package integration
+
+import (
+	"log"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"github.com/syncthing/syncthing/internal/symlinks"
+)
+
+func TestIgnores(t *testing.T) {
+	// Clean and start a syncthing instance
+
+	log.Println("Cleaning...")
+	err := removeAll("s1", "h1/index")
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	p := syncthingProcess{ // id1
+		log:    "1.out",
+		argv:   []string{"-home", "h1"},
+		port:   8081,
+		apiKey: apiKey,
+	}
+	err = p.start()
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer p.stop()
+
+	// Create eight empty files and directories
+
+	files := []string{"f1", "f2", "f3", "f4", "f11", "f12", "f13", "f14"}
+	dirs := []string{"d1", "d2", "d3", "d4", "d11", "d12", "d13", "d14"}
+	all := append(files, dirs...)
+
+	for _, file := range files {
+		fd, err := os.Create(filepath.Join("s1", file))
+		if err != nil {
+			t.Fatal(err)
+		}
+		fd.Close()
+	}
+
+	for _, dir := range dirs {
+		err := os.Mkdir(filepath.Join("s1", dir), 0755)
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+
+	var syms []string
+	if symlinksSupported() {
+		syms = []string{"s1", "s2", "s3", "s4", "s11", "s12", "s13", "s14"}
+		for _, sym := range syms {
+			p := filepath.Join("s1", sym)
+			symlinks.Create(p, p, 0)
+		}
+		all = append(all, syms...)
+	}
+
+	// Rescan and verify that we see them all
+
+	p.post("/rest/scan?folder=default", nil)
+	m, err := p.model("default")
+	if err != nil {
+		t.Fatal(err)
+	}
+	expected := len(all) // nothing is ignored
+	if m.LocalFiles != expected {
+		t.Fatalf("Incorrect number of files after initial scan, %d != %d", m.LocalFiles, expected)
+	}
+
+	// Add some of them to an ignore file
+
+	fd, err := os.Create("s1/.stignore")
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = fd.WriteString("f1*\nf2\nd1*\nd2\ns1*\ns2") // [fds][34] only non-ignored items
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = fd.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Rescan and verify that we see them
+
+	p.post("/rest/scan?folder=default", nil)
+	m, err = p.model("default")
+	if err != nil {
+		t.Fatal(err)
+	}
+	expected = len(all) * 2 / 8 // two out of eight items of each type should remain
+	if m.LocalFiles != expected {
+		t.Fatalf("Incorrect number of files after first ignore, %d != %d", m.LocalFiles, expected)
+	}
+
+	// Change the pattern to include some of the files and dirs previously ignored
+
+	fd, err = os.Create("s1/.stignore")
+	if err != nil {
+		t.Fatal(err)
+	}
+	_, err = fd.WriteString("f2\nd2\ns2\n")
+	if err != nil {
+		t.Fatal(err)
+	}
+	err = fd.Close()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	// Rescan and verify that we see them
+
+	p.post("/rest/scan?folder=default", nil)
+	m, err = p.model("default")
+	if err != nil {
+		t.Fatal(err)
+	}
+	expected = len(all) * 7 / 8 // seven out of eight items of each type should remain
+	if m.LocalFiles != expected {
+		t.Fatalf("Incorrect number of files after second ignore, %d != %d", m.LocalFiles, expected)
+	}
+}

test/syncthingprocess.go

@@ -173,6 +173,38 @@ func (p *syncthingProcess) peerCompletion() (map[string]int, error) {
 	return comp, err
 }
 
+type model struct {
+	GlobalBytes   int
+	GlobalDeleted int
+	GlobalFiles   int
+	InSyncBytes   int
+	InSyncFiles   int
+	Invalid       string
+	LocalBytes    int
+	LocalDeleted  int
+	LocalFiles    int
+	NeedBytes     int
+	NeedFiles     int
+	State         string
+	StateChanged  time.Time
+	Version       int
+}
+
+func (p *syncthingProcess) model(folder string) (model, error) {
+	resp, err := p.get("/rest/model?folder=" + folder)
+	if err != nil {
+		return model{}, err
+	}
+
+	var res model
+	err = json.NewDecoder(resp.Body).Decode(&res)
+	if err != nil {
+		return model{}, err
+	}
+
+	return res, nil
+}
+
 type event struct {
 	ID   int
 	Time time.Time