From c554cdac4c93e5315cee5171843e8cfc5251385d Mon Sep 17 00:00:00 2001 From: Pauline Middelink Date: Fri, 7 Jul 2017 03:19:06 +0200 Subject: [PATCH] Update HasTags() and HasPaths() to follow #1081 idea Replace all but 3 occurences of StringSliceVar to StringArrayVar. This will prevent the flag parser to interpretate the given values as CSV string. Both --tag, --keep-tag and --path can be given multiple times, the command will match snapshots matching ANY of the tags/paths. Only when a value is given which contains a comma separated list of tags/paths, ALL elements need to match. --- src/cmds/restic/cmd_backup.go | 6 +- src/cmds/restic/cmd_find.go | 6 +- src/cmds/restic/cmd_forget.go | 6 +- src/cmds/restic/cmd_ls.go | 4 +- src/cmds/restic/cmd_mount.go | 4 +- src/cmds/restic/cmd_restore.go | 8 +- src/cmds/restic/cmd_snapshots.go | 4 +- src/cmds/restic/cmd_tag.go | 4 +- src/restic/snapshot.go | 53 +++++-- ...filter_test.go => snapshot_policy_test.go} | 1 + src/restic/testdata/policy_keep_snapshots_20 | 131 ++++++++++++++++++ 11 files changed, 194 insertions(+), 33 deletions(-) rename src/restic/{snapshot_filter_test.go => snapshot_policy_test.go} (99%) create mode 100644 src/restic/testdata/policy_keep_snapshots_20 diff --git a/src/cmds/restic/cmd_backup.go b/src/cmds/restic/cmd_backup.go index 53204f2d2..41f6dcaf2 100644 --- a/src/cmds/restic/cmd_backup.go +++ b/src/cmds/restic/cmd_backup.go @@ -68,12 +68,12 @@ func init() { f := cmdBackup.Flags() f.StringVar(&backupOptions.Parent, "parent", "", "use this parent snapshot (default: last snapshot in the repo that has the same target files/directories)") f.BoolVarP(&backupOptions.Force, "force", "f", false, `force re-reading the target files/directories (overrides the "parent" flag)`) - f.StringSliceVarP(&backupOptions.Excludes, "exclude", "e", nil, "exclude a `pattern` (can be specified multiple times)") - f.StringSliceVar(&backupOptions.ExcludeFiles, "exclude-file", nil, "read exclude patterns from a `file` (can be specified multiple times)") + f.StringArrayVarP(&backupOptions.Excludes, "exclude", "e", nil, "exclude a `pattern` (can be specified multiple times)") + f.StringArrayVar(&backupOptions.ExcludeFiles, "exclude-file", nil, "read exclude patterns from a `file` (can be specified multiple times)") f.BoolVarP(&backupOptions.ExcludeOtherFS, "one-file-system", "x", false, "exclude other file systems") f.BoolVar(&backupOptions.Stdin, "stdin", false, "read backup from stdin") f.StringVar(&backupOptions.StdinFilename, "stdin-filename", "stdin", "file name to use when reading from stdin") - f.StringSliceVar(&backupOptions.Tags, "tag", nil, "add a `tag` for the new snapshot (can be specified multiple times)") + f.StringArrayVar(&backupOptions.Tags, "tag", nil, "add a `tag` for the new snapshot (can be specified multiple times)") f.StringVar(&backupOptions.Hostname, "hostname", hostname, "set the `hostname` for the snapshot manually") f.StringVar(&backupOptions.FilesFrom, "files-from", "", "read the files to backup from file (can be combined with file args)") } diff --git a/src/cmds/restic/cmd_find.go b/src/cmds/restic/cmd_find.go index 36932491c..e571cc265 100644 --- a/src/cmds/restic/cmd_find.go +++ b/src/cmds/restic/cmd_find.go @@ -45,13 +45,13 @@ func init() { f := cmdFind.Flags() f.StringVarP(&findOptions.Oldest, "oldest", "O", "", "oldest modification date/time") f.StringVarP(&findOptions.Newest, "newest", "N", "", "newest modification date/time") - f.StringSliceVarP(&findOptions.Snapshots, "snapshot", "s", nil, "snapshot `id` to search in (can be given multiple times)") + f.StringArrayVarP(&findOptions.Snapshots, "snapshot", "s", nil, "snapshot `id` to search in (can be given multiple times)") f.BoolVarP(&findOptions.CaseInsensitive, "ignore-case", "i", false, "ignore case for pattern") f.BoolVarP(&findOptions.ListLong, "long", "l", false, "use a long listing format showing size and mode") f.StringVarP(&findOptions.Host, "host", "H", "", "only consider snapshots for this `host`, when no snapshot ID is given") - f.StringSliceVar(&findOptions.Tags, "tag", nil, "only consider snapshots which include this `tag`, when no snapshot-ID is given") - f.StringSliceVar(&findOptions.Paths, "path", nil, "only consider snapshots which include this (absolute) `path`, when no snapshot-ID is given") + f.StringArrayVar(&findOptions.Tags, "tag", nil, "only consider snapshots which include this `tag`, when no snapshot-ID is given") + f.StringArrayVar(&findOptions.Paths, "path", nil, "only consider snapshots which include this (absolute) `path`, when no snapshot-ID is given") } type findPattern struct { diff --git a/src/cmds/restic/cmd_forget.go b/src/cmds/restic/cmd_forget.go index f4c03b4f1..ba268162c 100644 --- a/src/cmds/restic/cmd_forget.go +++ b/src/cmds/restic/cmd_forget.go @@ -55,14 +55,14 @@ func init() { f.IntVarP(&forgetOptions.Monthly, "keep-monthly", "m", 0, "keep the last `n` monthly snapshots") f.IntVarP(&forgetOptions.Yearly, "keep-yearly", "y", 0, "keep the last `n` yearly snapshots") - f.StringSliceVar(&forgetOptions.KeepTags, "keep-tag", []string{}, "keep snapshots with this `tag` (can be specified multiple times)") + f.StringArrayVar(&forgetOptions.KeepTags, "keep-tag", []string{}, "keep snapshots with this `tag` (can be specified multiple times)") f.BoolVarP(&forgetOptions.GroupByTags, "group-by-tags", "G", false, "Group by host,paths,tags instead of just host,paths") // Sadly the commonly used shortcut `H` is already used. f.StringVar(&forgetOptions.Host, "host", "", "only consider snapshots with the given `host`") // Deprecated since 2017-03-07. f.StringVar(&forgetOptions.Host, "hostname", "", "only consider snapshots with the given `hostname` (deprecated)") - f.StringSliceVar(&forgetOptions.Tags, "tag", nil, "only consider snapshots which include this `tag` (can be specified multiple times)") - f.StringSliceVar(&forgetOptions.Paths, "path", nil, "only consider snapshots which include this (absolute) `path` (can be specified multiple times)") + f.StringArrayVar(&forgetOptions.Tags, "tag", nil, "only consider snapshots which include this `tag` (can be specified multiple times)") + f.StringArrayVar(&forgetOptions.Paths, "path", nil, "only consider snapshots which include this (absolute) `path` (can be specified multiple times)") f.BoolVarP(&forgetOptions.DryRun, "dry-run", "n", false, "do not delete anything, just print what would be done") f.BoolVar(&forgetOptions.Prune, "prune", false, "automatically run the 'prune' command if snapshots have been removed") diff --git a/src/cmds/restic/cmd_ls.go b/src/cmds/restic/cmd_ls.go index d71d4e2b3..329ff5f70 100644 --- a/src/cmds/restic/cmd_ls.go +++ b/src/cmds/restic/cmd_ls.go @@ -41,8 +41,8 @@ func init() { flags.BoolVarP(&lsOptions.ListLong, "long", "l", false, "use a long listing format showing size and mode") flags.StringVarP(&lsOptions.Host, "host", "H", "", "only consider snapshots for this `host`, when no snapshot ID is given") - flags.StringSliceVar(&lsOptions.Tags, "tag", nil, "only consider snapshots which include this `tag`, when no snapshot ID is given") - flags.StringSliceVar(&lsOptions.Paths, "path", nil, "only consider snapshots which include this (absolute) `path`, when no snapshot ID is given") + flags.StringArrayVar(&lsOptions.Tags, "tag", nil, "only consider snapshots which include this `tag`, when no snapshot ID is given") + flags.StringArrayVar(&lsOptions.Paths, "path", nil, "only consider snapshots which include this (absolute) `path`, when no snapshot ID is given") } func printTree(repo *repository.Repository, id *restic.ID, prefix string) error { diff --git a/src/cmds/restic/cmd_mount.go b/src/cmds/restic/cmd_mount.go index 243fa4e37..323f34c8c 100644 --- a/src/cmds/restic/cmd_mount.go +++ b/src/cmds/restic/cmd_mount.go @@ -52,8 +52,8 @@ func init() { mountFlags.BoolVar(&mountOptions.AllowOther, "allow-other", false, "allow other users to access the data in the mounted directory") mountFlags.StringVarP(&mountOptions.Host, "host", "H", "", `only consider snapshots for this host`) - mountFlags.StringSliceVar(&mountOptions.Tags, "tag", nil, "only consider snapshots which include this `tag`") - mountFlags.StringSliceVar(&mountOptions.Paths, "path", nil, "only consider snapshots which include this (absolute) `path`") + mountFlags.StringArrayVar(&mountOptions.Tags, "tag", nil, "only consider snapshots which include this `tag`") + mountFlags.StringArrayVar(&mountOptions.Paths, "path", nil, "only consider snapshots which include this (absolute) `path`") } func mount(opts MountOptions, gopts GlobalOptions, mountpoint string) error { diff --git a/src/cmds/restic/cmd_restore.go b/src/cmds/restic/cmd_restore.go index 9dec03851..0d1575f29 100644 --- a/src/cmds/restic/cmd_restore.go +++ b/src/cmds/restic/cmd_restore.go @@ -40,13 +40,13 @@ func init() { cmdRoot.AddCommand(cmdRestore) flags := cmdRestore.Flags() - flags.StringSliceVarP(&restoreOptions.Exclude, "exclude", "e", nil, "exclude a `pattern` (can be specified multiple times)") - flags.StringSliceVarP(&restoreOptions.Include, "include", "i", nil, "include a `pattern`, exclude everything else (can be specified multiple times)") + flags.StringArrayVarP(&restoreOptions.Exclude, "exclude", "e", nil, "exclude a `pattern` (can be specified multiple times)") + flags.StringArrayVarP(&restoreOptions.Include, "include", "i", nil, "include a `pattern`, exclude everything else (can be specified multiple times)") flags.StringVarP(&restoreOptions.Target, "target", "t", "", "directory to extract data to") flags.StringVarP(&restoreOptions.Host, "host", "H", "", `only consider snapshots for this host when the snapshot ID is "latest"`) - flags.StringSliceVar(&restoreOptions.Tags, "tag", nil, "only consider snapshots which include this `tag` for snapshot ID \"latest\"") - flags.StringSliceVar(&restoreOptions.Paths, "path", nil, "only consider snapshots which include this (absolute) `path` for snapshot ID \"latest\"") + flags.StringArrayVar(&restoreOptions.Tags, "tag", nil, "only consider snapshots which include this `tag` for snapshot ID \"latest\"") + flags.StringArrayVar(&restoreOptions.Paths, "path", nil, "only consider snapshots which include this (absolute) `path` for snapshot ID \"latest\"") } func runRestore(opts RestoreOptions, gopts GlobalOptions, args []string) error { diff --git a/src/cmds/restic/cmd_snapshots.go b/src/cmds/restic/cmd_snapshots.go index 7a3fa9879..4c0eb8f6e 100644 --- a/src/cmds/restic/cmd_snapshots.go +++ b/src/cmds/restic/cmd_snapshots.go @@ -37,8 +37,8 @@ func init() { f := cmdSnapshots.Flags() f.StringVarP(&snapshotOptions.Host, "host", "H", "", "only consider snapshots for this `host`") - f.StringSliceVar(&snapshotOptions.Tags, "tag", nil, "only consider snapshots which include this `tag` (can be specified multiple times)") - f.StringSliceVar(&snapshotOptions.Paths, "path", nil, "only consider snapshots for this `path` (can be specified multiple times)") + f.StringArrayVar(&snapshotOptions.Tags, "tag", nil, "only consider snapshots which include this `tag` (can be specified multiple times)") + f.StringArrayVar(&snapshotOptions.Paths, "path", nil, "only consider snapshots for this `path` (can be specified multiple times)") } func runSnapshots(opts SnapshotOptions, gopts GlobalOptions, args []string) error { diff --git a/src/cmds/restic/cmd_tag.go b/src/cmds/restic/cmd_tag.go index 32c2ba583..495aed57e 100644 --- a/src/cmds/restic/cmd_tag.go +++ b/src/cmds/restic/cmd_tag.go @@ -48,8 +48,8 @@ func init() { tagFlags.StringSliceVar(&tagOptions.RemoveTags, "remove", nil, "`tag` which will be removed from the existing tags (can be given multiple times)") tagFlags.StringVarP(&tagOptions.Host, "host", "H", "", "only consider snapshots for this `host`, when no snapshot ID is given") - tagFlags.StringSliceVar(&tagOptions.Tags, "tag", nil, "only consider snapshots which include this `tag`, when no snapshot-ID is given") - tagFlags.StringSliceVar(&tagOptions.Paths, "path", nil, "only consider snapshots which include this (absolute) `path`, when no snapshot-ID is given") + tagFlags.StringArrayVar(&tagOptions.Tags, "tag", nil, "only consider snapshots which include this `tag`, when no snapshot-ID is given") + tagFlags.StringArrayVar(&tagOptions.Paths, "path", nil, "only consider snapshots which include this (absolute) `path`, when no snapshot-ID is given") } func changeTags(repo *repository.Repository, sn *restic.Snapshot, setTags, addTags, removeTags []string) (bool, error) { diff --git a/src/restic/snapshot.go b/src/restic/snapshot.go index 11693bd52..a641e5b8f 100644 --- a/src/restic/snapshot.go +++ b/src/restic/snapshot.go @@ -5,6 +5,7 @@ import ( "fmt" "os/user" "path/filepath" + "strings" "time" ) @@ -130,36 +131,64 @@ func (sn *Snapshot) RemoveTags(removeTags []string) (changed bool) { return } -// HasTags returns true if the snapshot has at least all of tags. +func (sn *Snapshot) hasTag(tag string) bool { + for _, snTag := range sn.Tags { + if tag == snTag { + return true + } + } + return false +} + +// HasTags returns true if the snapshot has at least one of the tags. Tags +// are compared as strings, unless they contain a comma. Then each of the comma +// separated parts of the tag need to be present. func (sn *Snapshot) HasTags(tags []string) bool { + if len(tags) == 0 { + return true + } nextTag: for _, tag := range tags { - for _, snTag := range sn.Tags { - if tag == snTag { + for _, s := range strings.Split(tag, ",") { + if !sn.hasTag(s) { + // fail, try next tag continue nextTag } } - - return false + return true } - return true + return false } -// HasPaths returns true if the snapshot has at least all of paths. +func (sn *Snapshot) hasPath(path string) bool { + for _, snPath := range sn.Paths { + if path == snPath { + return true + } + } + return false +} + +// HasPaths returns true if the snapshot has at least one of the paths. Paths +// are compared as strings unless they contain a comma. Then each of the comma +// separated parts of the path need to be present. func (sn *Snapshot) HasPaths(paths []string) bool { + if len(paths) == 0 { + return true + } nextPath: for _, path := range paths { - for _, snPath := range sn.Paths { - if path == snPath { + for _, p := range strings.Split(path, ",") { + if !sn.hasPath(p) { + // fail, try next path continue nextPath } } - - return false + return true } - return true + return false } // SamePaths returns true if the snapshot matches the entire paths set diff --git a/src/restic/snapshot_filter_test.go b/src/restic/snapshot_policy_test.go similarity index 99% rename from src/restic/snapshot_filter_test.go rename to src/restic/snapshot_policy_test.go index 1bf1e7460..63dec3583 100644 --- a/src/restic/snapshot_filter_test.go +++ b/src/restic/snapshot_policy_test.go @@ -164,6 +164,7 @@ var expireTests = []restic.ExpirePolicy{ {Yearly: 10}, {Daily: 7, Weekly: 2, Monthly: 3, Yearly: 10}, {Tags: []string{"foo"}}, + {Tags: []string{"foo,bar"}}, {Tags: []string{"foo", "bar"}}, } diff --git a/src/restic/testdata/policy_keep_snapshots_20 b/src/restic/testdata/policy_keep_snapshots_20 new file mode 100644 index 000000000..004b12156 --- /dev/null +++ b/src/restic/testdata/policy_keep_snapshots_20 @@ -0,0 +1,131 @@ +[ + { + "time": "2014-11-15T10:20:30Z", + "tree": null, + "paths": null, + "tags": [ + "foo", + "bar" + ] + }, + { + "time": "2014-11-13T10:20:30.1Z", + "tree": null, + "paths": null, + "tags": [ + "bar" + ] + }, + { + "time": "2014-11-13T10:20:30Z", + "tree": null, + "paths": null, + "tags": [ + "foo" + ] + }, + { + "time": "2014-11-12T10:20:30Z", + "tree": null, + "paths": null, + "tags": [ + "foo" + ] + }, + { + "time": "2014-11-10T10:20:30Z", + "tree": null, + "paths": null, + "tags": [ + "foo" + ] + }, + { + "time": "2014-11-08T10:20:30Z", + "tree": null, + "paths": null, + "tags": [ + "foo" + ] + }, + { + "time": "2014-10-22T10:20:30Z", + "tree": null, + "paths": null, + "tags": [ + "foo" + ] + }, + { + "time": "2014-10-20T10:20:30Z", + "tree": null, + "paths": null, + "tags": [ + "foo" + ] + }, + { + "time": "2014-10-11T10:20:30Z", + "tree": null, + "paths": null, + "tags": [ + "foo" + ] + }, + { + "time": "2014-10-10T10:20:30Z", + "tree": null, + "paths": null, + "tags": [ + "foo" + ] + }, + { + "time": "2014-10-09T10:20:30Z", + "tree": null, + "paths": null, + "tags": [ + "foo" + ] + }, + { + "time": "2014-10-08T10:20:30Z", + "tree": null, + "paths": null, + "tags": [ + "foo" + ] + }, + { + "time": "2014-10-06T10:20:30Z", + "tree": null, + "paths": null, + "tags": [ + "foo" + ] + }, + { + "time": "2014-10-05T10:20:30Z", + "tree": null, + "paths": null, + "tags": [ + "foo" + ] + }, + { + "time": "2014-10-02T10:20:30Z", + "tree": null, + "paths": null, + "tags": [ + "foo" + ] + }, + { + "time": "2014-10-01T10:20:30Z", + "tree": null, + "paths": null, + "tags": [ + "foo" + ] + } +] \ No newline at end of file