fix: sync --delete respects --exclude and --include (#815)

README.md

@@ -443,6 +443,9 @@ cp styles.css s3://bucket/static/styles.css
 cp readme.md s3://bucket/static/readme.md
 ```
 
+`--exclude` and `--include` also apply to `--delete`: destination objects that
+match an exclude pattern, or that match none of the include patterns, are kept.
+
 It's also possible to use wildcards to sync only a subset of files.
 
 To sync only `.html` files in S3 bucket above to same local file system;

command/context.go

@@ -62,6 +62,8 @@ func contextValue(c *cli.Context, flagname string) []string {
 }
 
 // generateCommand generates command string from given context, app command, default flags and urls.
+// A default flag with a nil value is omitted from the generated command, even
+// if it is set in the given context.
 func generateCommand(c *cli.Context, cmd string, defaultFlags map[string]interface{}, urls ...*url.URL) (string, error) {
 	command := AppCommand(cmd)
 	flagset := flag.NewFlagSet(command.Name, flag.ContinueOnError)
@@ -73,6 +75,9 @@ func generateCommand(c *cli.Context, cmd string, defaultFlags map[string]interfa
 
 	flags := []string{}
 	for flagname, flagvalue := range defaultFlags {
+		if flagvalue == nil {
+			continue
+		}
 		flags = append(flags, fmt.Sprintf("--%s='%v'", flagname, flagvalue))
 	}
 

command/context_test.go

@@ -135,6 +135,30 @@ func TestGenerateCommand(t *testing.T) {
 			},
 			expectedCommand: `cp --exclude='*.log' --exclude='*.txt' "/source/dir" "s3://bucket/prefix/"`,
 		},
+		{
+			name: "nil-default-flag-should-be-omitted-even-if-set-in-context",
+			cmd:  "rm",
+			flags: []cli.Flag{
+				&cli.StringSliceFlag{
+					Name:  "exclude",
+					Value: cli.NewStringSlice("*.txt"),
+				},
+				&cli.StringSliceFlag{
+					Name:  "include",
+					Value: cli.NewStringSlice("*.log"),
+				},
+			},
+			defaultFlags: map[string]interface{}{
+				"raw":     true,
+				"exclude": nil,
+				"include": nil,
+			},
+			urls: []*url.URL{
+				mustNewURL(t, "s3://bucket/key1"),
+				mustNewURL(t, "s3://bucket/key2"),
+			},
+			expectedCommand: `rm --raw='true' "s3://bucket/key1" "s3://bucket/key2"`,
+		},
 		{
 			name:  "command-with-multiple-args",
 			cmd:   "rm",

command/sync.go

@@ -6,6 +6,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"regexp"
 	"strings"
 	"sync"
 
@@ -131,6 +132,12 @@ type Sync struct {
 	delete      bool
 	sizeOnly    bool
 	exitOnError bool
+	exclude     []string
+	include     []string
+
+	// patterns
+	excludePatterns []*regexp.Regexp
+	includePatterns []*regexp.Regexp
 
 	// s3 options
 	storageOpts storage.Options
@@ -155,6 +162,8 @@ func NewSync(c *cli.Context) Sync {
 		delete:      c.Bool("delete"),
 		sizeOnly:    c.Bool("size-only"),
 		exitOnError: c.Bool("exit-on-error"),
+		exclude:     c.StringSlice("exclude"),
+		include:     c.StringSlice("include"),
 
 		// flags
 		followSymlinks: !c.Bool("no-follow-symlinks"),
@@ -180,6 +189,18 @@ func (s Sync) Run(c *cli.Context) error {
 		return err
 	}
 
+	s.excludePatterns, err = createRegexFromWildcard(s.exclude)
+	if err != nil {
+		printError(s.fullCommand, s.op, err)
+		return err
+	}
+
+	s.includePatterns, err = createRegexFromWildcard(s.include)
+	if err != nil {
+		printError(s.fullCommand, s.op, err)
+		return err
+	}
+
 	ctx, cancel := context.WithCancel(c.Context)
 
 	sourceObjects, destObjects, err := s.getSourceAndDestinationObjects(ctx, cancel, srcurl, dsturl)
@@ -505,14 +526,32 @@ func (s Sync) planRun(
 			dstURLs := make([]*url.URL, 0, extsortChunkSize)
 
 			for d := range onlyDest {
+				// objects filtered out by --exclude/--include are not part
+				// of the sync, so they must not be deleted from the
+				// destination either.
+				if len(s.excludePatterns) > 0 && isURLMatched(s.excludePatterns, d.Path, dsturl.Prefix) {
+					continue
+				}
+				if len(s.includePatterns) > 0 && !isURLMatched(s.includePatterns, d.Path, dsturl.Prefix) {
+					continue
+				}
 				dstURLs = append(dstURLs, d)
 			}
 
 			if len(dstURLs) == 0 {
 				return
 			}
 
-			command, err := generateCommand(c, "rm", defaultFlags, dstURLs...)
+			// --exclude and --include are already applied above, relative to
+			// the destination prefix. Omit them from the generated rm command,
+			// which would match them against the full object key instead.
+			rmFlags := map[string]interface{}{
+				"raw":     true,
+				"exclude": nil,
+				"include": nil,
+			}
+
+			command, err := generateCommand(c, "rm", rmFlags, dstURLs...)
 			if err != nil {
 				printDebug(s.op, err, dstURLs...)
 				return

e2e/sync_test.go

@@ -2439,6 +2439,200 @@ func TestSyncLocalDirectoryToS3WithExcludeFilter(t *testing.T) {
 	}
 }
 
+// sync --delete --exclude "sub/*" folder/ s3://bucket/prefix/
+func TestSyncLocalToS3BucketWithDeleteAndExcludeFilter(t *testing.T) {
+	t.Parallel()
+
+	s3client, s5cmd := setup(t)
+
+	bucket := s3BucketFromTestName(t)
+	createBucket(t, s3client, bucket)
+
+	folderLayout := []fs.PathOp{
+		fs.WithFile("readme.md", "S: this is a readme file"),
+	}
+
+	workdir := fs.NewDir(t, "somedir", folderLayout...)
+	defer workdir.Remove()
+
+	s3Content := map[string]string{
+		"prefix/sub/keep.txt": "D: this is a text file",
+		"prefix/old.log":      "D: this is a log file",
+	}
+
+	for filename, content := range s3Content {
+		putFile(t, s3client, bucket, filename, content)
+	}
+
+	// pattern is relative to the destination prefix.
+	const excludePattern = "sub/*"
+
+	src := fmt.Sprintf("%v/", workdir.Path())
+	src = filepath.ToSlash(src)
+	dst := fmt.Sprintf("s3://%v/prefix/", bucket)
+
+	cmd := s5cmd("sync", "--delete", "--exclude", excludePattern, src, dst)
+	result := icmd.RunCmd(cmd)
+
+	result.Assert(t, icmd.Success)
+
+	assertLines(t, result.Stdout(), map[int]compareFunc{
+		0: equals(`cp %vreadme.md %vreadme.md`, src, dst),
+		1: equals(`rm %vold.log`, dst),
+	}, sortInput(true))
+
+	// assert local filesystem
+	expected := fs.Expected(t, folderLayout...)
+	assert.Assert(t, fs.Equal(workdir.Path(), expected))
+
+	expectedS3Content := map[string]string{
+		"prefix/readme.md": "S: this is a readme file",
+		// excluded object exists only in destination and must not be deleted.
+		"prefix/sub/keep.txt": "D: this is a text file",
+	}
+
+	nonExpectedS3Content := map[string]string{
+		"prefix/old.log": "D: this is a log file",
+	}
+
+	// assert objects should be in S3
+	for key, content := range expectedS3Content {
+		assert.Assert(t, ensureS3Object(s3client, bucket, key, content))
+	}
+
+	// assert objects should not be in S3.
+	for key, content := range nonExpectedS3Content {
+		err := ensureS3Object(s3client, bucket, key, content)
+		assertError(t, err, errS3NoSuchKey)
+	}
+}
+
+// sync --delete --exclude "sub/*" s3://bucket/* folder/
+func TestSyncS3BucketToLocalWithDeleteAndExcludeFilter(t *testing.T) {
+	t.Parallel()
+
+	s3client, s5cmd := setup(t)
+
+	bucket := s3BucketFromTestName(t)
+	createBucket(t, s3client, bucket)
+
+	s3Content := map[string]string{
+		"readme.md": "S: this is a readme file",
+	}
+
+	for filename, content := range s3Content {
+		putFile(t, s3client, bucket, filename, content)
+	}
+
+	folderLayout := []fs.PathOp{
+		fs.WithFile("old.log", "D: this is a log file"),
+		fs.WithDir("sub",
+			fs.WithFile("keep.txt", "D: this is a text file"),
+		),
+	}
+
+	workdir := fs.NewDir(t, "somedir", folderLayout...)
+	defer workdir.Remove()
+
+	// pattern is relative to the destination directory.
+	const excludePattern = "sub/*"
+
+	src := fmt.Sprintf("s3://%v/", bucket)
+	dst := fmt.Sprintf("%v/", workdir.Path())
+	dst = filepath.ToSlash(dst)
+
+	cmd := s5cmd("sync", "--delete", "--exclude", excludePattern, src+"*", dst)
+	result := icmd.RunCmd(cmd)
+
+	result.Assert(t, icmd.Success)
+
+	assertLines(t, result.Stdout(), map[int]compareFunc{
+		0: equals(`cp %vreadme.md %vreadme.md`, src, dst),
+		1: equals(`rm %vold.log`, dst),
+	}, sortInput(true))
+
+	expectedFolderLayout := []fs.PathOp{
+		fs.WithFile("readme.md", "S: this is a readme file"),
+		// excluded file exists only in destination and must not be deleted.
+		fs.WithDir("sub",
+			fs.WithFile("keep.txt", "D: this is a text file"),
+		),
+	}
+
+	expected := fs.Expected(t, expectedFolderLayout...)
+	assert.Assert(t, fs.Equal(workdir.Path(), expected))
+
+	for key, content := range s3Content {
+		assert.Assert(t, ensureS3Object(s3client, bucket, key, content))
+	}
+}
+
+// sync --delete --include "*.md" --include "sub/*" folder/ s3://bucket/prefix/
+func TestSyncLocalToS3BucketWithDeleteAndIncludeFilter(t *testing.T) {
+	t.Parallel()
+
+	s3client, s5cmd := setup(t)
+
+	bucket := s3BucketFromTestName(t)
+	createBucket(t, s3client, bucket)
+
+	folderLayout := []fs.PathOp{
+		fs.WithFile("readme.md", "S: this is a readme file"),
+	}
+
+	workdir := fs.NewDir(t, "somedir", folderLayout...)
+	defer workdir.Remove()
+
+	s3Content := map[string]string{
+		"prefix/keep.txt":    "D: this is a text file",
+		"prefix/sub/old.log": "D: this is a log file",
+	}
+
+	for filename, content := range s3Content {
+		putFile(t, s3client, bucket, filename, content)
+	}
+
+	src := fmt.Sprintf("%v/", workdir.Path())
+	src = filepath.ToSlash(src)
+	dst := fmt.Sprintf("s3://%v/prefix/", bucket)
+
+	// patterns are relative to the destination prefix.
+	cmd := s5cmd("sync", "--delete", "--include", "*.md", "--include", "sub/*", src, dst)
+	result := icmd.RunCmd(cmd)
+
+	result.Assert(t, icmd.Success)
+
+	assertLines(t, result.Stdout(), map[int]compareFunc{
+		0: equals(`cp %vreadme.md %vreadme.md`, src, dst),
+		1: equals(`rm %vsub/old.log`, dst),
+	}, sortInput(true))
+
+	// assert local filesystem
+	expected := fs.Expected(t, folderLayout...)
+	assert.Assert(t, fs.Equal(workdir.Path(), expected))
+
+	expectedS3Content := map[string]string{
+		"prefix/readme.md": "S: this is a readme file",
+		// object not matching --include exists only in destination and must not be deleted.
+		"prefix/keep.txt": "D: this is a text file",
+	}
+
+	nonExpectedS3Content := map[string]string{
+		"prefix/sub/old.log": "D: this is a log file",
+	}
+
+	// assert objects should be in S3
+	for key, content := range expectedS3Content {
+		assert.Assert(t, ensureS3Object(s3client, bucket, key, content))
+	}
+
+	// assert objects should not be in S3.
+	for key, content := range nonExpectedS3Content {
+		err := ensureS3Object(s3client, bucket, key, content)
+		assertError(t, err, errS3NoSuchKey)
+	}
+}
+
 // sync --delete somedir s3://bucket/ (removes 10k objects)
 func TestIssue435(t *testing.T) {
 	t.Parallel()