Make --delete-after the default and refactor --delete-{before,during,after} parsing

docs/content/docs.md

@@ -507,6 +507,10 @@ to the console.
 Note that `--track-renames` is incompatible with `--no-traverse` and
 that it uses extra memory to keep track of all the rename candidates.
 
+Note also that `--track-renames` is incompatible with
+`--delete-before` and will select `--delete-after` instead of
+`--delete-during`.
+
 ### --delete-(before,during,after) ###
 
 This option allows you to specify when files on your destination are
@@ -514,16 +518,21 @@ deleted when you sync folders.
 
 Specifying the value `--delete-before` will delete all files present
 on the destination, but not on the source *before* starting the
-transfer of any new or updated files.  This uses extra memory as it
+transfer of any new or updated files. This uses two passes through the
-has to store the source listing before proceeding.
+file systems, one for the deletions and one for the copies.
 
-Specifying `--delete-during` (default value) will delete files while
+Specifying `--delete-during` will delete files while checking and
-checking and uploading files. This is usually the fastest option.
+uploading files. This is the fastest option and uses the least memory.
-Currently this works the same as `--delete-after` but it may change in
-the future.
 
-Specifying `--delete-after` will delay deletion of files until all new/updated
+Specifying `--delete-after` (the default value) will delay deletion of
-files have been successfully transfered.
+files until all new/updated files have been successfully transfered.
+The files to be deleted are collected in the copy pass then deleted
+after the copy pass has completed sucessfully.  The files to be
+deleted are held in memory so this mode may use more memory.  This is
+the safest mode as it will only delete files if there have been no
+errors subsequent to that.  If there have been errors before the
+deletions start then you will get the message `not deleting files as
+there were IO errors`.
 
 ### --timeout=TIME ###
 

fs/config.go

@@ -199,9 +199,7 @@ type ConfigInfo struct {
 	DumpAuth           bool
 	Filter             *Filter
 	InsecureSkipVerify bool // Skip server certificate verification
-	DeleteBefore       bool // Delete before checking
+	DeleteMode         DeleteMode
-	DeleteDuring       bool // Delete during checking/transfer
-	DeleteAfter        bool // Delete after successful transfer.
 	TrackRenames       bool // Track file renames.
 	LowLevelRetries    int
 	UpdateOlder        bool // Skip files that are newer on the destination
@@ -284,6 +282,19 @@ func makeConfigPath() string {
 	return hiddenConfigFileName
 }
 
+// DeleteMode describes the possible delete modes in the config
+type DeleteMode byte
+
+// DeleteMode constants
+const (
+	DeleteModeOff DeleteMode = iota
+	DeleteModeBefore
+	DeleteModeDuring
+	DeleteModeAfter
+	DeleteModeOnly
+	DeleteModeDefault = DeleteModeAfter
+)
+
 // LoadConfig loads the config file
 func LoadConfig() {
 	// Read some flags if set
@@ -318,20 +329,20 @@ func LoadConfig() {
 
 	ConfigPath = *configFile
 
-	Config.DeleteBefore = *deleteBefore
-	Config.DeleteDuring = *deleteDuring
-	Config.DeleteAfter = *deleteAfter
-
 	Config.TrackRenames = *trackRenames
 
 	switch {
 	case *deleteBefore && (*deleteDuring || *deleteAfter),
 		*deleteDuring && *deleteAfter:
 		log.Fatalf(`Only one of --delete-before, --delete-during or --delete-after can be used.`)
-
+	case *deleteBefore:
-	// If none are specified, use "during".
+		Config.DeleteMode = DeleteModeBefore
-	case !*deleteBefore && !*deleteDuring && !*deleteAfter:
+	case *deleteDuring:
-		Config.DeleteDuring = true
+		Config.DeleteMode = DeleteModeDuring
+	case *deleteAfter:
+		Config.DeleteMode = DeleteModeAfter
+	default:
+		Config.DeleteMode = DeleteModeDefault
 	}
 
 	if Config.IgnoreSize && Config.SizeOnly {

fs/sync.go

@@ -12,14 +12,13 @@ import (
 
 type syncCopyMove struct {
 	// parameters
-	fdst   Fs
+	fdst       Fs
-	fsrc   Fs
+	fsrc       Fs
-	Delete bool
+	deleteMode DeleteMode // how we are doing deletions
-	DoMove bool
+	DoMove     bool
-	dir    string
+	dir        string
 	// internal state
 	noTraverse     bool                // if set don't trafevers the dst
-	deleteBefore   bool                // set if we must delete objects before copying
 	trackRenames   bool                // set if we should do server side renames
 	dstFilesMu     sync.Mutex          // protect dstFiles
 	dstFiles       map[string]Object   // dst files, always filled
@@ -48,11 +47,11 @@ type syncCopyMove struct {
 	suffix         string              // suffix to add to files placed in backupDir
 }
 
-func newSyncCopyMove(fdst, fsrc Fs, Delete bool, DoMove bool) (*syncCopyMove, error) {
+func newSyncCopyMove(fdst, fsrc Fs, deleteMode DeleteMode, DoMove bool) (*syncCopyMove, error) {
 	s := &syncCopyMove{
 		fdst:           fdst,
 		fsrc:           fsrc,
-		Delete:         Delete,
+		deleteMode:     deleteMode,
 		DoMove:         DoMove,
 		dir:            "",
 		srcFilesChan:   make(chan Object, Config.Checkers+Config.Transfers),
@@ -62,13 +61,12 @@ func newSyncCopyMove(fdst, fsrc Fs, Delete bool, DoMove bool) (*syncCopyMove, er
 		abort:          make(chan struct{}),
 		toBeChecked:    make(ObjectPairChan, Config.Transfers),
 		toBeUploaded:   make(ObjectPairChan, Config.Transfers),
-		deleteBefore:   Delete && Config.DeleteBefore,
 		trackRenames:   Config.TrackRenames,
 		commonHash:     fsrc.Hashes().Overlap(fdst.Hashes()).GetOne(),
 		toBeRenamed:    make(ObjectPairChan, Config.Transfers),
 		trackRenamesCh: make(chan Object, Config.Checkers),
 	}
-	if s.noTraverse && s.Delete {
+	if s.noTraverse && s.deleteMode != DeleteModeOff {
 		Debug(s.fdst, "Ignoring --no-traverse with sync")
 		s.noTraverse = false
 	}
@@ -83,13 +81,15 @@ func newSyncCopyMove(fdst, fsrc Fs, Delete bool, DoMove bool) (*syncCopyMove, er
 			s.trackRenames = false
 		}
 	}
-	if s.deleteBefore && s.trackRenames {
+	if s.trackRenames {
-		ErrorLog(fdst, "Ignoring --delete-before with --track-renames - using --delete-after")
+		// track renames needs delete after
-		s.deleteBefore = false
+		if s.deleteMode != DeleteModeOff {
-	}
+			s.deleteMode = DeleteModeAfter
-	if s.noTraverse && s.trackRenames {
+		}
-		Debug(s.fdst, "Ignoring --no-traverse with --track-renames")
+		if s.noTraverse {
-		s.noTraverse = false
+			Debug(s.fdst, "Ignoring --no-traverse with --track-renames")
+			s.noTraverse = false
+		}
 	}
 	// Make Fs for --backup-dir if required
 	if Config.BackupDir != "" {
@@ -614,8 +614,8 @@ func (s *syncCopyMove) run() error {
 		go s.readDstFiles()
 	}
 
-	// If s.deleteBefore then we need to read the whole source map first
+	// If --delete-before then we need to read the whole source map first
-	readSourceMap := s.deleteBefore
+	readSourceMap := s.deleteMode == DeleteModeBefore
 
 	if readSourceMap {
 		// Read source files into the map
@@ -636,7 +636,7 @@ func (s *syncCopyMove) run() error {
 	}
 
 	// Delete files first if required
-	if s.deleteBefore {
+	if s.deleteMode == DeleteModeBefore {
 		err = s.deleteFiles(true)
 		if err != nil {
 			return err
@@ -710,7 +710,7 @@ func (s *syncCopyMove) run() error {
 	err = <-s.srcFilesResult
 
 	// Delete files during or after
-	if s.Delete && (Config.DeleteDuring || Config.DeleteAfter) {
+	if s.deleteMode == DeleteModeDuring || s.deleteMode == DeleteModeAfter {
 		if err != nil {
 			ErrorLog(s.fdst, "%v", ErrorNotDeleting)
 		} else {
@@ -721,31 +721,37 @@ func (s *syncCopyMove) run() error {
 	return s.currentError()
 }
 
-// Sync fsrc into fdst
+// Syncs fsrc into fdst
-func Sync(fdst, fsrc Fs) error {
+//
-	do, err := newSyncCopyMove(fdst, fsrc, true, false)
+// If Delete is true then it deletes any files in fdst that aren't in fsrc
+//
+// If DoMove is true then files will be moved instead of copied
+//
+// dir is the start directory, "" for root
+func runSyncCopyMove(fdst, fsrc Fs, deleteMode DeleteMode, DoMove bool) error {
+	if deleteMode != DeleteModeOff && DoMove {
+		return errors.New("can't delete and move at the same time")
+	}
+	do, err := newSyncCopyMove(fdst, fsrc, deleteMode, DoMove)
 	if err != nil {
 		return err
 	}
 	return do.run()
 }
 
+// Sync fsrc into fdst
+func Sync(fdst, fsrc Fs) error {
+	return runSyncCopyMove(fdst, fsrc, Config.DeleteMode, false)
+}
+
 // CopyDir copies fsrc into fdst
 func CopyDir(fdst, fsrc Fs) error {
-	do, err := newSyncCopyMove(fdst, fsrc, false, false)
+	return runSyncCopyMove(fdst, fsrc, DeleteModeOff, false)
-	if err != nil {
-		return err
-	}
-	return do.run()
 }
 
 // moveDir moves fsrc into fdst
 func moveDir(fdst, fsrc Fs) error {
-	do, err := newSyncCopyMove(fdst, fsrc, false, true)
+	return runSyncCopyMove(fdst, fsrc, DeleteModeOff, true)
-	if err != nil {
-		return err
-	}
-	return do.run()
 }
 
 // MoveDir moves fsrc into fdst

fs/sync_test.go

@@ -475,38 +475,28 @@ func TestSyncAfterRemovingAFileAndAddingAFileWithErrors(t *testing.T) {
 	fstest.CheckItems(t, r.fremote, file1, file2, file3)
 }
 
-// Sync test delete during
+// Sync test delete after
-func TestSyncDeleteDuring(t *testing.T) {
+func TestSyncDeleteAfter(t *testing.T) {
 	// This is the default so we've checked this already
 	// check it is the default
-	if !(!fs.Config.DeleteBefore && fs.Config.DeleteDuring && !fs.Config.DeleteAfter) {
+	require.Equal(t, fs.Config.DeleteMode, fs.DeleteModeAfter, "Didn't default to --delete-after")
-		t.Fatalf("Didn't default to --delete-during")
-	}
 }
 
-// Sync test delete before
+// Sync test delete during
-func TestSyncDeleteBefore(t *testing.T) {
+func TestSyncDeleteDuring(t *testing.T) {
-	fs.Config.DeleteBefore = true
+	fs.Config.DeleteMode = fs.DeleteModeDuring
-	fs.Config.DeleteDuring = false
-	fs.Config.DeleteAfter = false
 	defer func() {
-		fs.Config.DeleteBefore = false
+		fs.Config.DeleteMode = fs.DeleteModeDefault
-		fs.Config.DeleteDuring = true
-		fs.Config.DeleteAfter = false
 	}()
 
 	TestSyncAfterRemovingAFileAndAddingAFile(t)
 }
 
-// Sync test delete after
+// Sync test delete before
-func TestSyncDeleteAfter(t *testing.T) {
+func TestSyncDeleteBefore(t *testing.T) {
-	fs.Config.DeleteBefore = false
+	fs.Config.DeleteMode = fs.DeleteModeBefore
-	fs.Config.DeleteDuring = false
-	fs.Config.DeleteAfter = true
 	defer func() {
-		fs.Config.DeleteBefore = false
+		fs.Config.DeleteMode = fs.DeleteModeDefault
-		fs.Config.DeleteDuring = true
-		fs.Config.DeleteAfter = false
 	}()
 
 	TestSyncAfterRemovingAFileAndAddingAFile(t)