Merge pull request #1447 from fawick/fixMultipleRejectIfPresent

Fix multiple rejectIfPresent cancelling each other

cmd/restic/cmd_backup.go

@@ -372,9 +372,8 @@ func runBackup(opts BackupOptions, gopts GlobalOptions, args []string) error {
 		opts.ExcludeIfPresent = append(opts.ExcludeIfPresent, "CACHEDIR.TAG:Signature: 8a477f597d28d172789f06886806bc55")
 	}
 
-	rc := &rejectionCache{}
 	for _, spec := range opts.ExcludeIfPresent {
-		f, err := rejectIfPresent(spec, rc)
+		f, err := rejectIfPresent(spec)
 		if err != nil {
 			return err
 		}

cmd/restic/exclude.go

@@ -90,7 +90,7 @@ func rejectByPattern(patterns []string) RejectFunc {
 // non-nil if the filename component of excludeFileSpec is empty. If rc is
 // non-nil, it is going to be used in the RejectFunc to expedite the evaluation
 // of a directory based on previous visits.
-func rejectIfPresent(excludeFileSpec string, rc *rejectionCache) (RejectFunc, error) {
+func rejectIfPresent(excludeFileSpec string) (RejectFunc, error) {
 	if excludeFileSpec == "" {
 		return nil, errors.New("name for exclusion tagfile is empty")
 	}
@@ -106,6 +106,7 @@ func rejectIfPresent(excludeFileSpec string, rc *rejectionCache) (RejectFunc, er
 		tf = excludeFileSpec
 	}
 	debug.Log("using %q as exclusion tagfile", tf)
+	rc := &rejectionCache{}
 	fn := func(filename string, _ os.FileInfo) bool {
 		return isExcludedByFile(filename, tf, tc, rc)
 	}

cmd/restic/exclude_test.go

@@ -2,6 +2,7 @@ package main
 
 import (
 	"io/ioutil"
+	"os"
 	"path/filepath"
 	"testing"
 
@@ -82,3 +83,82 @@ func TestIsExcludedByFile(t *testing.T) {
 		})
 	}
 }
+
+// TestMultipleIsExcludedByFile is for testing that multiple instances of
+// the --exclude-if-present parameter (or the shortcut --exclude-caches do not
+// cancel each other out. It was initially written to demonstrate a bug in
+// rejectIfPresent.
+func TestMultipleIsExcludedByFile(t *testing.T) {
+	tempDir, cleanup := test.TempDir(t)
+	defer cleanup()
+
+	// Create some files in a temporary directory.
+	// Files in UPPERCASE will be used as exclusion triggers later on.
+	// We will test the inclusion later, so we add the expected value as
+	// a bool.
+	files := []struct {
+		path string
+		incl bool
+	}{
+		{"42", true},
+
+		// everything in foodir except the NOFOO tagfile
+		// should not be included.
+		{"foodir/NOFOO", true},
+		{"foodir/foo", false},
+		{"foodir/foosub/underfoo", false},
+
+		// everything in bardir except the NOBAR tagfile
+		// should not be included.
+		{"bardir/NOBAR", true},
+		{"bardir/bar", false},
+		{"bardir/barsub/underbar", false},
+
+		// everything in bazdir should be included.
+		{"bazdir/baz", true},
+		{"bazdir/bazsub/underbaz", true},
+	}
+	var errs []error
+	for _, f := range files {
+		// create directories first, then the file
+		p := filepath.Join(tempDir, filepath.FromSlash(f.path))
+		errs = append(errs, os.MkdirAll(filepath.Dir(p), 0700))
+		errs = append(errs, ioutil.WriteFile(p, []byte(f.path), 0600))
+	}
+	test.OKs(t, errs) // see if anything went wrong during the creation
+
+	// create two rejection functions, one that tests for the NOFOO file
+	// and one for the NOBAR file
+	fooExclude, _ := rejectIfPresent("NOFOO")
+	barExclude, _ := rejectIfPresent("NOBAR")
+
+	// To mock the archiver scanning walk, we create filepath.WalkFn
+	// that tests against the two rejection functions and stores
+	// the result in a map against we can test later.
+	m := make(map[string]bool)
+	walk := func(p string, fi os.FileInfo, err error) error {
+		if err != nil {
+			return err
+		}
+		excludedByFoo := fooExclude(p, fi)
+		excludedByBar := barExclude(p, fi)
+		excluded := excludedByFoo || excludedByBar
+		// the log message helps debugging in case the test fails
+		t.Logf("%q: %v || %v = %v", p, excludedByFoo, excludedByBar, excluded)
+		m[p] = !excluded
+		if excluded {
+			return filepath.SkipDir
+		}
+		return nil
+	}
+	// walk through the temporary file and check the error
+	test.OK(t, filepath.Walk(tempDir, walk))
+
+	// compare whether the walk gave the expected values for the test cases
+	for _, f := range files {
+		p := filepath.Join(tempDir, filepath.FromSlash(f.path))
+		if m[p] != f.incl {
+			t.Errorf("inclusion status of %s is wrong: want %v, got %v", f.path, f.incl, m[p])
+		}
+	}
+}