lib: Remove osutil.Remove & osutil.RemoveAll (fixes #3513)

These are no longer required with Go 1.7. Change made by removing the
functions, doing a global s/osutil.Remove/os.Remove/.

GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/3514

lib/fs/mtimefs_test.go

@@ -12,13 +12,11 @@ import (
 	"os"
 	"testing"
 	"time"
-
-	"github.com/syncthing/syncthing/lib/osutil"
 )
 
 func TestMtimeFS(t *testing.T) {
-	osutil.RemoveAll("testdata")
-	defer osutil.RemoveAll("testdata")
+	os.RemoveAll("testdata")
+	defer os.RemoveAll("testdata")
 	os.Mkdir("testdata", 0755)
 	ioutil.WriteFile("testdata/exists0", []byte("hello"), 0644)
 	ioutil.WriteFile("testdata/exists1", []byte("hello"), 0644)

lib/model/model_test.go

@@ -1343,8 +1343,8 @@ func TestIssue3028(t *testing.T) {
 }
 
 func TestIssue3164(t *testing.T) {
-	osutil.RemoveAll("testdata/issue3164")
-	defer osutil.RemoveAll("testdata/issue3164")
+	os.RemoveAll("testdata/issue3164")
+	defer os.RemoveAll("testdata/issue3164")
 
 	if err := os.MkdirAll("testdata/issue3164/oktodelete/foobar", 0777); err != nil {
 		t.Fatal(err)
@@ -1445,7 +1445,7 @@ func TestIssue2782(t *testing.T) {
 
 	testName := ".syncthing-test." + srand.String(16)
 	testDir := filepath.Join(home, testName)
-	if err := osutil.RemoveAll(testDir); err != nil {
+	if err := os.RemoveAll(testDir); err != nil {
 		t.Skip(err)
 	}
 	if err := osutil.MkdirAll(testDir+"/syncdir", 0755); err != nil {
@@ -1457,7 +1457,7 @@ func TestIssue2782(t *testing.T) {
 	if err := os.Symlink("syncdir", testDir+"/synclink"); err != nil {
 		t.Skip(err)
 	}
-	defer osutil.RemoveAll(testDir)
+	defer os.RemoveAll(testDir)
 
 	db := db.OpenMemory()
 	m := NewModel(defaultConfig, protocol.LocalDeviceID, "device", "syncthing", "dev", db, nil)

lib/model/rwfolder.go

@@ -602,7 +602,7 @@ func (f *rwFolder) handleDir(file protocol.FileInfo) {
 	// Most likely a file/link is getting replaced with a directory.
 	// Remove the file/link and fall through to directory creation.
 	case err == nil && (!info.IsDir() || info.Mode()&os.ModeSymlink != 0):
-		err = osutil.InWritableDir(osutil.Remove, realName)
+		err = osutil.InWritableDir(os.Remove, realName)
 		if err != nil {
 			l.Infof("Puller (folder %q, dir %q): %v", f.folderID, file.Name, err)
 			f.newError(file.Name, err)
@@ -687,13 +687,13 @@ func (f *rwFolder) deleteDir(file protocol.FileInfo, matcher *ignore.Matcher) {
 		for _, dirFile := range files {
 			fullDirFile := filepath.Join(file.Name, dirFile)
 			if defTempNamer.IsTemporary(dirFile) || (matcher != nil && matcher.Match(fullDirFile).IsDeletable()) {
-				osutil.RemoveAll(filepath.Join(f.dir, fullDirFile))
+				os.RemoveAll(filepath.Join(f.dir, fullDirFile))
 			}
 		}
 		dir.Close()
 	}
 
-	err = osutil.InWritableDir(osutil.Remove, realName)
+	err = osutil.InWritableDir(os.Remove, realName)
 	if err == nil || os.IsNotExist(err) {
 		// It was removed or it doesn't exist to start with
 		f.dbUpdates <- dbUpdateJob{file, dbUpdateDeleteDir}
@@ -740,7 +740,7 @@ func (f *rwFolder) deleteFile(file protocol.FileInfo) {
 	} else if f.versioner != nil {
 		err = osutil.InWritableDir(f.versioner.Archive, realName)
 	} else {
-		err = osutil.InWritableDir(osutil.Remove, realName)
+		err = osutil.InWritableDir(os.Remove, realName)
 	}
 
 	if err == nil || os.IsNotExist(err) {
@@ -825,7 +825,7 @@ func (f *rwFolder) renameFile(source, target protocol.FileInfo) {
 		// get rid of. Attempt to delete it instead so that we make *some*
 		// progress. The target is unhandled.
 
-		err = osutil.InWritableDir(osutil.Remove, from)
+		err = osutil.InWritableDir(os.Remove, from)
 		if err != nil {
 			l.Infof("Puller (folder %q, file %q): delete %q after failed rename: %v", f.folderID, target.Name, source.Name, err)
 			f.newError(target.Name, err)
@@ -976,7 +976,7 @@ func (f *rwFolder) handleFile(file protocol.FileInfo, copyChan chan<- copyBlocks
 			// Otherwise, discard the file ourselves in order for the
 			// sharedpuller not to panic when it fails to exclusively create a
 			// file which already exists
-			osutil.InWritableDir(osutil.Remove, tempName)
+			osutil.InWritableDir(os.Remove, tempName)
 		}
 	} else {
 		// Copy the blocks, as we don't want to shuffle them on the FileInfo
@@ -1262,7 +1262,7 @@ func (f *rwFolder) performFinish(state *sharedPullerState) error {
 			// and future hard ignores before attempting a directory delete.
 			// Should share code with f.deletDir().
 
-			if err = osutil.InWritableDir(osutil.Remove, state.realName); err != nil {
+			if err = osutil.InWritableDir(os.Remove, state.realName); err != nil {
 				return err
 			}
 
@@ -1458,14 +1458,14 @@ func removeAvailability(availabilities []Availability, availability Availability
 func (f *rwFolder) moveForConflict(name string) error {
 	if strings.Contains(filepath.Base(name), ".sync-conflict-") {
 		l.Infoln("Conflict for", name, "which is already a conflict copy; not copying again.")
-		if err := osutil.Remove(name); err != nil && !os.IsNotExist(err) {
+		if err := os.Remove(name); err != nil && !os.IsNotExist(err) {
 			return err
 		}
 		return nil
 	}
 
 	if f.maxConflicts == 0 {
-		if err := osutil.Remove(name); err != nil && !os.IsNotExist(err) {
+		if err := os.Remove(name); err != nil && !os.IsNotExist(err) {
 			return err
 		}
 		return nil
@@ -1487,7 +1487,7 @@ func (f *rwFolder) moveForConflict(name string) error {
 		if gerr == nil && len(matches) > f.maxConflicts {
 			sort.Sort(sort.Reverse(sort.StringSlice(matches)))
 			for _, match := range matches[f.maxConflicts:] {
-				gerr = osutil.Remove(match)
+				gerr = os.Remove(match)
 				if gerr != nil {
 					l.Debugln(f, "removing extra conflict", gerr)
 				}

lib/model/sorter.go

@@ -9,9 +9,9 @@ package model
 import (
 	"encoding/binary"
 	"io/ioutil"
+	"os"
 	"sort"
 
-	"github.com/syncthing/syncthing/lib/osutil"
 	"github.com/syncthing/syncthing/lib/protocol"
 	"github.com/syndtr/goleveldb/leveldb"
 	"github.com/syndtr/goleveldb/leveldb/opt"
@@ -186,7 +186,7 @@ func (s *onDiskIndexSorter) Sorted(fn func(protocol.FileInfo) bool) {
 func (s *onDiskIndexSorter) Close() {
 	l.Debugf("onDiskIndexSorter %p closes", s)
 	s.db.Close()
-	osutil.RemoveAll(s.dir)
+	os.RemoveAll(s.dir)
 }
 
 func (s *onDiskIndexSorter) full() bool {

lib/osutil/osutil.go

@@ -15,7 +15,6 @@ import (
 	"path/filepath"
 	"runtime"
 	"strings"
-	"syscall"
 
 	"github.com/calmh/du"
 	"github.com/syncthing/syncthing/lib/sync"
@@ -93,93 +92,6 @@ func InWritableDir(fn func(string) error, path string) error {
 	return fn(path)
 }
 
-// Remove removes the given path. On Windows, removes the read-only attribute
-// from the target prior to deletion.
-func Remove(path string) error {
-	if runtime.GOOS == "windows" {
-		info, err := os.Stat(path)
-		if err != nil {
-			return err
-		}
-		if info.Mode()&0200 == 0 {
-			os.Chmod(path, 0700)
-		}
-	}
-	return os.Remove(path)
-}
-
-// RemoveAll is a copy of os.RemoveAll, but uses osutil.Remove.
-// RemoveAll removes path and any children it contains.
-// It removes everything it can but returns the first error
-// it encounters.  If the path does not exist, RemoveAll
-// returns nil (no error).
-func RemoveAll(path string) error {
-	// Simple case: if Remove works, we're done.
-	err := Remove(path)
-	if err == nil || os.IsNotExist(err) {
-		return nil
-	}
-
-	// Otherwise, is this a directory we need to recurse into?
-	dir, serr := os.Lstat(path)
-	if serr != nil {
-		if serr, ok := serr.(*os.PathError); ok && (os.IsNotExist(serr.Err) || serr.Err == syscall.ENOTDIR) {
-			return nil
-		}
-		return serr
-	}
-	if !dir.IsDir() {
-		// Not a directory; return the error from Remove.
-		return err
-	}
-
-	// Directory.
-	fd, err := os.Open(path)
-	if err != nil {
-		if os.IsNotExist(err) {
-			// Race. It was deleted between the Lstat and Open.
-			// Return nil per RemoveAll's docs.
-			return nil
-		}
-		return err
-	}
-
-	// Remove contents & return first error.
-	err = nil
-	for {
-		names, err1 := fd.Readdirnames(100)
-		for _, name := range names {
-			err1 := RemoveAll(path + string(os.PathSeparator) + name)
-			if err == nil {
-				err = err1
-			}
-		}
-		if err1 == io.EOF {
-			break
-		}
-		// If Readdirnames returned an error, use it.
-		if err == nil {
-			err = err1
-		}
-		if len(names) == 0 {
-			break
-		}
-	}
-
-	// Close directory, because windows won't remove opened directory.
-	fd.Close()
-
-	// Remove directory.
-	err1 := Remove(path)
-	if err1 == nil || os.IsNotExist(err1) {
-		return nil
-	}
-	if err == nil {
-		err = err1
-	}
-	return err
-}
-
 func ExpandTilde(path string) (string, error) {
 	if path == "~" {
 		return getHomeDir()

lib/osutil/osutil_test.go

@@ -71,6 +71,8 @@ func TestInWriteableDir(t *testing.T) {
 }
 
 func TestInWritableDirWindowsRemove(t *testing.T) {
+	// os.Remove should remove read only things on windows
+
 	if runtime.GOOS != "windows" {
 		t.Skipf("Tests not required")
 		return
@@ -100,17 +102,46 @@ func TestInWritableDirWindowsRemove(t *testing.T) {
 	os.Chmod("testdata/windows/ro/readonly", 0500)
 
 	for _, path := range []string{"testdata/windows/ro/readonly", "testdata/windows/ro", "testdata/windows"} {
-		err := os.Remove(path)
-		if err == nil {
-			t.Errorf("Expected error %s", path)
+		err := osutil.InWritableDir(os.Remove, path)
+		if err != nil {
+			t.Errorf("Unexpected error %s: %s", path, err)
 		}
 	}
+}
 
-	for _, path := range []string{"testdata/windows/ro/readonly", "testdata/windows/ro", "testdata/windows"} {
-		err := osutil.InWritableDir(osutil.Remove, path)
+func TestInWritableDirWindowsRemoveAll(t *testing.T) {
+	// os.RemoveAll should remove read only things on windows
+
+	if runtime.GOOS != "windows" {
+		t.Skipf("Tests not required")
+		return
+	}
+
+	err := os.RemoveAll("testdata")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.Chmod("testdata/windows/ro/readonlynew", 0700)
+	defer os.RemoveAll("testdata")
+
+	create := func(name string) error {
+		fd, err := os.Create(name)
 		if err != nil {
-			t.Errorf("Unexpected error %s: %s", path, err)
+			return err
 		}
+		fd.Close()
+		return nil
+	}
+
+	os.Mkdir("testdata", 0700)
+
+	os.Mkdir("testdata/windows", 0500)
+	os.Mkdir("testdata/windows/ro", 0500)
+	create("testdata/windows/ro/readonly")
+	os.Chmod("testdata/windows/ro/readonly", 0500)
+
+	if err := os.RemoveAll("testdata/windows"); err != nil {
+		t.Errorf("Unexpected error: %s", err)
 	}
 }
 

lib/scanner/walk_test.go

@@ -289,8 +289,8 @@ func TestWalkSymlink(t *testing.T) {
 
 	// Create a folder with a symlink in it
 
-	osutil.RemoveAll("_symlinks")
-	defer osutil.RemoveAll("_symlinks")
+	os.RemoveAll("_symlinks")
+	defer os.RemoveAll("_symlinks")
 
 	os.Mkdir("_symlinks", 0755)
 	symlinks.Create("_symlinks/link", "destination", symlinks.TargetUnknown)

lib/versioner/staggered.go

@@ -165,7 +165,7 @@ func (v Staggered) expire(versions []string) {
 			continue
 		}
 
-		if err := osutil.Remove(file); err != nil {
+		if err := os.Remove(file); err != nil {
 			l.Warnf("Versioner: can't remove %q: %v", file, err)
 		}
 	}

lib/versioner/trashcan.go

@@ -144,7 +144,7 @@ func (t *Trashcan) cleanoutArchive() error {
 			// directory was empty and try to remove it. We ignore failure for
 			// the time being.
 			if currentDir != "" && filesInDir == 0 {
-				osutil.Remove(currentDir)
+				os.Remove(currentDir)
 			}
 			currentDir = path
 			filesInDir = 0
@@ -153,7 +153,7 @@ func (t *Trashcan) cleanoutArchive() error {
 
 		if info.ModTime().Before(cutoff) {
 			// The file is too old; remove it.
-			osutil.Remove(path)
+			os.Remove(path)
 		} else {
 			// Keep this file, and remember it so we don't unnecessarily try
 			// to remove this directory.
@@ -169,7 +169,7 @@ func (t *Trashcan) cleanoutArchive() error {
 	// The last directory seen by the walkFn may not have been removed as it
 	// should be.
 	if currentDir != "" && filesInDir == 0 {
-		osutil.Remove(currentDir)
+		os.Remove(currentDir)
 	}
 	return nil
 }

test/util.go

@@ -208,7 +208,7 @@ func alterFiles(dir string) error {
 							return err
 						}
 					} else {
-						err := osutil.Remove(path)
+						err := os.Remove(path)
 						if err != nil {
 							return err
 						}