From 83613634f93e66b8db3ee06d3e832760a2a30e27 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Wed, 14 Aug 2024 18:19:36 +0100 Subject: [PATCH] build: bisync: fix govet lint errors with golangci-lint v1.60.1 There were a lot of instances of this lint error printf: non-constant format string in call to github.com/rclone/rclone/fs.Logf (govet) Most of these could not easily be fixed so had nolint lines added. This should probably be done in a neater way perhaps by making LogColorf/ErrorColorf functions. --- cmd/bisync/checkfn.go | 4 ++-- cmd/bisync/compare.go | 41 ++++++++++++++++++++-------------------- cmd/bisync/operations.go | 36 +++++++++++++++++------------------ cmd/bisync/queue.go | 2 +- cmd/bisync/resolve.go | 2 +- cmd/bisync/resync.go | 2 +- 6 files changed, 43 insertions(+), 44 deletions(-) diff --git a/cmd/bisync/checkfn.go b/cmd/bisync/checkfn.go index 19ac7cd16..9b0b9f144 100644 --- a/cmd/bisync/checkfn.go +++ b/cmd/bisync/checkfn.go @@ -107,10 +107,10 @@ func CryptCheckFn(ctx context.Context, dst, src fs.Object) (differ bool, noHash } if cryptHash != underlyingHash { err = fmt.Errorf("hashes differ (%s:%s) %q vs (%s:%s) %q", fdst.Name(), fdst.Root(), cryptHash, fsrc.Name(), fsrc.Root(), underlyingHash) - fs.Debugf(src, err.Error()) + fs.Debugf(src, "%s", err.Error()) // using same error msg as CheckFn so integration tests match err = fmt.Errorf("%v differ", hashType) - fs.Errorf(src, err.Error()) + fs.Errorf(src, "%s", err.Error()) return true, false, nil } return false, false, nil diff --git a/cmd/bisync/compare.go b/cmd/bisync/compare.go index de7a41ac2..75eac3b30 100644 --- a/cmd/bisync/compare.go +++ b/cmd/bisync/compare.go @@ -62,42 +62,41 @@ func (b *bisyncRun) setCompareDefaults(ctx context.Context) error { b.setHashType(ci) } - // Checks and Warnings if b.opt.Compare.SlowHashSyncOnly && b.opt.Compare.SlowHashDetected && b.opt.Resync { - fs.Logf(nil, Color(terminal.Dim, "Ignoring checksums during --resync as --slow-hash-sync-only is set.")) + fs.Logf(nil, Color(terminal.Dim, "Ignoring checksums during --resync as --slow-hash-sync-only is set.")) ///nolint:govet ci.CheckSum = false // note not setting b.opt.Compare.Checksum = false as we still want to build listings on the non-slow side, if any } else if b.opt.Compare.Checksum && !ci.CheckSum { - fs.Logf(nil, Color(terminal.YellowFg, "WARNING: Checksums will be compared for deltas but not during sync as --checksum is not set.")) + fs.Logf(nil, Color(terminal.YellowFg, "WARNING: Checksums will be compared for deltas but not during sync as --checksum is not set.")) //nolint:govet } if b.opt.Compare.Modtime && (b.fs1.Precision() == fs.ModTimeNotSupported || b.fs2.Precision() == fs.ModTimeNotSupported) { - fs.Logf(nil, Color(terminal.YellowFg, "WARNING: Modtime compare was requested but at least one remote does not support it. It is recommended to use --checksum or --size-only instead.")) + fs.Logf(nil, Color(terminal.YellowFg, "WARNING: Modtime compare was requested but at least one remote does not support it. It is recommended to use --checksum or --size-only instead.")) //nolint:govet } if (ci.CheckSum || b.opt.Compare.Checksum) && b.opt.IgnoreListingChecksum { if (b.opt.Compare.HashType1 == hash.None || b.opt.Compare.HashType2 == hash.None) && !b.opt.Compare.DownloadHash { fs.Logf(nil, Color(terminal.YellowFg, `WARNING: Checksum compare was requested but at least one remote does not support checksums (or checksums are being ignored) and --ignore-listing-checksum is set. Ignoring Checksums globally and falling back to --compare modtime,size for sync. (Use --compare size or --size-only to ignore modtime). Path1 (%s): %s, Path2 (%s): %s`), - b.fs1.String(), b.opt.Compare.HashType1.String(), b.fs2.String(), b.opt.Compare.HashType2.String()) + b.fs1.String(), b.opt.Compare.HashType1.String(), b.fs2.String(), b.opt.Compare.HashType2.String()) //nolint:govet b.opt.Compare.Modtime = true b.opt.Compare.Size = true ci.CheckSum = false b.opt.Compare.Checksum = false } else { - fs.Logf(nil, Color(terminal.YellowFg, "WARNING: Ignoring checksum for deltas as --ignore-listing-checksum is set")) + fs.Logf(nil, Color(terminal.YellowFg, "WARNING: Ignoring checksum for deltas as --ignore-listing-checksum is set")) //nolint:govet // note: --checksum will still affect the internal sync calls } } if !ci.CheckSum && !b.opt.Compare.Checksum && !b.opt.IgnoreListingChecksum { - fs.Infof(nil, Color(terminal.Dim, "Setting --ignore-listing-checksum as neither --checksum nor --compare checksum are set.")) + fs.Infof(nil, Color(terminal.Dim, "Setting --ignore-listing-checksum as neither --checksum nor --compare checksum are set.")) //nolint:govet b.opt.IgnoreListingChecksum = true } if !b.opt.Compare.Size && !b.opt.Compare.Modtime && !b.opt.Compare.Checksum { - return errors.New(Color(terminal.RedFg, "must set a Compare method. (size, modtime, and checksum can't all be false.)")) + return errors.New(Color(terminal.RedFg, "must set a Compare method. (size, modtime, and checksum can't all be false.)")) //nolint:govet } notSupported := func(label string, value bool, opt *bool) { if value { - fs.Logf(nil, Color(terminal.YellowFg, "WARNING: %s is set but bisync does not support it. It will be ignored."), label) + fs.Logf(nil, Color(terminal.YellowFg, "WARNING: %s is set but bisync does not support it. It will be ignored."), label) //nolint:govet *opt = false } } @@ -124,13 +123,13 @@ func sizeDiffers(a, b int64) bool { func hashDiffers(a, b string, ht1, ht2 hash.Type, size1, size2 int64) bool { if a == "" || b == "" { if ht1 != hash.None && ht2 != hash.None && !(size1 <= 0 || size2 <= 0) { - fs.Logf(nil, Color(terminal.YellowFg, "WARNING: hash unexpectedly blank despite Fs support (%s, %s) (you may need to --resync!)"), a, b) + fs.Logf(nil, Color(terminal.YellowFg, "WARNING: hash unexpectedly blank despite Fs support (%s, %s) (you may need to --resync!)"), a, b) //nolint:govet } return false } if ht1 != ht2 { if !(downloadHash && ((ht1 == hash.MD5 && ht2 == hash.None) || (ht1 == hash.None && ht2 == hash.MD5))) { - fs.Infof(nil, Color(terminal.YellowFg, "WARNING: Can't compare hashes of different types (%s, %s)"), ht1.String(), ht2.String()) + fs.Infof(nil, Color(terminal.YellowFg, "WARNING: Can't compare hashes of different types (%s, %s)"), ht1.String(), ht2.String()) //nolint:govet return false } } @@ -152,7 +151,7 @@ func (b *bisyncRun) setHashType(ci *fs.ConfigInfo) { return } } else if b.opt.Compare.SlowHashSyncOnly && b.opt.Compare.SlowHashDetected { - fs.Logf(b.fs2, Color(terminal.YellowFg, "Ignoring --slow-hash-sync-only and falling back to --no-slow-hash as Path1 and Path2 have no hashes in common.")) + fs.Logf(b.fs2, Color(terminal.YellowFg, "Ignoring --slow-hash-sync-only and falling back to --no-slow-hash as Path1 and Path2 have no hashes in common.")) //nolint:govet b.opt.Compare.SlowHashSyncOnly = false b.opt.Compare.NoSlowHash = true ci.CheckSum = false @@ -160,7 +159,7 @@ func (b *bisyncRun) setHashType(ci *fs.ConfigInfo) { } if !b.opt.Compare.DownloadHash && !b.opt.Compare.SlowHashSyncOnly { - fs.Logf(b.fs2, Color(terminal.YellowFg, "--checksum is in use but Path1 and Path2 have no hashes in common; falling back to --compare modtime,size for sync. (Use --compare size or --size-only to ignore modtime)")) + fs.Logf(b.fs2, Color(terminal.YellowFg, "--checksum is in use but Path1 and Path2 have no hashes in common; falling back to --compare modtime,size for sync. (Use --compare size or --size-only to ignore modtime)")) //nolint:govet fs.Infof("Path1 hashes", "%v", b.fs1.Hashes().String()) fs.Infof("Path2 hashes", "%v", b.fs2.Hashes().String()) b.opt.Compare.Modtime = true @@ -168,25 +167,25 @@ func (b *bisyncRun) setHashType(ci *fs.ConfigInfo) { ci.CheckSum = false } if (b.opt.Compare.NoSlowHash || b.opt.Compare.SlowHashSyncOnly) && b.fs1.Features().SlowHash { - fs.Infof(nil, Color(terminal.YellowFg, "Slow hash detected on Path1. Will ignore checksum due to slow-hash settings")) + fs.Infof(nil, Color(terminal.YellowFg, "Slow hash detected on Path1. Will ignore checksum due to slow-hash settings")) //nolint:govet b.opt.Compare.HashType1 = hash.None } else { b.opt.Compare.HashType1 = b.fs1.Hashes().GetOne() if b.opt.Compare.HashType1 != hash.None { - fs.Logf(b.fs1, Color(terminal.YellowFg, "will use %s for same-side diffs on Path1 only"), b.opt.Compare.HashType1) + fs.Logf(b.fs1, Color(terminal.YellowFg, "will use %s for same-side diffs on Path1 only"), b.opt.Compare.HashType1) //nolint:govet } } if (b.opt.Compare.NoSlowHash || b.opt.Compare.SlowHashSyncOnly) && b.fs2.Features().SlowHash { - fs.Infof(nil, Color(terminal.YellowFg, "Slow hash detected on Path2. Will ignore checksum due to slow-hash settings")) + fs.Infof(nil, Color(terminal.YellowFg, "Slow hash detected on Path2. Will ignore checksum due to slow-hash settings")) //nolint:govet b.opt.Compare.HashType1 = hash.None } else { b.opt.Compare.HashType2 = b.fs2.Hashes().GetOne() if b.opt.Compare.HashType2 != hash.None { - fs.Logf(b.fs2, Color(terminal.YellowFg, "will use %s for same-side diffs on Path2 only"), b.opt.Compare.HashType2) + fs.Logf(b.fs2, Color(terminal.YellowFg, "will use %s for same-side diffs on Path2 only"), b.opt.Compare.HashType2) //nolint:govet } } if b.opt.Compare.HashType1 == hash.None && b.opt.Compare.HashType2 == hash.None && !b.opt.Compare.DownloadHash { - fs.Logf(nil, Color(terminal.YellowFg, "WARNING: Ignoring checksums globally as hashes are ignored or unavailable on both sides.")) + fs.Logf(nil, Color(terminal.YellowFg, "WARNING: Ignoring checksums globally as hashes are ignored or unavailable on both sides.")) //nolint:govet b.opt.Compare.Checksum = false ci.CheckSum = false b.opt.IgnoreListingChecksum = true @@ -233,7 +232,7 @@ func (b *bisyncRun) setFromCompareFlag(ctx context.Context) error { b.opt.Compare.Checksum = true CompareFlag.Checksum = true default: - return fmt.Errorf(Color(terminal.RedFg, "unknown compare option: %s (must be size, modtime, or checksum)"), opt) + return fmt.Errorf(Color(terminal.RedFg, "unknown compare option: %s (must be size, modtime, or checksum)"), opt) //nolint:govet } } @@ -285,14 +284,14 @@ func tryDownloadHash(ctx context.Context, o fs.DirEntry, hashVal string) (string } if o.Size() < 0 { downloadHashWarn.Do(func() { - fs.Logf(o, Color(terminal.YellowFg, "Skipping hash download as checksum not reliable with files of unknown length.")) + fs.Logf(o, Color(terminal.YellowFg, "Skipping hash download as checksum not reliable with files of unknown length.")) //nolint:govet }) fs.Debugf(o, "Skipping hash download as checksum not reliable with files of unknown length.") return hashVal, hash.ErrUnsupported } firstDownloadHash.Do(func() { - fs.Infof(obj.Fs().Name(), Color(terminal.Dim, "Downloading hashes...")) + fs.Infof(obj.Fs().Name(), Color(terminal.Dim, "Downloading hashes...")) //nolint:govet }) tr := accounting.Stats(ctx).NewCheckingTransfer(o, "computing hash with --download-hash") defer func() { diff --git a/cmd/bisync/operations.go b/cmd/bisync/operations.go index 4b5a288b1..a1787edaa 100644 --- a/cmd/bisync/operations.go +++ b/cmd/bisync/operations.go @@ -131,18 +131,18 @@ func Bisync(ctx context.Context, fs1, fs2 fs.Fs, optArg *Options) (err error) { finaliseOnce.Do(func() { if atexit.Signalled() { if b.opt.Resync { - fs.Logf(nil, Color(terminal.GreenFg, "No need to gracefully shutdown during --resync (just run it again.)")) + fs.Logf(nil, Color(terminal.GreenFg, "No need to gracefully shutdown during --resync (just run it again.)")) //nolint:govet } else { - fs.Logf(nil, Color(terminal.YellowFg, "Attempting to gracefully shutdown. (Send exit signal again for immediate un-graceful shutdown.)")) + fs.Logf(nil, Color(terminal.YellowFg, "Attempting to gracefully shutdown. (Send exit signal again for immediate un-graceful shutdown.)")) //nolint:govet b.InGracefulShutdown = true if b.SyncCI != nil { - fs.Infof(nil, Color(terminal.YellowFg, "Telling Sync to wrap up early.")) + fs.Infof(nil, Color(terminal.YellowFg, "Telling Sync to wrap up early.")) //nolint:govet b.SyncCI.MaxTransfer = 1 b.SyncCI.MaxDuration = 1 * time.Second b.SyncCI.CutoffMode = fs.CutoffModeSoft gracePeriod := 30 * time.Second // TODO: flag to customize this? if !waitFor("Canceling Sync if not done in", gracePeriod, func() bool { return b.CleanupCompleted }) { - fs.Logf(nil, Color(terminal.YellowFg, "Canceling sync and cleaning up")) + fs.Logf(nil, Color(terminal.YellowFg, "Canceling sync and cleaning up")) //nolint:govet b.CancelSync() waitFor("Aborting Bisync if not done in", 60*time.Second, func() bool { return b.CleanupCompleted }) } @@ -150,13 +150,13 @@ func Bisync(ctx context.Context, fs1, fs2 fs.Fs, optArg *Options) (err error) { // we haven't started to sync yet, so we're good. // no need to worry about the listing files, as we haven't overwritten them yet. b.CleanupCompleted = true - fs.Logf(nil, Color(terminal.GreenFg, "Graceful shutdown completed successfully.")) + fs.Logf(nil, Color(terminal.GreenFg, "Graceful shutdown completed successfully.")) //nolint:govet } } if !b.CleanupCompleted { if !b.opt.Resync { - fs.Logf(nil, Color(terminal.HiRedFg, "Graceful shutdown failed.")) - fs.Logf(nil, Color(terminal.RedFg, "Bisync interrupted. Must run --resync to recover.")) + fs.Logf(nil, Color(terminal.HiRedFg, "Graceful shutdown failed.")) //nolint:govet + fs.Logf(nil, Color(terminal.RedFg, "Bisync interrupted. Must run --resync to recover.")) //nolint:govet } markFailed(b.listing1) markFailed(b.listing2) @@ -180,14 +180,14 @@ func Bisync(ctx context.Context, fs1, fs2 fs.Fs, optArg *Options) (err error) { b.critical = false } if err == nil { - fs.Logf(nil, Color(terminal.GreenFg, "Graceful shutdown completed successfully.")) + fs.Logf(nil, Color(terminal.GreenFg, "Graceful shutdown completed successfully.")) //nolint:govet } } if b.critical { if b.retryable && b.opt.Resilient { - fs.Errorf(nil, Color(terminal.RedFg, "Bisync critical error: %v"), err) - fs.Errorf(nil, Color(terminal.YellowFg, "Bisync aborted. Error is retryable without --resync due to --resilient mode.")) + fs.Errorf(nil, Color(terminal.RedFg, "Bisync critical error: %v"), err) //nolint:govet + fs.Errorf(nil, Color(terminal.YellowFg, "Bisync aborted. Error is retryable without --resync due to --resilient mode.")) //nolint:govet } else { if bilib.FileExists(b.listing1) { _ = os.Rename(b.listing1, b.listing1+"-err") @@ -196,15 +196,15 @@ func Bisync(ctx context.Context, fs1, fs2 fs.Fs, optArg *Options) (err error) { _ = os.Rename(b.listing2, b.listing2+"-err") } fs.Errorf(nil, Color(terminal.RedFg, "Bisync critical error: %v"), err) - fs.Errorf(nil, Color(terminal.RedFg, "Bisync aborted. Must run --resync to recover.")) + fs.Errorf(nil, Color(terminal.RedFg, "Bisync aborted. Must run --resync to recover.")) //nolint:govet } return ErrBisyncAborted } if b.abort && !b.InGracefulShutdown { - fs.Logf(nil, Color(terminal.RedFg, "Bisync aborted. Please try again.")) + fs.Logf(nil, Color(terminal.RedFg, "Bisync aborted. Please try again.")) //nolint:govet } if err == nil { - fs.Infof(nil, Color(terminal.GreenFg, "Bisync successful")) + fs.Infof(nil, Color(terminal.GreenFg, "Bisync successful")) //nolint:govet } return err } @@ -270,7 +270,7 @@ func (b *bisyncRun) runLocked(octx context.Context) (err error) { if b.opt.Recover && bilib.FileExists(b.listing1+"-old") && bilib.FileExists(b.listing2+"-old") { errTip := fmt.Sprintf(Color(terminal.CyanFg, "Path1: %s\n"), Color(terminal.HiBlueFg, b.listing1)) errTip += fmt.Sprintf(Color(terminal.CyanFg, "Path2: %s"), Color(terminal.HiBlueFg, b.listing2)) - fs.Logf(nil, Color(terminal.YellowFg, "Listings not found. Reverting to prior backup as --recover is set. \n")+errTip) + fs.Logf(nil, Color(terminal.YellowFg, "Listings not found. Reverting to prior backup as --recover is set. \n")+errTip) //nolint:govet if opt.CheckSync != CheckSyncFalse { // Run CheckSync to ensure old listing is valid (garbage in, garbage out!) fs.Infof(nil, "Validating backup listings for Path1 %s vs Path2 %s", quotePath(path1), quotePath(path2)) @@ -279,7 +279,7 @@ func (b *bisyncRun) runLocked(octx context.Context) (err error) { b.retryable = true return err } - fs.Infof(nil, Color(terminal.GreenFg, "Backup listing is valid.")) + fs.Infof(nil, Color(terminal.GreenFg, "Backup listing is valid.")) //nolint:govet } b.revertToOldListings() } else { @@ -299,7 +299,7 @@ func (b *bisyncRun) runLocked(octx context.Context) (err error) { fs.Infof(nil, "Building Path1 and Path2 listings") ls1, ls2, err = b.makeMarchListing(fctx) if err != nil || accounting.Stats(fctx).Errored() { - fs.Errorf(nil, Color(terminal.RedFg, "There were errors while building listings. Aborting as it is too dangerous to continue.")) + fs.Errorf(nil, Color(terminal.RedFg, "There were errors while building listings. Aborting as it is too dangerous to continue.")) //nolint:govet b.critical = true b.retryable = true return err @@ -569,7 +569,7 @@ func (b *bisyncRun) setBackupDir(ctx context.Context, destPath int) context.Cont 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.")) + err = errors.New(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 @@ -625,7 +625,7 @@ func (b *bisyncRun) checkSyntax() error { 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)) + fs.Infof(Color(terminal.MagentaBg, "DEBUGNAME "+b.DebugName), Color(terminal.MagentaBg, msgiftrue)) //nolint:govet } } diff --git a/cmd/bisync/queue.go b/cmd/bisync/queue.go index f5617ed8a..665da3d5f 100644 --- a/cmd/bisync/queue.go +++ b/cmd/bisync/queue.go @@ -161,7 +161,7 @@ func WriteResults(ctx context.Context, sigil operations.Sigil, src, dst fs.DirEn prettyprint(result, "writing result", fs.LogLevelDebug) if result.Size < 0 && result.Flags != "d" && ((queueCI.CheckSum && !downloadHash) || queueCI.SizeOnly) { once.Do(func() { - fs.Logf(result.Name, Color(terminal.YellowFg, "Files of unknown size (such as Google Docs) do not sync reliably with --checksum or --size-only. Consider using modtime instead (the default) or --drive-skip-gdocs")) + fs.Logf(result.Name, Color(terminal.YellowFg, "Files of unknown size (such as Google Docs) do not sync reliably with --checksum or --size-only. Consider using modtime instead (the default) or --drive-skip-gdocs")) //nolint:govet }) } diff --git a/cmd/bisync/resolve.go b/cmd/bisync/resolve.go index 83b60044b..6544d18b9 100644 --- a/cmd/bisync/resolve.go +++ b/cmd/bisync/resolve.go @@ -142,7 +142,7 @@ func (b *bisyncRun) resolve(ctxMove context.Context, path1, path2, file, alias s if winningPath > 0 { fs.Infof(file, Color(terminal.GreenFg, "The winner is: Path%d"), winningPath) } else { - fs.Infof(file, Color(terminal.RedFg, "A winner could not be determined.")) + fs.Infof(file, Color(terminal.RedFg, "A winner could not be determined.")) //nolint:govet } } diff --git a/cmd/bisync/resync.go b/cmd/bisync/resync.go index b3fc7ed8d..d7ef97212 100644 --- a/cmd/bisync/resync.go +++ b/cmd/bisync/resync.go @@ -15,7 +15,7 @@ import ( // and either flag is sufficient without the other. func (b *bisyncRun) setResyncDefaults() { if b.opt.Resync && b.opt.ResyncMode == PreferNone { - fs.Debugf(nil, Color(terminal.Dim, "defaulting to --resync-mode path1 as --resync is set")) + fs.Debugf(nil, Color(terminal.Dim, "defaulting to --resync-mode path1 as --resync is set")) //nolint:govet b.opt.ResyncMode = PreferPath1 } if b.opt.ResyncMode != PreferNone {