bisync: add overlapping paths check

Before this change, Bisync did not check to make sure that Path1 and Path2 do
not overlap, nor did it check for overlaps with `--backup-dir`. While `sync`
does check for these things, it can sometimes be fooled because of the way
Bisync calls it with `--files-from` filters. Relying on sync could also leave a
run in a half-finished state if it were to error in one direction but not the
other (`--backup-dir` only checks for overlaps with the dest.)

After this change, Bisync does its own check up front, so we can quickly return
an error and exit before any changes are made.

cmd/bisync/bisync_test.go

@@ -837,6 +837,10 @@ func (b *bisyncTest) runBisync(ctx context.Context, args []string) (err error) {
 		case "subdir":
 			fs1 = addSubdir(b.path1, val)
 			fs2 = addSubdir(b.path2, val)
+		case "backupdir1":
+			opt.BackupDir1 = val
+		case "backupdir2":
+			opt.BackupDir2 = val
 		case "ignore-listing-checksum":
 			opt.IgnoreListingChecksum = true
 		case "no-norm":
@@ -1168,6 +1172,10 @@ func (b *bisyncTest) storeGolden() {
 		if fileType(fileName) == "lock" {
 			continue
 		}
+		if fileName == "backupdirs" {
+			log.Printf("skipping: %v", fileName)
+			continue
+		}
 		goldName := b.toGolden(fileName)
 		if goldName != fileName {
 			targetPath := filepath.Join(b.workDir, goldName)
@@ -1189,6 +1197,10 @@ func (b *bisyncTest) storeGolden() {
 		if fileType(fileName) == "lock" {
 			continue
 		}
+		if fileName == "backupdirs" {
+			log.Printf("skipping: %v", fileName)
+			continue
+		}
 		text := b.mangleResult(b.goldenDir, fileName, true)
 
 		goldName := b.toGolden(fileName)
@@ -1205,6 +1217,9 @@ func (b *bisyncTest) storeGolden() {
 
 // mangleResult prepares test logs or listings for comparison
 func (b *bisyncTest) mangleResult(dir, file string, golden bool) string {
+	if file == "backupdirs" {
+		return "skipping backupdirs"
+	}
 	buf, err := os.ReadFile(filepath.Join(dir, file))
 	require.NoError(b.t, err)
 

cmd/bisync/operations.go

@@ -248,6 +248,14 @@ func (b *bisyncRun) runLocked(octx context.Context) (err error) {
 	b.octx = octx
 	b.fctx = fctx
 
+	// overlapping paths check
+	err = b.overlappingPathsCheck(fctx, b.fs1, b.fs2)
+	if err != nil {
+		b.critical = true
+		b.retryable = true
+		return err
+	}
+
 	// Generate Path1 and Path2 listings and copy any unique Path2 files to Path1
 	if opt.Resync {
 		return b.resync(octx, fctx)
@@ -676,12 +684,45 @@ func (b *bisyncRun) setBackupDir(ctx context.Context, destPath int) context.Cont
 		ci.BackupDir = b.opt.BackupDir1
 	}
 	if destPath == 2 && b.opt.BackupDir2 != "" {
-		ci.BackupDir = b.opt.BackupDir1
+		ci.BackupDir = b.opt.BackupDir2
 	}
 	fs.Debugf(ci.BackupDir, "updated backup-dir for Path%d", destPath)
 	return ctx
 }
 
+func (b *bisyncRun) overlappingPathsCheck(fctx context.Context, fs1, fs2 fs.Fs) error {
+	if operations.OverlappingFilterCheck(fctx, fs2, fs1) {
+		err = fmt.Errorf(Color(terminal.RedFg, "Overlapping paths detected. Cannot bisync between paths that overlap, unless excluded by filters."))
+		return err
+	}
+	// need to test our BackupDirs too, as sync will be fooled by our --files-from filters
+	testBackupDir := func(ctx context.Context, destPath int) error {
+		src := fs1
+		dst := fs2
+		if destPath == 1 {
+			src = fs2
+			dst = fs1
+		}
+		ctxBackupDir := b.setBackupDir(ctx, destPath)
+		ci := fs.GetConfig(ctxBackupDir)
+		if ci.BackupDir != "" {
+			// operations.BackupDir should return an error if not properly excluded
+			_, err = operations.BackupDir(fctx, dst, src, "")
+			return err
+		}
+		return nil
+	}
+	err = testBackupDir(fctx, 1)
+	if err != nil {
+		return err
+	}
+	err = testBackupDir(fctx, 2)
+	if err != nil {
+		return err
+	}
+	return nil
+}
+
 func (b *bisyncRun) debug(nametocheck, msgiftrue string) {
 	if b.DebugName != "" && b.DebugName == nametocheck {
 		fs.Infof(Color(terminal.MagentaBg, "DEBUGNAME "+b.DebugName), Color(terminal.MagentaBg, msgiftrue))

cmd/bisync/testdata/test_backupdir/golden/_testdir_path1.._testdir_path2.copy1to2.que

@@ -0,0 +1,5 @@
+"file11.txt"
+"file2.txt"
+"file4.txt"
+"file5.txt..path1"
+"file7.txt"

cmd/bisync/testdata/test_backupdir/golden/_testdir_path1.._testdir_path2.copy2to1.que

@@ -0,0 +1,5 @@
+"file1.txt"
+"file10.txt"
+"file3.txt"
+"file5.txt..path2"
+"file6.txt"

cmd/bisync/testdata/test_backupdir/golden/_testdir_path1.._testdir_path2.delete1.que

@@ -0,0 +1 @@
+"file3.txt"

cmd/bisync/testdata/test_backupdir/golden/_testdir_path1.._testdir_path2.delete2.que

@@ -0,0 +1 @@
+"file4.txt"

cmd/bisync/testdata/test_backupdir/golden/_testdir_path1.._testdir_path2.path1.lst-err

@@ -0,0 +1,10 @@
+# bisync listing v1 from test
+-      109 - - 2000-01-01T00:00:00.000000000+0000 "RCLONE_TEST"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file1.txt"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file10.txt"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file11.txt"
+-       13 - - 2001-01-02T00:00:00.000000000+0000 "file2.txt"
+-       39 - - 2001-03-04T00:00:00.000000000+0000 "file5.txt..path1"
+-       39 - - 2001-01-02T00:00:00.000000000+0000 "file5.txt..path2"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file6.txt"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file7.txt"

cmd/bisync/testdata/test_backupdir/golden/_testdir_path1.._testdir_path2.path1.lst-new

@@ -0,0 +1,10 @@
+# bisync listing v1 from test
+-      109 - - 2000-01-01T00:00:00.000000000+0000 "RCLONE_TEST"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file1.txt"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file10.txt"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file11.txt"
+-       13 - - 2001-01-02T00:00:00.000000000+0000 "file2.txt"
+-       39 - - 2001-03-04T00:00:00.000000000+0000 "file5.txt..path1"
+-       39 - - 2001-01-02T00:00:00.000000000+0000 "file5.txt..path2"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file6.txt"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file7.txt"

cmd/bisync/testdata/test_backupdir/golden/_testdir_path1.._testdir_path2.path1.lst-old

@@ -0,0 +1,10 @@
+# bisync listing v1 from test
+-      109 - - 2000-01-01T00:00:00.000000000+0000 "RCLONE_TEST"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file1.txt"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file10.txt"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file11.txt"
+-       13 - - 2001-01-02T00:00:00.000000000+0000 "file2.txt"
+-       39 - - 2001-03-04T00:00:00.000000000+0000 "file5.txt..path1"
+-       39 - - 2001-01-02T00:00:00.000000000+0000 "file5.txt..path2"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file6.txt"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file7.txt"

cmd/bisync/testdata/test_backupdir/golden/_testdir_path1.._testdir_path2.path2.lst-err

@@ -0,0 +1,10 @@
+# bisync listing v1 from test
+-      109 - - 2000-01-01T00:00:00.000000000+0000 "RCLONE_TEST"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file1.txt"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file10.txt"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file11.txt"
+-       13 - - 2001-01-02T00:00:00.000000000+0000 "file2.txt"
+-       39 - - 2001-03-04T00:00:00.000000000+0000 "file5.txt..path1"
+-       39 - - 2001-01-02T00:00:00.000000000+0000 "file5.txt..path2"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file6.txt"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file7.txt"

cmd/bisync/testdata/test_backupdir/golden/_testdir_path1.._testdir_path2.path2.lst-new

@@ -0,0 +1,10 @@
+# bisync listing v1 from test
+-      109 - - 2000-01-01T00:00:00.000000000+0000 "RCLONE_TEST"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file1.txt"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file10.txt"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file11.txt"
+-       13 - - 2001-01-02T00:00:00.000000000+0000 "file2.txt"
+-       39 - - 2001-03-04T00:00:00.000000000+0000 "file5.txt..path1"
+-       39 - - 2001-01-02T00:00:00.000000000+0000 "file5.txt..path2"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file6.txt"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file7.txt"

cmd/bisync/testdata/test_backupdir/golden/_testdir_path1.._testdir_path2.path2.lst-old

@@ -0,0 +1,10 @@
+# bisync listing v1 from test
+-      109 - - 2000-01-01T00:00:00.000000000+0000 "RCLONE_TEST"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file1.txt"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file10.txt"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file11.txt"
+-       13 - - 2001-01-02T00:00:00.000000000+0000 "file2.txt"
+-       39 - - 2001-03-04T00:00:00.000000000+0000 "file5.txt..path1"
+-       39 - - 2001-01-02T00:00:00.000000000+0000 "file5.txt..path2"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file6.txt"
+-       19 - - 2001-01-02T00:00:00.000000000+0000 "file7.txt"

cmd/bisync/testdata/test_backupdir/golden/test.log

@@ -0,0 +1,153 @@
+(01)  : test backupdir
+
+
+(02)  : test initial bisync
+(03)  : bisync resync backupdir1={workdir/}backupdirs/backupdir1 backupdir2={workdir/}backupdirs/backupdir2
+INFO  : Setting --ignore-listing-checksum as neither --checksum nor --compare checksum are set.
+INFO  : Bisyncing with Comparison Settings:
+{
+"Modtime": true,
+"Size": true,
+"Checksum": false,
+"NoSlowHash": false,
+"SlowHashSyncOnly": false,
+"DownloadHash": false
+}
+INFO  : Synching Path1 "{path1/}" with Path2 "{path2/}"
+INFO  : Copying unique Path2 files to Path1
+INFO  : - Path2    Resync is copying UNIQUE files to  - Path1
+INFO  : - Path1    Resync is copying UNIQUE OR DIFFERING files to - Path2
+INFO  : Resync updating listings
+INFO  : Bisync successful
+
+(04)  : test make modifications on both paths
+(05)  : test new on path2 - file10
+(06)  : touch-copy 2001-01-02 {datadir/}file10.txt {path2/}
+
+(07)  : test newer on path2 - file1
+(08)  : touch-copy 2001-01-02 {datadir/}file1.txt {path2/}
+
+(09)  : test new on path1 - file11
+(10)  : touch-copy 2001-01-02 {datadir/}file11.txt {path1/}
+
+(11)  : test newer on path1 - file2
+(12)  : touch-copy 2001-01-02 {datadir/}file2.txt {path1/}
+
+(13)  : test deleted on path2 - file3
+(14)  : delete-file {path2/}file3.txt
+
+(15)  : test deleted on path1 - file4
+(16)  : delete-file {path1/}file4.txt
+
+(17)  : test deleted on both paths - file8
+(18)  : delete-file {path1/}file8.txt
+(19)  : delete-file {path2/}file8.txt
+
+(20)  : test changed on both paths - file5 (file5R, file5L)
+(21)  : touch-glob 2001-01-02 {datadir/} file5R.txt
+(22)  : copy-as {datadir/}file5R.txt {path2/} file5.txt
+(23)  : touch-glob 2001-03-04 {datadir/} file5L.txt
+(24)  : copy-as {datadir/}file5L.txt {path1/} file5.txt
+
+(25)  : test newer on path2 and deleted on path1 - file6
+(26)  : touch-copy 2001-01-02 {datadir/}file6.txt {path2/}
+(27)  : delete-file {path1/}file6.txt
+
+(28)  : test newer on path1 and deleted on path2 - file7
+(29)  : touch-copy 2001-01-02 {datadir/}file7.txt {path1/}
+(30)  : delete-file {path2/}file7.txt
+
+(31)  : test bisync run
+(32)  : bisync backupdir1={workdir/}backupdirs/backupdir1 backupdir2={workdir/}backupdirs/backupdir2
+INFO  : Setting --ignore-listing-checksum as neither --checksum nor --compare checksum are set.
+INFO  : Bisyncing with Comparison Settings:
+{
+"Modtime": true,
+"Size": true,
+"Checksum": false,
+"NoSlowHash": false,
+"SlowHashSyncOnly": false,
+"DownloadHash": false
+}
+INFO  : Synching Path1 "{path1/}" with Path2 "{path2/}"
+INFO  : Building Path1 and Path2 listings
+INFO  : Path1 checking for diffs
+INFO  : - Path1    File changed: size (larger), time (newer) - file2.txt
+INFO  : - Path1    File was deleted          - file4.txt
+INFO  : - Path1    File changed: size (larger), time (newer) - file5.txt
+INFO  : - Path1    File was deleted          - file6.txt
+INFO  : - Path1    File changed: size (larger), time (newer) - file7.txt
+INFO  : - Path1    File was deleted          - file8.txt
+INFO  : - Path1    File is new               - file11.txt
+INFO  : Path1:    7 changes:    1 new,    3 modified,    3 deleted
+INFO  : (Modified:    3 newer,    0 older,    3 larger,    0 smaller)
+INFO  : Path2 checking for diffs
+INFO  : - Path2    File changed: size (larger), time (newer) - file1.txt
+INFO  : - Path2    File was deleted          - file3.txt
+INFO  : - Path2    File changed: size (larger), time (newer) - file5.txt
+INFO  : - Path2    File changed: size (larger), time (newer) - file6.txt
+INFO  : - Path2    File was deleted          - file7.txt
+INFO  : - Path2    File was deleted          - file8.txt
+INFO  : - Path2    File is new               - file10.txt
+INFO  : Path2:    7 changes:    1 new,    3 modified,    3 deleted
+INFO  : (Modified:    3 newer,    0 older,    3 larger,    0 smaller)
+INFO  : Applying changes
+INFO  : Checking potential conflicts...
+ERROR : file5.txt: md5 differ
+NOTICE: {path2String}: 1 differences found
+NOTICE: {path2String}: 1 errors while checking
+INFO  : Finished checking the potential conflicts. 1 differences found
+INFO  : - Path1    Queue copy to Path2       - {path2/}file11.txt
+INFO  : - Path1    Queue copy to Path2       - {path2/}file2.txt
+INFO  : - Path2    Queue delete              - {path2/}file4.txt
+NOTICE: - WARNING  New or changed in both paths       - file5.txt
+NOTICE: - Path1    Renaming Path1 copy                - {path1/}file5.txt..path1
+NOTICE: - Path1    Queue copy to Path2       - {path2/}file5.txt..path1
+NOTICE: - Path2    Renaming Path2 copy                - {path2/}file5.txt..path2
+NOTICE: - Path2    Queue copy to Path1       - {path1/}file5.txt..path2
+INFO  : - Path2    Queue copy to Path1       - {path1/}file6.txt
+INFO  : - Path1    Queue copy to Path2       - {path2/}file7.txt
+INFO  : - Path2    Queue copy to Path1       - {path1/}file1.txt
+INFO  : - Path2    Queue copy to Path1       - {path1/}file10.txt
+INFO  : - Path1    Queue delete              - {path1/}file3.txt
+INFO  : - Path2    Do queued copies to                - Path1
+INFO  : - Path1    Do queued copies to                - Path2
+INFO  : Updating listings
+INFO  : Validating listings for Path1 "{path1/}" vs Path2 "{path2/}"
+INFO  : Bisync successful
+(33)  : bisync backupdir1={workdir/}backupdirs/backupdir1 backupdir2={workdir/}backupdirs/backupdir2
+INFO  : Setting --ignore-listing-checksum as neither --checksum nor --compare checksum are set.
+INFO  : Bisyncing with Comparison Settings:
+{
+"Modtime": true,
+"Size": true,
+"Checksum": false,
+"NoSlowHash": false,
+"SlowHashSyncOnly": false,
+"DownloadHash": false
+}
+INFO  : Synching Path1 "{path1/}" with Path2 "{path2/}"
+INFO  : Building Path1 and Path2 listings
+INFO  : Path1 checking for diffs
+INFO  : Path2 checking for diffs
+INFO  : No changes found
+INFO  : Updating listings
+INFO  : Validating listings for Path1 "{path1/}" vs Path2 "{path2/}"
+INFO  : Bisync successful
+
+(34)  : test overlapping path -- should fail
+(35)  : bisync backupdir1={path1/}subdir/backupdir1 backupdir2={path2/}subdir/backupdir2
+INFO  : Setting --ignore-listing-checksum as neither --checksum nor --compare checksum are set.
+INFO  : Bisyncing with Comparison Settings:
+{
+"Modtime": true,
+"Size": true,
+"Checksum": false,
+"NoSlowHash": false,
+"SlowHashSyncOnly": false,
+"DownloadHash": false
+}
+INFO  : Synching Path1 "{path1/}" with Path2 "{path2/}"
+ERROR : Bisync critical error: destination and parameter to --backup-dir mustn't overlap
+ERROR : Bisync aborted. Must run --resync to recover.
+Bisync error: bisync aborted

cmd/bisync/testdata/test_backupdir/initial/RCLONE_TEST

@@ -0,0 +1 @@
+This file is used for testing the health of rclone accesses to the local/remote file system.  Do not delete.

cmd/bisync/testdata/test_backupdir/modfiles/file1.txt

@@ -0,0 +1 @@
+This file is newer

cmd/bisync/testdata/test_backupdir/modfiles/file10.txt

@@ -0,0 +1 @@
+This file is newer

cmd/bisync/testdata/test_backupdir/modfiles/file11.txt

@@ -0,0 +1 @@
+This file is newer

cmd/bisync/testdata/test_backupdir/modfiles/file2.txt

@@ -0,0 +1 @@
+Newer version

cmd/bisync/testdata/test_backupdir/modfiles/file5L.txt

@@ -0,0 +1 @@
+This file is newer and not equal to 5R

cmd/bisync/testdata/test_backupdir/modfiles/file5R.txt

@@ -0,0 +1 @@
+This file is newer and not equal to 5L

cmd/bisync/testdata/test_backupdir/modfiles/file6.txt

@@ -0,0 +1 @@
+This file is newer

cmd/bisync/testdata/test_backupdir/modfiles/file7.txt

@@ -0,0 +1 @@
+This file is newer

cmd/bisync/testdata/test_backupdir/scenario.txt

@@ -0,0 +1,59 @@
+test backupdir
+# Exercise all of the various file change scenarios
+# - New on Path2                            file10
+# - Newer on Path2                          file1
+# - New on Path1                            file11
+# - Newer on Path1                          file2
+# - Deleted on Path2                        file3
+# - Deleted on Path1                        file4
+# - Changed on Path2 and on Path1           file5 (file5r, file5l)
+# - Newer on Path2 and deleted on Path1     file6
+# - Newer on Path1 and deleted on Path2     file7
+# - Deleted on both paths                   file8
+
+test initial bisync
+bisync resync backupdir1={workdir/}backupdirs/backupdir1 backupdir2={workdir/}backupdirs/backupdir2
+
+test make modifications on both paths
+test new on path2 - file10
+touch-copy 2001-01-02 {datadir/}file10.txt {path2/}
+
+test newer on path2 - file1
+touch-copy 2001-01-02 {datadir/}file1.txt {path2/}
+
+test new on path1 - file11
+touch-copy 2001-01-02 {datadir/}file11.txt {path1/}
+
+test newer on path1 - file2
+touch-copy 2001-01-02 {datadir/}file2.txt {path1/}
+
+test deleted on path2 - file3
+delete-file {path2/}file3.txt
+
+test deleted on path1 - file4
+delete-file {path1/}file4.txt
+
+test deleted on both paths - file8
+delete-file {path1/}file8.txt
+delete-file {path2/}file8.txt
+
+test changed on both paths - file5 (file5R, file5L)
+touch-glob 2001-01-02 {datadir/} file5R.txt
+copy-as {datadir/}file5R.txt {path2/} file5.txt
+touch-glob 2001-03-04 {datadir/} file5L.txt
+copy-as {datadir/}file5L.txt {path1/} file5.txt
+
+test newer on path2 and deleted on path1 - file6
+touch-copy 2001-01-02 {datadir/}file6.txt {path2/}
+delete-file {path1/}file6.txt
+
+test newer on path1 and deleted on path2 - file7
+touch-copy 2001-01-02 {datadir/}file7.txt {path1/}
+delete-file {path2/}file7.txt
+
+test bisync run
+bisync backupdir1={workdir/}backupdirs/backupdir1 backupdir2={workdir/}backupdirs/backupdir2
+bisync backupdir1={workdir/}backupdirs/backupdir1 backupdir2={workdir/}backupdirs/backupdir2
+
+test overlapping path -- should fail
+bisync backupdir1={path1/}subdir/backupdir1 backupdir2={path2/}subdir/backupdir2