restore: refactor include and exclude

- added includePatternOptions similar to excludePatternOptions
- followed similar approach to backup for selecting files for restore

cmd/restic/cmd_restore.go

@@ -2,12 +2,10 @@ package main
 
 import (
 	"context"
-	"strings"
 	"time"
 
 	"github.com/restic/restic/internal/debug"
 	"github.com/restic/restic/internal/errors"
-	"github.com/restic/restic/internal/filter"
 	"github.com/restic/restic/internal/restic"
 	"github.com/restic/restic/internal/restorer"
 	"github.com/restic/restic/internal/ui"
@@ -45,15 +43,9 @@ Exit status is 0 if the command was successful, and non-zero if there was any er
 
 // RestoreOptions collects all options for the restore command.
 type RestoreOptions struct {
-	Exclude                 []string
-	ExcludeFiles            []string
-	InsensitiveExclude      []string
-	InsensitiveExcludeFiles []string
-	Include                 []string
-	IncludeFiles            []string
-	InsensitiveInclude      []string
-	InsensitiveIncludeFiles []string
-	Target                  string
+	excludePatternOptions
+	includePatternOptions
+	Target string
 	restic.SnapshotFilter
 	Sparse bool
 	Verify bool
@@ -65,15 +57,10 @@ func init() {
 	cmdRoot.AddCommand(cmdRestore)
 
 	flags := cmdRestore.Flags()
-	flags.StringArrayVarP(&restoreOptions.Exclude, "exclude", "e", nil, "exclude a `pattern` (can be specified multiple times)")
-	flags.StringArrayVar(&restoreOptions.InsensitiveExclude, "iexclude", nil, "same as --exclude but ignores the casing of `pattern`")
-	flags.StringArrayVarP(&restoreOptions.Include, "include", "i", nil, "include a `pattern`, exclude everything else (can be specified multiple times)")
-	flags.StringArrayVar(&restoreOptions.InsensitiveInclude, "iinclude", nil, "same as --include but ignores the casing of `pattern`")
 	flags.StringVarP(&restoreOptions.Target, "target", "t", "", "directory to extract data to")
-	flags.StringArrayVar(&restoreOptions.ExcludeFiles, "exclude-file", nil, "read exclude patterns from a `file` (can be specified multiple times)")
-	flags.StringArrayVar(&restoreOptions.InsensitiveExcludeFiles, "iexclude-file", nil, "same as --exclude-file but ignores casing of `file`names in patterns")
-	flags.StringArrayVar(&restoreOptions.IncludeFiles, "include-file", nil, "read include patterns from a `file` (can be specified multiple times)")
-	flags.StringArrayVar(&restoreOptions.InsensitiveIncludeFiles, "iinclude-file", nil, "same as --include-file but ignores casing of `file`names in patterns")
+
+	initExcludePatternOptions(flags, &restoreOptions.excludePatternOptions)
+	initIncludePatternOptions(flags, &restoreOptions.includePatternOptions)
 
 	initSingleSnapshotFilter(flags, &restoreOptions.SnapshotFilter)
 	flags.BoolVar(&restoreOptions.Sparse, "sparse", false, "restore files as sparse")
@@ -83,38 +70,8 @@ func init() {
 func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions,
 	term *termstatus.Terminal, args []string) error {
 
-	hasExcludes := len(opts.Exclude) > 0 || len(opts.InsensitiveExclude) > 0
-	hasIncludes := len(opts.Include) > 0 || len(opts.InsensitiveInclude) > 0
-
-	// Validate provided patterns
-	if len(opts.Exclude) > 0 {
-		if err := filter.ValidatePatterns(opts.Exclude); err != nil {
-			return errors.Fatalf("--exclude: %s", err)
-		}
-	}
-	if len(opts.InsensitiveExclude) > 0 {
-		if err := filter.ValidatePatterns(opts.InsensitiveExclude); err != nil {
-			return errors.Fatalf("--iexclude: %s", err)
-		}
-	}
-	if len(opts.Include) > 0 {
-		if err := filter.ValidatePatterns(opts.Include); err != nil {
-			return errors.Fatalf("--include: %s", err)
-		}
-	}
-	if len(opts.InsensitiveInclude) > 0 {
-		if err := filter.ValidatePatterns(opts.InsensitiveInclude); err != nil {
-			return errors.Fatalf("--iinclude: %s", err)
-		}
-	}
-
-	for i, str := range opts.InsensitiveExclude {
-		opts.InsensitiveExclude[i] = strings.ToLower(str)
-	}
-
-	for i, str := range opts.InsensitiveInclude {
-		opts.InsensitiveInclude[i] = strings.ToLower(str)
-	}
+	hasExcludes := len(opts.Excludes) > 0 || len(opts.InsensitiveExcludes) > 0
+	hasIncludes := len(opts.Includes) > 0 || len(opts.InsensitiveIncludes) > 0
 
 	switch {
 	case len(args) == 0:
@@ -182,94 +139,38 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions,
 		msg.E("Warning: %s\n", message)
 	}
 
-	excludePatterns := filter.ParsePatterns(opts.Exclude)
-	insensitiveExcludePatterns := filter.ParsePatterns(opts.InsensitiveExclude)
-
-	if len(opts.ExcludeFiles) > 0 {
-		patternsFromFile, err := readPatternsFromFiles(opts.ExcludeFiles)
-		if err != nil {
-			return err
-		}
-
-		excludePatternsFromFile := filter.ParsePatterns(patternsFromFile)
-		excludePatterns = append(excludePatterns, excludePatternsFromFile...)
-	}
-
-	if len(opts.InsensitiveExcludeFiles) > 0 {
-		patternsFromFile, err := readPatternsFromFiles(opts.ExcludeFiles)
-		if err != nil {
-			return err
-		}
-
-		for i, str := range patternsFromFile {
-			patternsFromFile[i] = strings.ToLower(str)
-		}
-
-		iexcludePatternsFromFile := filter.ParsePatterns(patternsFromFile)
-		insensitiveExcludePatterns = append(insensitiveExcludePatterns, iexcludePatternsFromFile...)
+	excludePatterns, err := opts.excludePatternOptions.CollectPatterns()
+	if err != nil {
+		return err
 	}
 
 	selectExcludeFilter := func(item string, _ string, node *restic.Node) (selectedForRestore bool, childMayBeSelected bool) {
-		matched, err := filter.List(excludePatterns, item)
-		if err != nil {
-			msg.E("error for exclude pattern: %v", err)
+		for _, rejectFn := range excludePatterns {
+			matched := rejectFn(item)
+
+			// An exclude filter is basically a 'wildcard but foo',
+			// so even if a childMayMatch, other children of a dir may not,
+			// therefore childMayMatch does not matter, but we should not go down
+			// unless the dir is selected for restore
+			selectedForRestore = !matched
+			childMayBeSelected = selectedForRestore && node.Type == "dir"
+
+			return selectedForRestore, childMayBeSelected
 		}
-
-		matchedInsensitive, err := filter.List(insensitiveExcludePatterns, strings.ToLower(item))
-		if err != nil {
-			msg.E("error for iexclude pattern: %v", err)
-		}
-
-		// An exclude filter is basically a 'wildcard but foo',
-		// so even if a childMayMatch, other children of a dir may not,
-		// therefore childMayMatch does not matter, but we should not go down
-		// unless the dir is selected for restore
-		selectedForRestore = !matched && !matchedInsensitive
-		childMayBeSelected = selectedForRestore && node.Type == "dir"
-
 		return selectedForRestore, childMayBeSelected
 	}
 
-	includePatterns := filter.ParsePatterns(opts.Include)
-	insensitiveIncludePatterns := filter.ParsePatterns(opts.InsensitiveInclude)
-
-	if len(opts.IncludeFiles) > 0 {
-		patternsFromFile, err := readPatternsFromFiles(opts.IncludeFiles)
-		if err != nil {
-			return err
-		}
-
-		for i, str := range patternsFromFile {
-			patternsFromFile[i] = strings.ToLower(str)
-		}
-
-		includePatternsFromFile := filter.ParsePatterns(patternsFromFile)
-		includePatterns = append(includePatterns, includePatternsFromFile...)
-	}
-
-	if len(opts.InsensitiveIncludeFiles) > 0 {
-		patternsFromFile, err := readPatternsFromFiles(opts.InsensitiveIncludeFiles)
-		if err != nil {
-			return err
-		}
-
-		iincludePatternsFromFile := filter.ParsePatterns(patternsFromFile)
-		insensitiveIncludePatterns = append(insensitiveIncludePatterns, iincludePatternsFromFile...)
+	includePatterns, err := opts.includePatternOptions.CollectPatterns()
+	if err != nil {
+		return err
 	}
 
 	selectIncludeFilter := func(item string, _ string, node *restic.Node) (selectedForRestore bool, childMayBeSelected bool) {
-		matched, childMayMatch, err := filter.ListWithChild(includePatterns, item)
-		if err != nil {
-			msg.E("error for include pattern: %v", err)
+		for _, includeFn := range includePatterns {
+			selectedForRestore, childMayBeSelected = includeFn(item)
 		}
 
-		matchedInsensitive, childMayMatchInsensitive, err := filter.ListWithChild(insensitiveIncludePatterns, strings.ToLower(item))
-		if err != nil {
-			msg.E("error for iexclude pattern: %v", err)
-		}
-
-		selectedForRestore = matched || matchedInsensitive
-		childMayBeSelected = (childMayMatch || childMayMatchInsensitive) && node.Type == "dir"
+		childMayBeSelected = childMayBeSelected && node.Type == "dir"
 
 		return selectedForRestore, childMayBeSelected
 	}

cmd/restic/cmd_restore_integration_test.go

@@ -24,9 +24,9 @@ func testRunRestore(t testing.TB, opts GlobalOptions, dir string, snapshotID res
 
 func testRunRestoreExcludes(t testing.TB, gopts GlobalOptions, dir string, snapshotID restic.ID, excludes []string) {
 	opts := RestoreOptions{
-		Target:  dir,
-		Exclude: excludes,
+		Target: dir,
 	}
+	opts.Excludes = excludes
 
 	rtest.OK(t, testRunRestoreAssumeFailure(snapshotID.String(), opts, gopts))
 }
@@ -51,13 +51,64 @@ func testRunRestoreLatest(t testing.TB, gopts GlobalOptions, dir string, paths [
 
 func testRunRestoreIncludes(t testing.TB, gopts GlobalOptions, dir string, snapshotID restic.ID, includes []string) {
 	opts := RestoreOptions{
-		Target:  dir,
-		Include: includes,
+		Target: dir,
 	}
+	opts.Includes = includes
 
 	rtest.OK(t, testRunRestoreAssumeFailure(snapshotID.String(), opts, gopts))
 }
 
+func TestRestoreIncludesComplex(t *testing.T) {
+	testfiles := []struct {
+		path    string
+		size    uint
+		include bool // Whether this file should be included in the restore
+	}{
+		{"dir1/include_me.txt", 100, true},
+		{"dir1/something_else.txt", 200, false},
+		{"dir2/also_include_me.txt", 150, true},
+		{"dir2/important_file.txt", 150, true},
+		{"dir3/not_included.txt", 180, false},
+		{"dir4/subdir/should_include_me.txt", 120, true},
+	}
+
+	env, cleanup := withTestEnvironment(t)
+	defer cleanup()
+
+	testRunInit(t, env.gopts)
+
+	// Create test files and directories
+	for _, testFile := range testfiles {
+		fullPath := filepath.Join(env.testdata, testFile.path)
+		rtest.OK(t, os.MkdirAll(filepath.Dir(fullPath), 0755))
+		rtest.OK(t, appendRandomData(fullPath, testFile.size))
+	}
+
+	opts := BackupOptions{}
+
+	// Perform backup
+	testRunBackup(t, filepath.Dir(env.testdata), []string{filepath.Base(env.testdata)}, opts, env.gopts)
+	testRunCheck(t, env.gopts)
+
+	snapshotID := testListSnapshots(t, env.gopts, 1)[0]
+
+	// Restore using includes
+	includePatterns := []string{"dir1/*include_me.txt", "dir2/**", "dir4/**/*_me.txt"}
+	restoredir := filepath.Join(env.base, "restore")
+	testRunRestoreIncludes(t, env.gopts, restoredir, snapshotID, includePatterns)
+
+	// Check that only the included files are restored
+	for _, testFile := range testfiles {
+		restoredFilePath := filepath.Join(restoredir, "testdata", testFile.path)
+		_, err := os.Stat(restoredFilePath)
+		if testFile.include {
+			rtest.OK(t, err)
+		} else {
+			rtest.Assert(t, os.IsNotExist(err), "File %s should not have been restored", testFile.path)
+		}
+	}
+}
+
 func TestRestoreFilter(t *testing.T) {
 	testfiles := []struct {
 		name string

cmd/restic/include.go

@@ -0,0 +1,100 @@
+package main
+
+import (
+	"strings"
+
+	"github.com/restic/restic/internal/errors"
+	"github.com/restic/restic/internal/filter"
+	"github.com/spf13/pflag"
+)
+
+// IncludeByNameFunc is a function that takes a filename that should be included
+// in the restore process and returns whether it should be included.
+type IncludeByNameFunc func(item string) (matched bool, childMayMatch bool)
+
+type includePatternOptions struct {
+	Includes                []string
+	InsensitiveIncludes     []string
+	IncludeFiles            []string
+	InsensitiveIncludeFiles []string
+}
+
+func initIncludePatternOptions(f *pflag.FlagSet, opts *includePatternOptions) {
+	f.StringArrayVarP(&opts.Includes, "include", "i", nil, "include a `pattern` (can be specified multiple times)")
+	f.StringArrayVar(&opts.InsensitiveIncludes, "iinclude", nil, "same as --include `pattern` but ignores the casing of filenames")
+	f.StringArrayVar(&opts.IncludeFiles, "include-file", nil, "read include patterns from a `file` (can be specified multiple times)")
+	f.StringArrayVar(&opts.InsensitiveIncludeFiles, "iinclude-file", nil, "same as --include-file but ignores casing of `file`names in patterns")
+}
+
+func (opts includePatternOptions) CollectPatterns() ([]IncludeByNameFunc, error) {
+	var fs []IncludeByNameFunc
+	if len(opts.IncludeFiles) > 0 {
+		includePatterns, err := readPatternsFromFiles(opts.IncludeFiles)
+		if err != nil {
+			return nil, err
+		}
+
+		if err := filter.ValidatePatterns(includePatterns); err != nil {
+			return nil, errors.Fatalf("--include-file: %s", err)
+		}
+
+		opts.Includes = append(opts.Includes, includePatterns...)
+	}
+
+	if len(opts.InsensitiveIncludeFiles) > 0 {
+		includePatterns, err := readPatternsFromFiles(opts.InsensitiveIncludeFiles)
+		if err != nil {
+			return nil, err
+		}
+
+		if err := filter.ValidatePatterns(includePatterns); err != nil {
+			return nil, errors.Fatalf("--iinclude-file: %s", err)
+		}
+
+		opts.InsensitiveIncludes = append(opts.InsensitiveIncludes, includePatterns...)
+	}
+
+	if len(opts.InsensitiveIncludes) > 0 {
+		if err := filter.ValidatePatterns(opts.InsensitiveIncludes); err != nil {
+			return nil, errors.Fatalf("--iinclude: %s", err)
+		}
+
+		fs = append(fs, includeByInsensitivePattern(opts.InsensitiveIncludes))
+	}
+
+	if len(opts.Includes) > 0 {
+		if err := filter.ValidatePatterns(opts.Includes); err != nil {
+			return nil, errors.Fatalf("--include: %s", err)
+		}
+
+		fs = append(fs, includeByPattern(opts.Includes))
+	}
+	return fs, nil
+}
+
+// includeByPattern returns a IncludeByNameFunc which includes files that match
+// one of the patterns.
+func includeByPattern(patterns []string) IncludeByNameFunc {
+	parsedPatterns := filter.ParsePatterns(patterns)
+	return func(item string) (matched bool, childMayMatch bool) {
+		matched, childMayMatch, err := filter.ListWithChild(parsedPatterns, item)
+		if err != nil {
+			Warnf("error for include pattern: %v", err)
+		}
+
+		return matched, childMayMatch
+	}
+}
+
+// includeByInsensitivePattern returns a IncludeByNameFunc which includes files that match
+// one of the patterns, ignoring the casing of the filenames.
+func includeByInsensitivePattern(patterns []string) IncludeByNameFunc {
+	for index, path := range patterns {
+		patterns[index] = strings.ToLower(path)
+	}
+
+	includeFunc := includeByPattern(patterns)
+	return func(item string) (matched bool, childMayMatch bool) {
+		return includeFunc(strings.ToLower(item))
+	}
+}

cmd/restic/integration_filter_pattern_test.go

@@ -70,28 +70,28 @@ func TestRestoreFailsWhenUsingInvalidPatterns(t *testing.T) {
 	var err error
 
 	// Test --exclude
-	err = testRunRestoreAssumeFailure("latest", RestoreOptions{Exclude: []string{"*[._]log[.-][0-9]", "!*[._]log[.-][0-9]"}}, env.gopts)
+	err = testRunRestoreAssumeFailure("latest", RestoreOptions{excludePatternOptions: excludePatternOptions{Excludes: []string{"*[._]log[.-][0-9]", "!*[._]log[.-][0-9]"}}}, env.gopts)
 
 	rtest.Equals(t, `Fatal: --exclude: invalid pattern(s) provided:
 *[._]log[.-][0-9]
 !*[._]log[.-][0-9]`, err.Error())
 
 	// Test --iexclude
-	err = testRunRestoreAssumeFailure("latest", RestoreOptions{InsensitiveExclude: []string{"*[._]log[.-][0-9]", "!*[._]log[.-][0-9]"}}, env.gopts)
+	err = testRunRestoreAssumeFailure("latest", RestoreOptions{excludePatternOptions: excludePatternOptions{InsensitiveExcludes: []string{"*[._]log[.-][0-9]", "!*[._]log[.-][0-9]"}}}, env.gopts)
 
 	rtest.Equals(t, `Fatal: --iexclude: invalid pattern(s) provided:
 *[._]log[.-][0-9]
 !*[._]log[.-][0-9]`, err.Error())
 
 	// Test --include
-	err = testRunRestoreAssumeFailure("latest", RestoreOptions{Include: []string{"*[._]log[.-][0-9]", "!*[._]log[.-][0-9]"}}, env.gopts)
+	err = testRunRestoreAssumeFailure("latest", RestoreOptions{includePatternOptions: includePatternOptions{Includes: []string{"*[._]log[.-][0-9]", "!*[._]log[.-][0-9]"}}}, env.gopts)
 
 	rtest.Equals(t, `Fatal: --include: invalid pattern(s) provided:
 *[._]log[.-][0-9]
 !*[._]log[.-][0-9]`, err.Error())
 
 	// Test --iinclude
-	err = testRunRestoreAssumeFailure("latest", RestoreOptions{InsensitiveInclude: []string{"*[._]log[.-][0-9]", "!*[._]log[.-][0-9]"}}, env.gopts)
+	err = testRunRestoreAssumeFailure("latest", RestoreOptions{includePatternOptions: includePatternOptions{InsensitiveIncludes: []string{"*[._]log[.-][0-9]", "!*[._]log[.-][0-9]"}}}, env.gopts)
 
 	rtest.Equals(t, `Fatal: --iinclude: invalid pattern(s) provided:
 *[._]log[.-][0-9]