From b02117ef0b9aeb06781e470b45480e0705be938a Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Sun, 19 May 2024 23:30:14 +0530 Subject: [PATCH 01/12] restore: read includes, insensitive includes, excludes and insensitive excludes from a file feature for gh-4781 --- cmd/restic/cmd_restore.go | 60 +++++++++++++++++++++++++++++++++++---- cmd/restic/exclude.go | 20 ++++++------- 2 files changed, 65 insertions(+), 15 deletions(-) diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index 5161be50d..f8a62c360 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -45,11 +45,15 @@ 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 - InsensitiveExclude []string - Include []string - InsensitiveInclude []string - Target string + Exclude []string + ExcludeFiles []string + InsensitiveExclude []string + InsensitiveExcludeFiles []string + Include []string + IncludeFiles []string + InsensitiveInclude []string + InsensitiveIncludeFiles []string + Target string restic.SnapshotFilter Sparse bool Verify bool @@ -66,6 +70,10 @@ func init() { 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") initSingleSnapshotFilter(flags, &restoreOptions.SnapshotFilter) flags.BoolVar(&restoreOptions.Sparse, "sparse", false, "restore files as sparse") @@ -176,6 +184,27 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, 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 + } + + iexcludePatternsFromFile := filter.ParsePatterns(patternsFromFile) + insensitiveExcludePatterns = append(insensitiveExcludePatterns, iexcludePatternsFromFile...) + } + selectExcludeFilter := func(item string, _ string, node *restic.Node) (selectedForRestore bool, childMayBeSelected bool) { matched, err := filter.List(excludePatterns, item) if err != nil { @@ -199,6 +228,27 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, 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 + } + + 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...) + } + selectIncludeFilter := func(item string, _ string, node *restic.Node) (selectedForRestore bool, childMayBeSelected bool) { matched, childMayMatch, err := filter.ListWithChild(includePatterns, item) if err != nil { diff --git a/cmd/restic/exclude.go b/cmd/restic/exclude.go index d9bb63aeb..4657e4915 100644 --- a/cmd/restic/exclude.go +++ b/cmd/restic/exclude.go @@ -385,12 +385,12 @@ func rejectBySize(maxSizeStr string) (RejectFunc, error) { }, nil } -// readExcludePatternsFromFiles reads all exclude files and returns the list of -// exclude patterns. For each line, leading and trailing white space is removed +// readPatternsFromFiles reads all files and returns the list of +// patterns. For each line, leading and trailing white space is removed // and comment lines are ignored. For each remaining pattern, environment // variables are resolved. For adding a literal dollar sign ($), write $$ to // the file. -func readExcludePatternsFromFiles(excludeFiles []string) ([]string, error) { +func readPatternsFromFiles(files []string) ([]string, error) { getenvOrDollar := func(s string) string { if s == "$" { return "$" @@ -398,8 +398,8 @@ func readExcludePatternsFromFiles(excludeFiles []string) ([]string, error) { return os.Getenv(s) } - var excludes []string - for _, filename := range excludeFiles { + var patterns []string + for _, filename := range files { err := func() (err error) { data, err := textfile.Read(filename) if err != nil { @@ -421,15 +421,15 @@ func readExcludePatternsFromFiles(excludeFiles []string) ([]string, error) { } line = os.Expand(line, getenvOrDollar) - excludes = append(excludes, line) + patterns = append(patterns, line) } return scanner.Err() }() if err != nil { - return nil, fmt.Errorf("failed to read excludes from file %q: %w", filename, err) + return nil, fmt.Errorf("failed to read patterns from file %q: %w", filename, err) } } - return excludes, nil + return patterns, nil } type excludePatternOptions struct { @@ -454,7 +454,7 @@ func (opts excludePatternOptions) CollectPatterns() ([]RejectByNameFunc, error) var fs []RejectByNameFunc // add patterns from file if len(opts.ExcludeFiles) > 0 { - excludePatterns, err := readExcludePatternsFromFiles(opts.ExcludeFiles) + excludePatterns, err := readPatternsFromFiles(opts.ExcludeFiles) if err != nil { return nil, err } @@ -467,7 +467,7 @@ func (opts excludePatternOptions) CollectPatterns() ([]RejectByNameFunc, error) } if len(opts.InsensitiveExcludeFiles) > 0 { - excludes, err := readExcludePatternsFromFiles(opts.InsensitiveExcludeFiles) + excludes, err := readPatternsFromFiles(opts.InsensitiveExcludeFiles) if err != nil { return nil, err } From 7d5dd6db669ddd99dd7c58a8621f9e0ed3def400 Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Sun, 19 May 2024 23:40:20 +0530 Subject: [PATCH 02/12] fix: add string.Lower for insenstive includes and excludes read from file --- cmd/restic/cmd_restore.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index f8a62c360..f71133a0a 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -201,6 +201,10 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, return err } + for i, str := range patternsFromFile { + patternsFromFile[i] = strings.ToLower(str) + } + iexcludePatternsFromFile := filter.ParsePatterns(patternsFromFile) insensitiveExcludePatterns = append(insensitiveExcludePatterns, iexcludePatternsFromFile...) } @@ -235,6 +239,10 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, return err } + for i, str := range patternsFromFile { + patternsFromFile[i] = strings.ToLower(str) + } + includePatternsFromFile := filter.ParsePatterns(patternsFromFile) includePatterns = append(includePatterns, includePatternsFromFile...) } From fdf2e4ed0ebe5c1af33b0b23fb9aade3060ef075 Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Sat, 1 Jun 2024 17:49:15 +0530 Subject: [PATCH 03/12] 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 | 155 ++++-------------- cmd/restic/cmd_restore_integration_test.go | 59 ++++++- cmd/restic/include.go | 100 +++++++++++ cmd/restic/integration_filter_pattern_test.go | 8 +- 4 files changed, 187 insertions(+), 135 deletions(-) create mode 100644 cmd/restic/include.go diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index f71133a0a..e833f7c83 100644 --- a/cmd/restic/cmd_restore.go +++ b/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 } diff --git a/cmd/restic/cmd_restore_integration_test.go b/cmd/restic/cmd_restore_integration_test.go index 8da6f522a..483cacc7f 100644 --- a/cmd/restic/cmd_restore_integration_test.go +++ b/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 diff --git a/cmd/restic/include.go b/cmd/restic/include.go new file mode 100644 index 000000000..dcc4c7f37 --- /dev/null +++ b/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)) + } +} diff --git a/cmd/restic/integration_filter_pattern_test.go b/cmd/restic/integration_filter_pattern_test.go index 2eacdeea9..f8da7592e 100644 --- a/cmd/restic/integration_filter_pattern_test.go +++ b/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] From 14d2799b44b08587ab1695ae42a5e602ec7dae04 Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Sat, 1 Jun 2024 18:04:14 +0530 Subject: [PATCH 04/12] fix: move include and exclude pattern validations to top --- cmd/restic/cmd_restore.go | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index e833f7c83..44394769c 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -73,6 +73,16 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, hasExcludes := len(opts.Excludes) > 0 || len(opts.InsensitiveExcludes) > 0 hasIncludes := len(opts.Includes) > 0 || len(opts.InsensitiveIncludes) > 0 + excludePatterns, err := opts.excludePatternOptions.CollectPatterns() + if err != nil { + return err + } + + includePatterns, err := opts.includePatternOptions.CollectPatterns() + if err != nil { + return err + } + switch { case len(args) == 0: return errors.Fatal("no snapshot ID specified") @@ -139,11 +149,6 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, msg.E("Warning: %s\n", message) } - excludePatterns, err := opts.excludePatternOptions.CollectPatterns() - if err != nil { - return err - } - selectExcludeFilter := func(item string, _ string, node *restic.Node) (selectedForRestore bool, childMayBeSelected bool) { for _, rejectFn := range excludePatterns { matched := rejectFn(item) @@ -160,11 +165,6 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, return selectedForRestore, childMayBeSelected } - includePatterns, err := opts.includePatternOptions.CollectPatterns() - if err != nil { - return err - } - selectIncludeFilter := func(item string, _ string, node *restic.Node) (selectedForRestore bool, childMayBeSelected bool) { for _, includeFn := range includePatterns { selectedForRestore, childMayBeSelected = includeFn(item) From 4e449ffaff483092c421b8a3a3d2b5137620bf13 Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Sat, 1 Jun 2024 19:46:12 +0530 Subject: [PATCH 05/12] test: add tests for reading patterns from file --- cmd/restic/cmd_restore.go | 8 +-- cmd/restic/integration_filter_pattern_test.go | 55 +++++++++++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index 44394769c..3ce2c8649 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -73,12 +73,12 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, hasExcludes := len(opts.Excludes) > 0 || len(opts.InsensitiveExcludes) > 0 hasIncludes := len(opts.Includes) > 0 || len(opts.InsensitiveIncludes) > 0 - excludePatterns, err := opts.excludePatternOptions.CollectPatterns() + excludePatternFns, err := opts.excludePatternOptions.CollectPatterns() if err != nil { return err } - includePatterns, err := opts.includePatternOptions.CollectPatterns() + includePatternFns, err := opts.includePatternOptions.CollectPatterns() if err != nil { return err } @@ -150,7 +150,7 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, } selectExcludeFilter := func(item string, _ string, node *restic.Node) (selectedForRestore bool, childMayBeSelected bool) { - for _, rejectFn := range excludePatterns { + for _, rejectFn := range excludePatternFns { matched := rejectFn(item) // An exclude filter is basically a 'wildcard but foo', @@ -166,7 +166,7 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, } selectIncludeFilter := func(item string, _ string, node *restic.Node) (selectedForRestore bool, childMayBeSelected bool) { - for _, includeFn := range includePatterns { + for _, includeFn := range includePatternFns { selectedForRestore, childMayBeSelected = includeFn(item) } diff --git a/cmd/restic/integration_filter_pattern_test.go b/cmd/restic/integration_filter_pattern_test.go index f8da7592e..0f547bdc2 100644 --- a/cmd/restic/integration_filter_pattern_test.go +++ b/cmd/restic/integration_filter_pattern_test.go @@ -97,3 +97,58 @@ func TestRestoreFailsWhenUsingInvalidPatterns(t *testing.T) { *[._]log[.-][0-9] !*[._]log[.-][0-9]`, err.Error()) } + +func TestRestoreFailsWhenUsingInvalidPatternsFromFile(t *testing.T) { + env, cleanup := withTestEnvironment(t) + defer cleanup() + + testRunInit(t, env.gopts) + + // Create an include file with some invalid patterns + includeFile := env.base + "/includefile" + fileErr := os.WriteFile(includeFile, []byte("*.go\n*[._]log[.-][0-9]\n!*[._]log[.-][0-9]"), 0644) + if fileErr != nil { + t.Fatalf("Could not write include file: %v", fileErr) + } + + err := testRunRestoreAssumeFailure("latest", RestoreOptions{includePatternOptions: includePatternOptions{IncludeFiles: []string{includeFile}}}, env.gopts) + rtest.Equals(t, `Fatal: --include-file: invalid pattern(s) provided: +*[._]log[.-][0-9] +!*[._]log[.-][0-9]`, err.Error()) + + // Create an exclude file with some invalid patterns + excludeFile := env.base + "/excludefile" + fileErr = os.WriteFile(excludeFile, []byte("*.go\n*[._]log[.-][0-9]\n!*[._]log[.-][0-9]"), 0644) + if fileErr != nil { + t.Fatalf("Could not write exclude file: %v", fileErr) + } + + err = testRunRestoreAssumeFailure("latest", RestoreOptions{excludePatternOptions: excludePatternOptions{ExcludeFiles: []string{excludeFile}}}, env.gopts) + rtest.Equals(t, `Fatal: --exclude-file: invalid pattern(s) provided: +*[._]log[.-][0-9] +!*[._]log[.-][0-9]`, err.Error()) + + // Create an insentive include file with some invalid patterns + insensitiveIncludeFile := env.base + "/insensitiveincludefile" + fileErr = os.WriteFile(insensitiveIncludeFile, []byte("*.go\n*[._]log[.-][0-9]\n!*[._]log[.-][0-9]"), 0644) + if fileErr != nil { + t.Fatalf("Could not write insensitive include file: %v", fileErr) + } + + err = testRunRestoreAssumeFailure("latest", RestoreOptions{includePatternOptions: includePatternOptions{InsensitiveIncludeFiles: []string{insensitiveIncludeFile}}}, env.gopts) + rtest.Equals(t, `Fatal: --iinclude-file: invalid pattern(s) provided: +*[._]log[.-][0-9] +!*[._]log[.-][0-9]`, err.Error()) + + // Create an insensitive exclude file with some invalid patterns + insensitiveExcludeFile := env.base + "/insensitiveexcludefile" + fileErr = os.WriteFile(insensitiveExcludeFile, []byte("*.go\n*[._]log[.-][0-9]\n!*[._]log[.-][0-9]"), 0644) + if fileErr != nil { + t.Fatalf("Could not write insensitive exclude file: %v", fileErr) + } + + err = testRunRestoreAssumeFailure("latest", RestoreOptions{excludePatternOptions: excludePatternOptions{InsensitiveExcludeFiles: []string{insensitiveExcludeFile}}}, env.gopts) + rtest.Equals(t, `Fatal: --iexclude-file: invalid pattern(s) provided: +*[._]log[.-][0-9] +!*[._]log[.-][0-9]`, err.Error()) +} From 1a7574e4b4555c25a548a60d1df2be0235bc840f Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Sat, 1 Jun 2024 20:02:16 +0530 Subject: [PATCH 06/12] test: add tests for include By pattern --- cmd/restic/include_test.go | 59 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 cmd/restic/include_test.go diff --git a/cmd/restic/include_test.go b/cmd/restic/include_test.go new file mode 100644 index 000000000..751bfbb76 --- /dev/null +++ b/cmd/restic/include_test.go @@ -0,0 +1,59 @@ +package main + +import ( + "testing" +) + +func TestIncludeByPattern(t *testing.T) { + var tests = []struct { + filename string + include bool + }{ + {filename: "/home/user/foo.go", include: true}, + {filename: "/home/user/foo.c", include: false}, + {filename: "/home/user/foobar", include: false}, + {filename: "/home/user/foobar/x", include: false}, + {filename: "/home/user/README", include: false}, + {filename: "/home/user/README.md", include: true}, + } + + patterns := []string{"*.go", "README.md"} + + for _, tc := range tests { + t.Run(tc.filename, func(t *testing.T) { + includeFunc := includeByPattern(patterns) + matched, _ := includeFunc(tc.filename) + if matched != tc.include { + t.Fatalf("wrong result for filename %v: want %v, got %v", + tc.filename, tc.include, matched) + } + }) + } +} + +func TestIncludeByInsensitivePattern(t *testing.T) { + var tests = []struct { + filename string + include bool + }{ + {filename: "/home/user/foo.GO", include: true}, + {filename: "/home/user/foo.c", include: false}, + {filename: "/home/user/foobar", include: false}, + {filename: "/home/user/FOObar/x", include: false}, + {filename: "/home/user/README", include: false}, + {filename: "/home/user/readme.MD", include: true}, + } + + patterns := []string{"*.go", "README.md"} + + for _, tc := range tests { + t.Run(tc.filename, func(t *testing.T) { + includeFunc := includeByInsensitivePattern(patterns) + matched, _ := includeFunc(tc.filename) + if matched != tc.include { + t.Fatalf("wrong result for filename %v: want %v, got %v", + tc.filename, tc.include, matched) + } + }) + } +} From 2a2c09e6660e55b1859392236c30b00b939cde92 Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Sat, 1 Jun 2024 20:03:01 +0530 Subject: [PATCH 07/12] changelog: update changelog for gh-4781 --- changelog/unreleased/issue-4781 | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changelog/unreleased/issue-4781 diff --git a/changelog/unreleased/issue-4781 b/changelog/unreleased/issue-4781 new file mode 100644 index 000000000..b4af20885 --- /dev/null +++ b/changelog/unreleased/issue-4781 @@ -0,0 +1,8 @@ +Enhancement: Add restore flags to read include and exclude patterns from files + +Restic now supports reading include and exclude patterns from files using the +`--include-file`, `--exclude-file`, `--iinclude-file` and `--iexclude-file` +flags. + +https://github.com/restic/restic/issues/4781 +https://github.com/restic/restic/pull/4811 \ No newline at end of file From e579dfe72aba472256b9733909ef8021361caec7 Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Sat, 1 Jun 2024 20:24:53 +0530 Subject: [PATCH 08/12] doc: update documentation for restore command --- doc/050_restore.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/050_restore.rst b/doc/050_restore.rst index ce17a1cf7..5fc94c145 100644 --- a/doc/050_restore.rst +++ b/doc/050_restore.rst @@ -68,6 +68,9 @@ There are case insensitive variants of ``--exclude`` and ``--include`` called ``--iexclude`` and ``--iinclude``. These options will behave the same way but ignore the casing of paths. +There are also ``--include-file``, ``--exclude-file``, ``--iinclude-file`` and + ``--iexclude-file`` flags that read the include and exclude patterns from a file. + Restoring symbolic links on windows is only possible when the user has ``SeCreateSymbolicLinkPrivilege`` privilege or is running as admin. This is a restriction of windows not restic. From 3a521761214995dcefedabdb329dacd67cbf217d Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Sat, 1 Jun 2024 20:37:52 +0530 Subject: [PATCH 09/12] restore: accumulate results of multiple pattern checks addressing review comments --- cmd/restic/cmd_restore.go | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index 3ce2c8649..1a342800a 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -150,24 +150,27 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, } selectExcludeFilter := func(item string, _ string, node *restic.Node) (selectedForRestore bool, childMayBeSelected bool) { + matched := false for _, rejectFn := range excludePatternFns { - 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 + matched = 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 } selectIncludeFilter := func(item string, _ string, node *restic.Node) (selectedForRestore bool, childMayBeSelected bool) { + selectedForRestore = false + childMayBeSelected = false for _, includeFn := range includePatternFns { - selectedForRestore, childMayBeSelected = includeFn(item) + matched, childMayMatch := includeFn(item) + selectedForRestore = selectedForRestore || matched + childMayBeSelected = childMayBeSelected || childMayMatch } childMayBeSelected = childMayBeSelected && node.Type == "dir" From 24a247a0dc26cb2fa397d67e398633e610c75945 Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Sat, 8 Jun 2024 13:21:34 +0530 Subject: [PATCH 10/12] test: update test case name --- cmd/restic/cmd_restore_integration_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/restic/cmd_restore_integration_test.go b/cmd/restic/cmd_restore_integration_test.go index 483cacc7f..27272fe40 100644 --- a/cmd/restic/cmd_restore_integration_test.go +++ b/cmd/restic/cmd_restore_integration_test.go @@ -58,7 +58,7 @@ func testRunRestoreIncludes(t testing.TB, gopts GlobalOptions, dir string, snaps rtest.OK(t, testRunRestoreAssumeFailure(snapshotID.String(), opts, gopts)) } -func TestRestoreIncludesComplex(t *testing.T) { +func TestRestoreIncludes(t *testing.T) { testfiles := []struct { path string size uint From 188684ee9e7c119d9e3abd07ba2c210fc1a680a0 Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Sun, 9 Jun 2024 19:07:23 +0530 Subject: [PATCH 11/12] fix: include and exclude logic, add tests for include file and exclude file --- cmd/restic/cmd_restore.go | 20 ++- cmd/restic/cmd_restore_integration_test.go | 134 +++++++++++++++--- cmd/restic/exclude.go | 2 +- cmd/restic/include.go | 2 +- cmd/restic/integration_filter_pattern_test.go | 33 +---- 5 files changed, 139 insertions(+), 52 deletions(-) diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index 1a342800a..03a990cc6 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -70,9 +70,6 @@ func init() { func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, term *termstatus.Terminal, args []string) error { - hasExcludes := len(opts.Excludes) > 0 || len(opts.InsensitiveExcludes) > 0 - hasIncludes := len(opts.Includes) > 0 || len(opts.InsensitiveIncludes) > 0 - excludePatternFns, err := opts.excludePatternOptions.CollectPatterns() if err != nil { return err @@ -83,6 +80,9 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, return err } + hasExcludes := len(excludePatternFns) > 0 + hasIncludes := len(includePatternFns) > 0 + switch { case len(args) == 0: return errors.Fatal("no snapshot ID specified") @@ -153,6 +153,13 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, matched := false for _, rejectFn := range excludePatternFns { matched = matched || rejectFn(item) + + // implementing a short-circuit here to improve the performance + // to prevent additional pattern matching once the first pattern + // matches. + if matched { + break + } } // An exclude filter is basically a 'wildcard but foo', // so even if a childMayMatch, other children of a dir may not, @@ -171,9 +178,12 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, matched, childMayMatch := includeFn(item) selectedForRestore = selectedForRestore || matched childMayBeSelected = childMayBeSelected || childMayMatch - } + childMayBeSelected = childMayBeSelected && node.Type == "dir" - childMayBeSelected = childMayBeSelected && node.Type == "dir" + if selectedForRestore || childMayBeSelected { + break + } + } return selectedForRestore, childMayBeSelected } diff --git a/cmd/restic/cmd_restore_integration_test.go b/cmd/restic/cmd_restore_integration_test.go index 27272fe40..0e1620f7e 100644 --- a/cmd/restic/cmd_restore_integration_test.go +++ b/cmd/restic/cmd_restore_integration_test.go @@ -7,6 +7,7 @@ import ( "math/rand" "os" "path/filepath" + "strings" "syscall" "testing" "time" @@ -58,6 +59,67 @@ func testRunRestoreIncludes(t testing.TB, gopts GlobalOptions, dir string, snaps rtest.OK(t, testRunRestoreAssumeFailure(snapshotID.String(), opts, gopts)) } +func testRunRestoreIncludesFromFile(t testing.TB, gopts GlobalOptions, dir string, snapshotID restic.ID, includesFile string) { + opts := RestoreOptions{ + Target: dir, + } + opts.IncludeFiles = []string{includesFile} + + rtest.OK(t, testRunRestoreAssumeFailure(snapshotID.String(), opts, gopts)) +} + +func testRunRestoreExcludesFromFile(t testing.TB, gopts GlobalOptions, dir string, snapshotID restic.ID, excludesFile string) { + opts := RestoreOptions{ + Target: dir, + } + opts.ExcludeFiles = []string{excludesFile} + + rtest.OK(t, testRunRestoreAssumeFailure(snapshotID.String(), opts, gopts)) +} + +func TestRestoreMustFailWhenUsingBothIncludesAndExcludes(t *testing.T) { + testfiles := []struct { + path string + size uint + }{ + {"dir1/include_me.txt", 100}, + } + + 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] + + // Add both include and exclude patterns + includePatterns := []string{"dir1/*include_me.txt", "dir2/**", "dir4/**/*_me.txt"} + excludePatterns := []string{"dir1/*include_me.txt", "dir2/**", "dir4/**/*_me.txt"} + + restoredir := filepath.Join(env.base, "restore") + + restoreOpts := RestoreOptions{ + Target: restoredir, + } + restoreOpts.Includes = includePatterns + restoreOpts.Excludes = excludePatterns + + err := testRunRestoreAssumeFailure(snapshotID.String(), restoreOpts, env.gopts) + rtest.Assert(t, err != nil, "restore must fail if include and exclude patterns are provided") +} + func TestRestoreIncludes(t *testing.T) { testfiles := []struct { path string @@ -97,16 +159,33 @@ func TestRestoreIncludes(t *testing.T) { 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) + testRestoreFileInclusions := func(t *testing.T, env *testEnvironment, includePatterns []string) { + // 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) + } } } + + testRestoreFileInclusions(t, env, includePatterns) + + // Create an include file with some patterns + patternsFile := env.base + "/patternsFile" + fileErr := os.WriteFile(patternsFile, []byte(strings.Join(includePatterns, "\n")), 0644) + if fileErr != nil { + t.Fatalf("Could not write include file: %v", fileErr) + } + + restoredir = filepath.Join(env.base, "restore-include-from-file") + + testRunRestoreIncludesFromFile(t, env.gopts, restoredir, snapshotID, patternsFile) + + testRestoreFileInclusions(t, env, includePatterns) } func TestRestoreFilter(t *testing.T) { @@ -144,19 +223,38 @@ func TestRestoreFilter(t *testing.T) { rtest.OK(t, testFileSize(filepath.Join(env.base, "restore0", "testdata", testFile.name), int64(testFile.size))) } - for i, pat := range []string{"*.c", "*.exe", "*", "*file3*"} { - base := filepath.Join(env.base, fmt.Sprintf("restore%d", i+1)) - testRunRestoreExcludes(t, env.gopts, base, snapshotID, []string{pat}) - for _, testFile := range testfiles { - err := testFileSize(filepath.Join(base, "testdata", testFile.name), int64(testFile.size)) - if ok, _ := filter.Match(pat, filepath.Base(testFile.name)); !ok { - rtest.OK(t, err) - } else { - rtest.Assert(t, os.IsNotExist(err), - "expected %v to not exist in restore step %v, but it exists, err %v", testFile.name, i+1, err) + excludePatterns := []string{"*.c", "*.exe", "*", "*file3*"} + + testRestoreFileExclusions := func(t *testing.T, env *testEnvironment, excludePatterns []string) { + for i, pat := range excludePatterns { + base := filepath.Join(env.base, fmt.Sprintf("restore%d", i+1)) + testRunRestoreExcludes(t, env.gopts, base, snapshotID, []string{pat}) + for _, testFile := range testfiles { + err := testFileSize(filepath.Join(base, "testdata", testFile.name), int64(testFile.size)) + if ok, _ := filter.Match(pat, filepath.Base(testFile.name)); !ok { + rtest.OK(t, err) + } else { + rtest.Assert(t, os.IsNotExist(err), + "expected %v to not exist in restore step %v, but it exists, err %v", testFile.name, i+1, err) + } } } } + + testRestoreFileExclusions(t, env, excludePatterns) + + // Create an include file with some patterns + patternsFile := env.base + "/patternsFile" + fileErr := os.WriteFile(patternsFile, []byte(strings.Join(excludePatterns, "\n")), 0644) + if fileErr != nil { + t.Fatalf("Could not write include file: %v", fileErr) + } + + restoredir := filepath.Join(env.base, "restore-exclude-from-file") + + testRunRestoreExcludesFromFile(t, env.gopts, restoredir, snapshotID, patternsFile) + + testRestoreFileExclusions(t, env, excludePatterns) } func TestRestore(t *testing.T) { diff --git a/cmd/restic/exclude.go b/cmd/restic/exclude.go index 4657e4915..9f5f40511 100644 --- a/cmd/restic/exclude.go +++ b/cmd/restic/exclude.go @@ -443,7 +443,7 @@ func initExcludePatternOptions(f *pflag.FlagSet, opts *excludePatternOptions) { f.StringArrayVarP(&opts.Excludes, "exclude", "e", nil, "exclude a `pattern` (can be specified multiple times)") f.StringArrayVar(&opts.InsensitiveExcludes, "iexclude", nil, "same as --exclude `pattern` but ignores the casing of filenames") f.StringArrayVar(&opts.ExcludeFiles, "exclude-file", nil, "read exclude patterns from a `file` (can be specified multiple times)") - f.StringArrayVar(&opts.InsensitiveExcludeFiles, "iexclude-file", nil, "same as --exclude-file but ignores casing of `file`names in patterns") + f.StringArrayVar(&opts.InsensitiveExcludeFiles, "iexclude-file", nil, "same as --exclude-file but ignores casing of filenames in patterns") } func (opts *excludePatternOptions) Empty() bool { diff --git a/cmd/restic/include.go b/cmd/restic/include.go index dcc4c7f37..64659d98f 100644 --- a/cmd/restic/include.go +++ b/cmd/restic/include.go @@ -23,7 +23,7 @@ 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") + f.StringArrayVar(&opts.InsensitiveIncludeFiles, "iinclude-file", nil, "same as --include-file but ignores casing of filenames in patterns") } func (opts includePatternOptions) CollectPatterns() ([]IncludeByNameFunc, error) { diff --git a/cmd/restic/integration_filter_pattern_test.go b/cmd/restic/integration_filter_pattern_test.go index 0f547bdc2..dccbcc0a0 100644 --- a/cmd/restic/integration_filter_pattern_test.go +++ b/cmd/restic/integration_filter_pattern_test.go @@ -105,49 +105,28 @@ func TestRestoreFailsWhenUsingInvalidPatternsFromFile(t *testing.T) { testRunInit(t, env.gopts) // Create an include file with some invalid patterns - includeFile := env.base + "/includefile" - fileErr := os.WriteFile(includeFile, []byte("*.go\n*[._]log[.-][0-9]\n!*[._]log[.-][0-9]"), 0644) + patternsFile := env.base + "/patternsFile" + fileErr := os.WriteFile(patternsFile, []byte("*.go\n*[._]log[.-][0-9]\n!*[._]log[.-][0-9]"), 0644) if fileErr != nil { t.Fatalf("Could not write include file: %v", fileErr) } - err := testRunRestoreAssumeFailure("latest", RestoreOptions{includePatternOptions: includePatternOptions{IncludeFiles: []string{includeFile}}}, env.gopts) + err := testRunRestoreAssumeFailure("latest", RestoreOptions{includePatternOptions: includePatternOptions{IncludeFiles: []string{patternsFile}}}, env.gopts) rtest.Equals(t, `Fatal: --include-file: invalid pattern(s) provided: *[._]log[.-][0-9] !*[._]log[.-][0-9]`, err.Error()) - // Create an exclude file with some invalid patterns - excludeFile := env.base + "/excludefile" - fileErr = os.WriteFile(excludeFile, []byte("*.go\n*[._]log[.-][0-9]\n!*[._]log[.-][0-9]"), 0644) - if fileErr != nil { - t.Fatalf("Could not write exclude file: %v", fileErr) - } - - err = testRunRestoreAssumeFailure("latest", RestoreOptions{excludePatternOptions: excludePatternOptions{ExcludeFiles: []string{excludeFile}}}, env.gopts) + err = testRunRestoreAssumeFailure("latest", RestoreOptions{excludePatternOptions: excludePatternOptions{ExcludeFiles: []string{patternsFile}}}, env.gopts) rtest.Equals(t, `Fatal: --exclude-file: invalid pattern(s) provided: *[._]log[.-][0-9] !*[._]log[.-][0-9]`, err.Error()) - // Create an insentive include file with some invalid patterns - insensitiveIncludeFile := env.base + "/insensitiveincludefile" - fileErr = os.WriteFile(insensitiveIncludeFile, []byte("*.go\n*[._]log[.-][0-9]\n!*[._]log[.-][0-9]"), 0644) - if fileErr != nil { - t.Fatalf("Could not write insensitive include file: %v", fileErr) - } - - err = testRunRestoreAssumeFailure("latest", RestoreOptions{includePatternOptions: includePatternOptions{InsensitiveIncludeFiles: []string{insensitiveIncludeFile}}}, env.gopts) + err = testRunRestoreAssumeFailure("latest", RestoreOptions{includePatternOptions: includePatternOptions{InsensitiveIncludeFiles: []string{patternsFile}}}, env.gopts) rtest.Equals(t, `Fatal: --iinclude-file: invalid pattern(s) provided: *[._]log[.-][0-9] !*[._]log[.-][0-9]`, err.Error()) - // Create an insensitive exclude file with some invalid patterns - insensitiveExcludeFile := env.base + "/insensitiveexcludefile" - fileErr = os.WriteFile(insensitiveExcludeFile, []byte("*.go\n*[._]log[.-][0-9]\n!*[._]log[.-][0-9]"), 0644) - if fileErr != nil { - t.Fatalf("Could not write insensitive exclude file: %v", fileErr) - } - - err = testRunRestoreAssumeFailure("latest", RestoreOptions{excludePatternOptions: excludePatternOptions{InsensitiveExcludeFiles: []string{insensitiveExcludeFile}}}, env.gopts) + err = testRunRestoreAssumeFailure("latest", RestoreOptions{excludePatternOptions: excludePatternOptions{InsensitiveExcludeFiles: []string{patternsFile}}}, env.gopts) rtest.Equals(t, `Fatal: --iexclude-file: invalid pattern(s) provided: *[._]log[.-][0-9] !*[._]log[.-][0-9]`, err.Error()) From fe412e255394a1926c5cdb22e4bd881a7c9371cf Mon Sep 17 00:00:00 2001 From: Srigovind Nayak Date: Mon, 10 Jun 2024 01:55:39 +0530 Subject: [PATCH 12/12] fix: restore inclusion logic and restore tests doc: update exclude and include docs --- cmd/restic/cmd_restore.go | 4 +- cmd/restic/cmd_restore_integration_test.go | 82 ++++++++-------------- cmd/restic/exclude.go | 2 +- cmd/restic/include.go | 2 +- 4 files changed, 35 insertions(+), 55 deletions(-) diff --git a/cmd/restic/cmd_restore.go b/cmd/restic/cmd_restore.go index 03a990cc6..12b698950 100644 --- a/cmd/restic/cmd_restore.go +++ b/cmd/restic/cmd_restore.go @@ -178,12 +178,12 @@ func runRestore(ctx context.Context, opts RestoreOptions, gopts GlobalOptions, matched, childMayMatch := includeFn(item) selectedForRestore = selectedForRestore || matched childMayBeSelected = childMayBeSelected || childMayMatch - childMayBeSelected = childMayBeSelected && node.Type == "dir" - if selectedForRestore || childMayBeSelected { + if selectedForRestore && childMayBeSelected { break } } + childMayBeSelected = childMayBeSelected && node.Type == "dir" return selectedForRestore, childMayBeSelected } diff --git a/cmd/restic/cmd_restore_integration_test.go b/cmd/restic/cmd_restore_integration_test.go index 0e1620f7e..b0543850b 100644 --- a/cmd/restic/cmd_restore_integration_test.go +++ b/cmd/restic/cmd_restore_integration_test.go @@ -13,7 +13,6 @@ import ( "time" "github.com/restic/restic/internal/feature" - "github.com/restic/restic/internal/filter" "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" "github.com/restic/restic/internal/ui/termstatus" @@ -78,32 +77,11 @@ func testRunRestoreExcludesFromFile(t testing.TB, gopts GlobalOptions, dir strin } func TestRestoreMustFailWhenUsingBothIncludesAndExcludes(t *testing.T) { - testfiles := []struct { - path string - size uint - }{ - {"dir1/include_me.txt", 100}, - } - 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] - // Add both include and exclude patterns includePatterns := []string{"dir1/*include_me.txt", "dir2/**", "dir4/**/*_me.txt"} excludePatterns := []string{"dir1/*include_me.txt", "dir2/**", "dir4/**/*_me.txt"} @@ -116,8 +94,9 @@ func TestRestoreMustFailWhenUsingBothIncludesAndExcludes(t *testing.T) { restoreOpts.Includes = includePatterns restoreOpts.Excludes = excludePatterns - err := testRunRestoreAssumeFailure(snapshotID.String(), restoreOpts, env.gopts) - rtest.Assert(t, err != nil, "restore must fail if include and exclude patterns are provided") + err := testRunRestoreAssumeFailure("latest", restoreOpts, env.gopts) + rtest.Assert(t, err != nil && strings.Contains(err.Error(), "exclude and include patterns are mutually exclusive"), + "expected: %s error, got %v", "exclude and include patterns are mutually exclusive", err) } func TestRestoreIncludes(t *testing.T) { @@ -159,7 +138,7 @@ func TestRestoreIncludes(t *testing.T) { restoredir := filepath.Join(env.base, "restore") testRunRestoreIncludes(t, env.gopts, restoredir, snapshotID, includePatterns) - testRestoreFileInclusions := func(t *testing.T, env *testEnvironment, includePatterns []string) { + testRestoreFileInclusions := func(t *testing.T) { // Check that only the included files are restored for _, testFile := range testfiles { restoredFilePath := filepath.Join(restoredir, "testdata", testFile.path) @@ -172,7 +151,7 @@ func TestRestoreIncludes(t *testing.T) { } } - testRestoreFileInclusions(t, env, includePatterns) + testRestoreFileInclusions(t) // Create an include file with some patterns patternsFile := env.base + "/patternsFile" @@ -185,18 +164,19 @@ func TestRestoreIncludes(t *testing.T) { testRunRestoreIncludesFromFile(t, env.gopts, restoredir, snapshotID, patternsFile) - testRestoreFileInclusions(t, env, includePatterns) + testRestoreFileInclusions(t) } func TestRestoreFilter(t *testing.T) { testfiles := []struct { - name string - size uint + name string + size uint + exclude bool }{ - {"testfile1.c", 100}, - {"testfile2.exe", 101}, - {"subdir1/subdir2/testfile3.docx", 102}, - {"subdir1/subdir2/testfile4.c", 102}, + {"testfile1.c", 100, true}, + {"testfile2.exe", 101, true}, + {"subdir1/subdir2/testfile3.docx", 102, true}, + {"subdir1/subdir2/testfile4.c", 102, false}, } env, cleanup := withTestEnvironment(t) @@ -223,38 +203,38 @@ func TestRestoreFilter(t *testing.T) { rtest.OK(t, testFileSize(filepath.Join(env.base, "restore0", "testdata", testFile.name), int64(testFile.size))) } - excludePatterns := []string{"*.c", "*.exe", "*", "*file3*"} + excludePatterns := []string{"testfile1.c", "*.exe", "*file3*"} - testRestoreFileExclusions := func(t *testing.T, env *testEnvironment, excludePatterns []string) { - for i, pat := range excludePatterns { - base := filepath.Join(env.base, fmt.Sprintf("restore%d", i+1)) - testRunRestoreExcludes(t, env.gopts, base, snapshotID, []string{pat}) - for _, testFile := range testfiles { - err := testFileSize(filepath.Join(base, "testdata", testFile.name), int64(testFile.size)) - if ok, _ := filter.Match(pat, filepath.Base(testFile.name)); !ok { - rtest.OK(t, err) - } else { - rtest.Assert(t, os.IsNotExist(err), - "expected %v to not exist in restore step %v, but it exists, err %v", testFile.name, i+1, err) - } + // checks if the files are excluded correctly + testRestoredFileExclusions := func(t *testing.T, restoredir string) { + for _, testFile := range testfiles { + restoredFilePath := filepath.Join(restoredir, "testdata", testFile.name) + _, err := os.Stat(restoredFilePath) + if testFile.exclude { + rtest.Assert(t, os.IsNotExist(err), "File %s should not have been restored", testFile.name) + } else { + rtest.OK(t, testFileSize(restoredFilePath, int64(testFile.size))) } } } - testRestoreFileExclusions(t, env, excludePatterns) + // restore with excludes + restoredir := filepath.Join(env.base, "restore-with-excludes") + testRunRestoreExcludes(t, env.gopts, restoredir, snapshotID, excludePatterns) + testRestoredFileExclusions(t, restoredir) - // Create an include file with some patterns + // Create an exclude file with some patterns patternsFile := env.base + "/patternsFile" fileErr := os.WriteFile(patternsFile, []byte(strings.Join(excludePatterns, "\n")), 0644) if fileErr != nil { t.Fatalf("Could not write include file: %v", fileErr) } - restoredir := filepath.Join(env.base, "restore-exclude-from-file") - + // restore with excludes from file + restoredir = filepath.Join(env.base, "restore-with-exclude-from-file") testRunRestoreExcludesFromFile(t, env.gopts, restoredir, snapshotID, patternsFile) - testRestoreFileExclusions(t, env, excludePatterns) + testRestoredFileExclusions(t, restoredir) } func TestRestore(t *testing.T) { diff --git a/cmd/restic/exclude.go b/cmd/restic/exclude.go index 9f5f40511..4657e4915 100644 --- a/cmd/restic/exclude.go +++ b/cmd/restic/exclude.go @@ -443,7 +443,7 @@ func initExcludePatternOptions(f *pflag.FlagSet, opts *excludePatternOptions) { f.StringArrayVarP(&opts.Excludes, "exclude", "e", nil, "exclude a `pattern` (can be specified multiple times)") f.StringArrayVar(&opts.InsensitiveExcludes, "iexclude", nil, "same as --exclude `pattern` but ignores the casing of filenames") f.StringArrayVar(&opts.ExcludeFiles, "exclude-file", nil, "read exclude patterns from a `file` (can be specified multiple times)") - f.StringArrayVar(&opts.InsensitiveExcludeFiles, "iexclude-file", nil, "same as --exclude-file but ignores casing of filenames in patterns") + f.StringArrayVar(&opts.InsensitiveExcludeFiles, "iexclude-file", nil, "same as --exclude-file but ignores casing of `file`names in patterns") } func (opts *excludePatternOptions) Empty() bool { diff --git a/cmd/restic/include.go b/cmd/restic/include.go index 64659d98f..dcc4c7f37 100644 --- a/cmd/restic/include.go +++ b/cmd/restic/include.go @@ -23,7 +23,7 @@ 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 filenames in patterns") + 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) {