Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add check command option to consider modtime in addition to size and hash #7429

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/bisync/deltas.go
Expand Up @@ -116,7 +116,7 @@ func (b *bisyncRun) checkconflicts(ctxCheck context.Context, filterCheck *filter
// note that cryptCheck() is not currently exported

fs.Infof(nil, "Checking potential conflicts...")
check := operations.Check(ctxCheck, opt)
check := operations.Check(ctxCheck, false, opt)
fs.Infof(nil, "Finished checking the potential conflicts. %s", check)

//reset error count, because we don't want to count check errors as bisync errors
Expand Down
Expand Up @@ -60,7 +60,7 @@ INFO : - Path2 File was deleted - subdir_with_ࢺ_/filena
INFO : Path2: 9 changes: 5 new, 2 newer, 0 older, 2 deleted
INFO : Applying changes
INFO : Checking potential conflicts...
ERROR : subdir_with_ࢺ_/filechangedbothpaths_ࢺ_.txt: sizes differ
ERROR : subdir_with_ࢺ_/filechangedbothpaths_ࢺ_.txt: size differ
NOTICE: Local file system at {path2}: 1 differences found
NOTICE: Local file system at {path2}: 1 errors while checking
INFO : Finished checking the potential conflicts. 1 differences found
Expand Down
6 changes: 4 additions & 2 deletions cmd/check/check.go
Expand Up @@ -19,6 +19,7 @@ import (

// Globals
var (
modtime = false
download = false
oneway = false
combined = ""
Expand All @@ -33,6 +34,7 @@ var (
func init() {
cmd.Root.AddCommand(commandDefinition)
cmdFlags := commandDefinition.Flags()
flags.BoolVarP(cmdFlags, &modtime, "consider-modtime", "", modtime, "Consider modtime (if available) in addition to size and checksum", "")
flags.BoolVarP(cmdFlags, &download, "download", "", download, "Check by downloading rather than with hash", "")
flags.StringVarP(cmdFlags, &checkFileHashType, "checkfile", "C", checkFileHashType, "Treat source:path as a SUM file with hashes of given type", "")
AddFlags(cmdFlags)
Expand Down Expand Up @@ -191,15 +193,15 @@ the |source:path| must point to a text file in the SUM format.
}

if download {
return operations.CheckDownload(context.Background(), opt)
return operations.CheckDownload(context.Background(), modtime, opt)
}
hashType := fsrc.Hashes().Overlap(fdst.Hashes()).GetOne()
if hashType == hash.None {
fs.Errorf(nil, "No common hash found - not using a hash for checks")
} else {
fs.Infof(nil, "Using %v for hash comparisons", hashType)
}
return operations.Check(context.Background(), opt)
return operations.Check(context.Background(), modtime, opt)
})
return nil
},
Expand Down
2 changes: 1 addition & 1 deletion cmd/test/info/base32768.go
Expand Up @@ -81,7 +81,7 @@ func (r *results) checkBase32768() {
}

// Check local to remote
err = operations.Check(ctx, &operations.CheckOpt{
err = operations.Check(ctx, false, &operations.CheckOpt{
Fdst: fRemote,
Fsrc: fLocal,
})
Expand Down
2 changes: 1 addition & 1 deletion fs/config/configflags/configflags.go
Expand Up @@ -80,7 +80,7 @@ func AddFlags(ci *fs.ConfigInfo, flagSet *pflag.FlagSet) {
flags.BoolVarP(flagSet, &ci.UseServerModTime, "use-server-modtime", "", ci.UseServerModTime, "Use server modified time instead of object metadata", "Config")
flags.BoolVarP(flagSet, &ci.NoGzip, "no-gzip-encoding", "", ci.NoGzip, "Don't set Accept-Encoding: gzip", "Networking")
flags.IntVarP(flagSet, &ci.MaxDepth, "max-depth", "", ci.MaxDepth, "If set limits the recursion depth to this", "Filter")
flags.BoolVarP(flagSet, &ci.IgnoreSize, "ignore-size", "", false, "Ignore size when skipping use mod-time or checksum", "Copy")
flags.BoolVarP(flagSet, &ci.IgnoreSize, "ignore-size", "", false, "Ignore size when skipping, use mod-time or checksum", "Copy")
flags.BoolVarP(flagSet, &ci.IgnoreChecksum, "ignore-checksum", "", ci.IgnoreChecksum, "Skip post copy check of checksums", "Copy")
flags.BoolVarP(flagSet, &ci.IgnoreCaseSync, "ignore-case-sync", "", ci.IgnoreCaseSync, "Ignore case when synchronizing", "Copy")
flags.BoolVarP(flagSet, &ci.NoTraverse, "no-traverse", "", ci.NoTraverse, "Don't traverse destination file system on copy", "Copy")
Expand Down
34 changes: 26 additions & 8 deletions fs/operations/check.go
Expand Up @@ -124,10 +124,13 @@ func (c *checkMarch) checkIdentical(ctx context.Context, dst, src fs.Object) (di
defer func() {
tr.Done(ctx, err)
}()
if sizeDiffers(ctx, src, dst) {
err = fmt.Errorf("sizes differ")
fs.Errorf(src, "%v", err)
return true, false, nil
if !ci.IgnoreSize {
same, compared := CheckSizes(ctx, src, dst)
if !same && compared {
err = fmt.Errorf("size differ")
fs.Errorf(src, "%v", err)
return true, false, nil
}
}
if ci.SizeOnly {
return false, false, nil
Expand Down Expand Up @@ -269,7 +272,7 @@ func (c *checkMarch) reportResults(ctx context.Context, err error) error {
}

// Check the files in fsrc and fdst according to Size and hash
func Check(ctx context.Context, opt *CheckOpt) error {
func Check(ctx context.Context, withModTime bool, opt *CheckOpt) error {
optCopy := *opt
optCopy.Check = func(ctx context.Context, dst, src fs.Object) (differ bool, noHash bool, err error) {
same, ht, err := CheckHashes(ctx, src, dst)
Expand All @@ -284,6 +287,14 @@ func Check(ctx context.Context, opt *CheckOpt) error {
fs.Errorf(src, "%v", err)
return true, false, nil
}
if withModTime {
same, compared := CheckModTimes(ctx, src, dst)
if !same && compared {
err = fmt.Errorf("modtime differ")
fs.Errorf(src, "%v", err)
return true, false, nil
}
}
return false, false, nil
}

Expand Down Expand Up @@ -360,15 +371,22 @@ func checkIdenticalDownload(ctx context.Context, dst, src fs.Object) (differ boo
return
}

// CheckDownload checks the files in fsrc and fdst according to Size
// and the actual contents of the files.
func CheckDownload(ctx context.Context, opt *CheckOpt) error {
// CheckDownload checks the files in fsrc and fdst according actual contents of the files.
func CheckDownload(ctx context.Context, withModTime bool, opt *CheckOpt) error {
optCopy := *opt
optCopy.Check = func(ctx context.Context, a, b fs.Object) (differ bool, noHash bool, err error) {
differ, err = CheckIdenticalDownload(ctx, a, b)
if err != nil {
return true, true, fmt.Errorf("failed to download: %w", err)
}
if withModTime {
sameTime, compared := CheckModTimes(ctx, a, b)
if !sameTime && compared {
err = fmt.Errorf("modtime differ")
fs.Errorf(a, "%v", err)
return true, false, nil
}
}
return differ, false, nil
}
return CheckFn(ctx, &optCopy)
Expand Down
10 changes: 7 additions & 3 deletions fs/operations/check_test.go
Expand Up @@ -180,7 +180,9 @@ func testCheck(t *testing.T, checkFunction func(ctx context.Context, opt *operat
}

func TestCheck(t *testing.T) {
testCheck(t, operations.Check)
testCheck(t, func(ctx context.Context, opt *operations.CheckOpt) error {
return operations.Check(ctx, false, opt)
})
}

func TestCheckFsError(t *testing.T) {
Expand All @@ -198,12 +200,14 @@ func TestCheckFsError(t *testing.T) {
Fsrc: srcFs,
OneWay: false,
}
err = operations.Check(ctx, &opt)
err = operations.Check(ctx, false, &opt)
require.Error(t, err)
}

func TestCheckDownload(t *testing.T) {
testCheck(t, operations.CheckDownload)
testCheck(t, func(ctx context.Context, opt *operations.CheckOpt) error {
return operations.CheckDownload(ctx, false, opt)
})
}

func TestCheckSizeOnly(t *testing.T) {
Expand Down
64 changes: 59 additions & 5 deletions fs/operations/operations.go
Expand Up @@ -39,14 +39,69 @@ import (
"golang.org/x/sync/errgroup"
)

// CheckSizes compares the sizes of two files
//
// Returns:
//
// equal - false if sizes were compared and found different.
// compared - If both of the files had a valid size that could be compared.
func CheckSizes(ctx context.Context, src, dst fs.ObjectInfo) (equal bool, compared bool) {
if src.Size() < 0 {
fs.Debugf(src, "Src size negative - aborting Dst size check")
return true, false
}
if dst.Size() < 0 {
fs.Debugf(src, "Dst size negative - aborting Src size check")
return true, false
}
srcSize := src.Size()
dstSize := dst.Size()
if srcSize == dstSize {
fs.Debugf(src, "size = %d OK", srcSize)
return true, true
}
fs.Debugf(src, "size = %d (%v)", srcSize, src.Fs())
fs.Debugf(dst, "size = %d (%v)", dstSize, dst.Fs())
return false, true
}

// CheckModTimes compare the modification times of two files
//
// Returns:
//
// equal - false if modtimes were compared and found different.
// compared - If both of the files had a valid modtime that could be compared.
func CheckModTimes(ctx context.Context, src, dst fs.ObjectInfo) (equal bool, compared bool) {
if src.Fs().Precision() == fs.ModTimeNotSupported {
fs.Debugf(src, "Src modtime not supported - aborting Dst modtime check")
return true, false
}
if src.Fs().Precision() == fs.ModTimeNotSupported {
fs.Debugf(src, "Dst modtime not supported - aborting Src modtime check")
return true, false
}
srcModTime := src.ModTime(ctx)
dstModTime := dst.ModTime(ctx)
dt := dstModTime.Sub(srcModTime)
modifyWindow := fs.GetModifyWindow(ctx, src.Fs(), dst.Fs())
if dt < modifyWindow && dt > -modifyWindow {
fs.Debugf(src, "modtime = %s OK (differ by %s, within tolerance %s)", srcModTime, dt, modifyWindow)
return true, true
}
fs.Debugf(src, "modtime = %s (%v)", srcModTime, src.Fs())
fs.Debugf(dst, "modtime = %s (%v)", dstModTime, dst.Fs())
return false, true
}

// CheckHashes checks the two files to see if they have common
// known hash types and compares them
//
// Returns.
// Returns:
//
// equal - which is equality of the hashes
// equal - false if a compatible hash was compared and found different.
// Use the ht return value to see if hashes was compared.
//
// hash - the HashType. This is HashNone if either of the hashes were
// ht - the HashType. This is HashNone if either of the hashes were
// unset or a compatible hash couldn't be found.
//
// err - may return an error which will already have been logged
Expand All @@ -62,14 +117,13 @@ func CheckHashes(ctx context.Context, src fs.ObjectInfo, dst fs.Object) (equal b
return equal, ht, err
}

var errNoHash = errors.New("no hash available")

// checkHashes does the work of CheckHashes but takes a hash.Type and
// returns the effective hash type used.
func checkHashes(ctx context.Context, src fs.ObjectInfo, dst fs.Object, ht hash.Type) (equal bool, htOut hash.Type, srcHash, dstHash string, err error) {
// Calculate hashes in parallel
g, ctx := errgroup.WithContext(ctx)
var srcErr, dstErr error
var errNoHash = errors.New("no hash available")
g.Go(func() (err error) {
srcHash, srcErr = src.Hash(ctx, ht)
if srcErr != nil {
Expand Down
4 changes: 2 additions & 2 deletions fs/operations/rc.go
Expand Up @@ -861,10 +861,10 @@ func rcCheck(ctx context.Context, in rc.Params) (out rc.Params, err error) {
err = CheckSum(context.Background(), dstFs, checkFileFs, checkFileRemote, checkFileHashType, opt, download)
} else {
if download {
err = CheckDownload(context.Background(), opt)
err = CheckDownload(context.Background(), false, opt)
} else {
out["hashType"] = srcFs.Hashes().Overlap(dstFs.Hashes()).GetOne().String()
err = Check(context.Background(), opt)
err = Check(context.Background(), false, opt)
}
}
if err != nil {
Expand Down