bisync: normalize session name to non-canonical - fixes #7423

Before this change, bisync used the "canonical" Fs name in the filename for its
listing files, including any {hexstring} suffix. An unintended consequence of
this was that if a user added a backend-specific flag from the command line
(thus "overriding" the config), bisync would fail to find the listing files it
created during the prior run without this flag, due to the path now having a
{hexstring} suffix that wasn't there before (or vice versa, if the flag was
present when the session was established, and later removed.) This would
sometimes cause bisync to fail with a critical error (if no listing existed
with the alternate name), or worse -- it would sometimes cause bisync to use an
old, incorrect listing (if old listings with the alternate name DID still
exist, from before the user changed their flags.)

After this change, the issue is fixed by always normalizing the SessionName to
the non-canonical version (no {hexstring} suffix), regardless of the flags. To
avoid a breaking change, we first check if a suffixed listing exists. If so, we
rename it (and overwrite the non-suffixed version, if any.) If not, we carry on
with the non-suffixed version. (We should only find a suffixed version if
created prior to this commit.)

The result for the user is that the same pair of paths will always use the same
.lst filenames, with or without backend-specific flags.

cmd/bisync/bilib/canonical.go

@@ -2,12 +2,15 @@
 package bilib
 
 import (
+	"context"
 	"os"
+	"path/filepath"
 	"regexp"
 	"runtime"
 	"strings"
 
 	"github.com/rclone/rclone/fs"
+	"github.com/rclone/rclone/fs/operations"
 )
 
 // FsPath converts Fs to a suitable rclone argument
@@ -38,5 +41,57 @@ var nonCanonicalChars = regexp.MustCompile(`[\s\\/:?*]`)
 
 // SessionName makes a unique base name for the sync operation
 func SessionName(fs1, fs2 fs.Fs) string {
-	return CanonicalPath(FsPath(fs1)) + ".." + CanonicalPath(FsPath(fs2))
+	return StripHexString(CanonicalPath(FsPath(fs1))) + ".." + StripHexString(CanonicalPath(FsPath(fs2)))
+}
+
+// StripHexString strips the (first) canonical {hexstring} suffix
+func StripHexString(path string) string {
+	open := strings.IndexRune(path, '{')
+	close := strings.IndexRune(path, '}')
+	if open >= 0 && close > open {
+		return path[:open] + path[close+1:] // (trailing underscore)
+	}
+	return path
+}
+
+// HasHexString returns true if path contains at least one canonical {hexstring} suffix
+func HasHexString(path string) bool {
+	open := strings.IndexRune(path, '{')
+	if open >= 0 && strings.IndexRune(path, '}') > open {
+		return true
+	}
+	return false
+}
+
+// BasePath joins the workDir with the SessionName, stripping {hexstring} suffix if necessary
+func BasePath(ctx context.Context, workDir string, fs1, fs2 fs.Fs) string {
+	suffixedSession := CanonicalPath(FsPath(fs1)) + ".." + CanonicalPath(FsPath(fs2))
+	suffixedBasePath := filepath.Join(workDir, suffixedSession)
+	listing1 := suffixedBasePath + ".path1.lst"
+	listing2 := suffixedBasePath + ".path2.lst"
+
+	sessionName := SessionName(fs1, fs2)
+	basePath := filepath.Join(workDir, sessionName)
+
+	// Normalize to non-canonical version for overridden configs
+	// to ensure that backend-specific flags don't change the listing filename.
+	// For backward-compatibility, we first check if we found a listing file with the suffixed version.
+	// If so, we rename it (and overwrite non-suffixed version, if any.)
+	// If not, we carry on with the non-suffixed version.
+	// We should only find a suffixed version if bisync v1.66 or older created it.
+	if HasHexString(suffixedSession) && FileExists(listing1) {
+		fs.Infof(listing1, "renaming to: %s", basePath+".path1.lst")
+		if !operations.SkipDestructive(ctx, listing1, "rename to "+basePath+".path1.lst") {
+			_ = os.Rename(listing1, basePath+".path1.lst")
+		}
+	}
+	if HasHexString(suffixedSession) && FileExists(listing2) {
+		fs.Infof(listing2, "renaming to: %s", basePath+".path2.lst")
+		if !operations.SkipDestructive(ctx, listing1, "rename to "+basePath+".path2.lst") {
+			_ = os.Rename(listing2, basePath+".path2.lst")
+		} else {
+			return suffixedBasePath
+		}
+	}
+	return basePath
 }

cmd/bisync/operations.go

@@ -88,7 +88,7 @@ func Bisync(ctx context.Context, fs1, fs2 fs.Fs, optArg *Options) (err error) {
 	}
 
 	// Produce a unique name for the sync operation
-	b.basePath = filepath.Join(b.workDir, bilib.SessionName(b.fs1, b.fs2))
+	b.basePath = bilib.BasePath(ctx, b.workDir, b.fs1, b.fs2)
 	b.listing1 = b.basePath + ".path1.lst"
 	b.listing2 = b.basePath + ".path2.lst"
 	b.newListing1 = b.listing1 + "-new"

docs/content/bisync.md

@@ -614,19 +614,6 @@ and there is also a [known issue concerning Google Drive users with many empty d
 For now, the recommended way to avoid using `--fast-list` is to add `--disable ListR` 
 to all bisync commands. The default behavior may change in a future version.
 
-### Overridden Configs
-
-When rclone detects an overridden config, it adds a suffix like `{ABCDE}` on the fly 
-to the internal name of the remote. Bisync follows suit by including this suffix in its listing filenames. 
-However, this suffix does not necessarily persist from run to run, especially if different flags are provided. 
-So if next time the suffix assigned is `{FGHIJ}`, bisync will get confused, 
-because it's looking for a listing file with `{FGHIJ}`, when the file it wants has `{ABCDE}`. 
-As a result, it throws 
-`Bisync critical error: cannot find prior Path1 or Path2 listings, likely due to critical error on prior run` 
-and refuses to run again until the user runs a `--resync` (unless using `--resilient`). 
-The best workaround at the moment is to set any backend-specific flags in the [config file](/commands/rclone_config/) 
-instead of specifying them with command flags. (You can still override them as needed for other rclone commands.)
-
 ### Case (and unicode) sensitivity {#case-sensitivity}
 
 As of `v1.66`, case and unicode form differences no longer cause critical errors,
@@ -974,10 +961,6 @@ skip them.) To work around this, use the default (modtime and size) instead of
 To ignore Google Docs entirely, use
 [`--drive-skip-gdocs`](/drive/#drive-skip-gdocs).
 
-(Note that all flags starting with `--drive` are backend-specific, and
-therefore will cause the behavior explained in [Overridden
-Configs](/#overridden-configs).)
-
 ## Usage examples
 
 ### Cron {#cron}
@@ -1369,6 +1352,7 @@ for performance improvements and less [risk of error](https://forum.rclone.org/t
 * Google Docs (and other files of unknown size) are now supported (with the same options as in `sync`)
 * Equality checks before a sync conflict rename now fall back to `cryptcheck` (when possible) or `--download`,
 instead of of `--size-only`, when `check` is not available.
+* Bisync no longer fails to find the correct listing file when configs are overridden with backend-specific flags.
 
 ### `v1.64`
 * Fixed an [issue](https://forum.rclone.org/t/bisync-bugs-and-feature-requests/37636#:~:text=1.%20Dry%20runs%20are%20not%20completely%20dry)