Merge pull request #2614 from greatroar/simplify-fs

Simplify internal/fs

cmd/restic/integration_test.go

@@ -21,7 +21,6 @@ import (
 
 	"github.com/restic/restic/internal/errors"
 	"github.com/restic/restic/internal/filter"
-	"github.com/restic/restic/internal/fs"
 	"github.com/restic/restic/internal/repository"
 	"github.com/restic/restic/internal/restic"
 	rtest "github.com/restic/restic/internal/test"
@@ -66,7 +65,7 @@ func testRunBackupAssumeFailure(t testing.TB, dir string, target []string, opts
 	gopts.stdout = ioutil.Discard
 	t.Logf("backing up %v in %v", target, dir)
 	if dir != "" {
-		cleanup := fs.TestChdir(t, dir)
+		cleanup := rtest.Chdir(t, dir)
 		defer cleanup()
 	}
 
@@ -1035,7 +1034,7 @@ func TestRestoreLatest(t *testing.T) {
 
 	// chdir manually here so we can get the current directory. This is not the
 	// same as the temp dir returned by ioutil.TempDir() on darwin.
-	back := fs.TestChdir(t, filepath.Dir(env.testdata))
+	back := rtest.Chdir(t, filepath.Dir(env.testdata))
 	defer back()
 
 	curdir, err := os.Getwd()

internal/archiver/archiver_test.go

@@ -803,7 +803,7 @@ func TestArchiverSaveDir(t *testing.T) {
 				chdir = filepath.Join(chdir, test.chdir)
 			}
 
-			back := fs.TestChdir(t, chdir)
+			back := restictest.Chdir(t, chdir)
 			defer back()
 
 			fi, err := fs.Lstat(test.target)
@@ -1063,7 +1063,7 @@ func TestArchiverSaveTree(t *testing.T) {
 
 			arch.runWorkers(ctx, &tmb)
 
-			back := fs.TestChdir(t, tempdir)
+			back := restictest.Chdir(t, tempdir)
 			defer back()
 
 			if test.prepare != nil {
@@ -1353,7 +1353,7 @@ func TestArchiverSnapshot(t *testing.T) {
 				chdir = filepath.Join(chdir, filepath.FromSlash(test.chdir))
 			}
 
-			back := fs.TestChdir(t, chdir)
+			back := restictest.Chdir(t, chdir)
 			defer back()
 
 			var targets []string
@@ -1507,7 +1507,7 @@ func TestArchiverSnapshotSelect(t *testing.T) {
 			arch := New(repo, fs.Track{FS: fs.Local{}}, Options{})
 			arch.Select = test.selFn
 
-			back := fs.TestChdir(t, tempdir)
+			back := restictest.Chdir(t, tempdir)
 			defer back()
 
 			targets := []string{"."}
@@ -1614,7 +1614,7 @@ func TestArchiverParent(t *testing.T) {
 
 			arch := New(repo, testFS, Options{})
 
-			back := fs.TestChdir(t, tempdir)
+			back := restictest.Chdir(t, tempdir)
 			defer back()
 
 			_, firstSnapshotID, err := arch.Snapshot(ctx, []string{"."}, SnapshotOptions{Time: time.Now()})
@@ -1774,7 +1774,7 @@ func TestArchiverErrorReporting(t *testing.T) {
 			tempdir, repo, cleanup := prepareTempdirRepoSrc(t, test.src)
 			defer cleanup()
 
-			back := fs.TestChdir(t, tempdir)
+			back := restictest.Chdir(t, tempdir)
 			defer back()
 
 			if test.prepare != nil {
@@ -1915,7 +1915,7 @@ func TestArchiverAbortEarlyOnError(t *testing.T) {
 			tempdir, repo, cleanup := prepareTempdirRepoSrc(t, test.src)
 			defer cleanup()
 
-			back := fs.TestChdir(t, tempdir)
+			back := restictest.Chdir(t, tempdir)
 			defer back()
 
 			testFS := &TrackFS{
@@ -2046,7 +2046,7 @@ func TestMetadataChanged(t *testing.T) {
 	tempdir, repo, cleanup := prepareTempdirRepoSrc(t, files)
 	defer cleanup()
 
-	back := fs.TestChdir(t, tempdir)
+	back := restictest.Chdir(t, tempdir)
 	defer back()
 
 	// get metadata
@@ -2121,7 +2121,7 @@ func TestRacyFileSwap(t *testing.T) {
 	tempdir, repo, cleanup := prepareTempdirRepoSrc(t, files)
 	defer cleanup()
 
-	back := fs.TestChdir(t, tempdir)
+	back := restictest.Chdir(t, tempdir)
 	defer back()
 
 	// get metadata of current folder

internal/archiver/scanner_test.go

@@ -88,7 +88,7 @@ func TestScanner(t *testing.T) {
 
 			TestCreateFiles(t, tempdir, test.src)
 
-			back := fs.TestChdir(t, tempdir)
+			back := restictest.Chdir(t, tempdir)
 			defer back()
 
 			cur, err := os.Getwd()
@@ -225,7 +225,7 @@ func TestScannerError(t *testing.T) {
 
 			TestCreateFiles(t, tempdir, test.src)
 
-			back := fs.TestChdir(t, tempdir)
+			back := restictest.Chdir(t, tempdir)
 			defer back()
 
 			cur, err := os.Getwd()
@@ -299,7 +299,7 @@ func TestScannerCancel(t *testing.T) {
 
 	TestCreateFiles(t, tempdir, src)
 
-	back := fs.TestChdir(t, tempdir)
+	back := restictest.Chdir(t, tempdir)
 	defer back()
 
 	cur, err := os.Getwd()

internal/archiver/testing_test.go

@@ -495,7 +495,7 @@ func TestTestEnsureSnapshot(t *testing.T) {
 
 			createFilesAt(t, targetDir, test.files)
 
-			back := fs.TestChdir(t, tempdir)
+			back := restictest.Chdir(t, tempdir)
 			defer back()
 
 			repo, cleanup := repository.TestRepository(t)

internal/archiver/tree_test.go

@@ -444,7 +444,7 @@ func TestTree(t *testing.T) {
 
 			TestCreateFiles(t, tempdir, test.src)
 
-			back := fs.TestChdir(t, tempdir)
+			back := restictest.Chdir(t, tempdir)
 			defer back()
 
 			tree, err := NewTree(fs.Local{}, test.targets)

internal/dump/tar_test.go

@@ -79,7 +79,7 @@ func TestWriteTar(t *testing.T) {
 
 			arch := archiver.New(repo, fs.Track{FS: fs.Local{}}, archiver.Options{})
 
-			back := fs.TestChdir(t, tmpdir)
+			back := rtest.Chdir(t, tmpdir)
 			defer back()
 
 			sn, _, err := arch.Snapshot(ctx, []string{"."}, archiver.SnapshotOptions{})

internal/fs/file.go

@@ -12,6 +12,14 @@ func Mkdir(name string, perm os.FileMode) error {
 	return os.Mkdir(fixpath(name), perm)
 }
 
+// MkdirAll creates a directory named path, along with any necessary parents,
+// and returns nil, or else returns an error. The permission bits perm are used
+// for all directories that MkdirAll creates. If path is already a directory,
+// MkdirAll does nothing and returns nil.
+func MkdirAll(path string, perm os.FileMode) error {
+	return os.MkdirAll(fixpath(path), perm)
+}
+
 // Readlink returns the destination of the named symbolic link.
 // If there is an error, it will be of type *PathError.
 func Readlink(name string) (string, error) {
@@ -32,14 +40,6 @@ func RemoveAll(path string) error {
 	return os.RemoveAll(fixpath(path))
 }
 
-// Rename renames (moves) oldpath to newpath.
-// If newpath already exists, Rename replaces it.
-// OS-specific restrictions may apply when oldpath and newpath are in different directories.
-// If there is an error, it will be of type *LinkError.
-func Rename(oldpath, newpath string) error {
-	return os.Rename(fixpath(oldpath), fixpath(newpath))
-}
-
 // Symlink creates newname as a symbolic link to oldname.
 // If there is an error, it will be of type *LinkError.
 func Symlink(oldname, newname string) error {

internal/fs/file_unix.go

@@ -14,14 +14,6 @@ func fixpath(name string) string {
 	return name
 }
 
-// MkdirAll creates a directory named path, along with any necessary parents,
-// and returns nil, or else returns an error. The permission bits perm are used
-// for all directories that MkdirAll creates. If path is already a directory,
-// MkdirAll does nothing and returns nil.
-func MkdirAll(path string, perm os.FileMode) error {
-	return os.MkdirAll(fixpath(path), perm)
-}
-
 // TempFile creates a temporary file which has already been deleted (on
 // supported platforms)
 func TempFile(dir, prefix string) (f *os.File, err error) {

internal/fs/file_windows.go

@@ -5,7 +5,6 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
-	"syscall"
 )
 
 // fixpath returns an absolute path on windows, so restic can open long file
@@ -31,62 +30,6 @@ func fixpath(name string) string {
 	return name
 }
 
-// MkdirAll creates a directory named path, along with any necessary parents,
-// and returns nil, or else returns an error. The permission bits perm are used
-// for all directories that MkdirAll creates. If path is already a directory,
-// MkdirAll does nothing and returns nil.
-//
-// Adapted from the stdlib MkdirAll, added test for volume name.
-func MkdirAll(path string, perm os.FileMode) error {
-	// Fast path: if we can tell whether path is a directory or file, stop with success or error.
-	dir, err := os.Stat(path)
-	if err == nil {
-		if dir.IsDir() {
-			return nil
-		}
-		return &os.PathError{
-			Op:   "mkdir",
-			Path: path,
-			Err:  syscall.ENOTDIR,
-		}
-	}
-
-	// Slow path: make sure parent exists and then call Mkdir for path.
-	i := len(path)
-	for i > 0 && os.IsPathSeparator(path[i-1]) { // Skip trailing path separator.
-		i--
-	}
-
-	j := i
-	for j > 0 && !os.IsPathSeparator(path[j-1]) { // Scan backward over element.
-		j--
-	}
-
-	if j > 1 {
-		// Create parent
-		parent := path[0 : j-1]
-		if parent != filepath.VolumeName(parent) {
-			err = MkdirAll(parent, perm)
-			if err != nil {
-				return err
-			}
-		}
-	}
-
-	// Parent now exists; invoke Mkdir and use its result.
-	err = os.Mkdir(path, perm)
-	if err != nil {
-		// Handle arguments like "foo/." by
-		// double-checking that directory doesn't exist.
-		dir, err1 := os.Lstat(path)
-		if err1 == nil && dir.IsDir() {
-			return nil
-		}
-		return err
-	}
-	return nil
-}
-
 // TempFile creates a temporary file.
 func TempFile(dir, prefix string) (f *os.File, err error) {
 	return ioutil.TempFile(dir, prefix)

internal/fs/fs_reader.go

@@ -1,7 +1,6 @@
 package fs
 
 import (
-	"fmt"
 	"io"
 	"os"
 	"path"
@@ -208,7 +207,6 @@ func (r *readerFile) Read(p []byte) (int, error) {
 
 	// return an error if we did not read any data
 	if err == io.EOF && !r.AllowEmptyFile && !r.bytesRead {
-		fmt.Printf("reader: %d bytes read, err %v, bytesRead %v, allowEmpty %v\n", n, err, r.bytesRead, r.AllowEmptyFile)
 		return n, &os.PathError{
 			Path: r.fakeFile.name,
 			Op:   "read",
@@ -252,10 +250,6 @@ func (f fakeFile) Seek(int64, int) (int64, error) {
 	return 0, os.ErrInvalid
 }
 
-func (f fakeFile) Write(p []byte) (int, error) {
-	return 0, os.ErrInvalid
-}
-
 func (f fakeFile) Read(p []byte) (int, error) {
 	return 0, os.ErrInvalid
 }
@@ -279,7 +273,7 @@ type fakeDir struct {
 }
 
 func (d fakeDir) Readdirnames(n int) ([]string, error) {
-	if n >= 0 {
+	if n > 0 {
 		return nil, errors.New("not implemented")
 	}
 	names := make([]string, 0, len(d.entries))
@@ -291,7 +285,7 @@ func (d fakeDir) Readdirnames(n int) ([]string, error) {
 }
 
 func (d fakeDir) Readdir(n int) ([]os.FileInfo, error) {
-	if n >= 0 {
+	if n > 0 {
 		return nil, errors.New("not implemented")
 	}
 	return d.entries, nil
@@ -303,7 +297,6 @@ type fakeFileInfo struct {
 	size    int64
 	mode    os.FileMode
 	modtime time.Time
-	sys     interface{}
 }
 
 func (fi fakeFileInfo) Name() string {
@@ -327,5 +320,5 @@ func (fi fakeFileInfo) IsDir() bool {
 }
 
 func (fi fakeFileInfo) Sys() interface{} {
-	return fi.sys
+	return nil
 }

internal/fs/helpers.go

@@ -1,10 +1,6 @@
 package fs
 
-import (
+import "os"
-	"io/ioutil"
-	"os"
-	"testing"
-)
 
 // IsRegularFile returns true if fi belongs to a normal file. If fi is nil,
 // false is returned.
@@ -13,49 +9,5 @@ func IsRegularFile(fi os.FileInfo) bool {
 		return false
 	}
 
-	return fi.Mode()&(os.ModeType|os.ModeCharDevice) == 0
+	return fi.Mode()&os.ModeType == 0
-}
-
-// TestChdir changes the current directory to dest, the function back returns to the previous directory.
-func TestChdir(t testing.TB, dest string) (back func()) {
-	t.Helper()
-
-	prev, err := os.Getwd()
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	t.Logf("chdir to %v", dest)
-	err = os.Chdir(dest)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	return func() {
-		t.Helper()
-		t.Logf("chdir back to %v", prev)
-		err = os.Chdir(prev)
-		if err != nil {
-			t.Fatal(err)
-		}
-	}
-}
-
-// TestTempFile returns a new temporary file, which is removed when cleanup()
-// is called.
-func TestTempFile(t testing.TB, prefix string) (File, func()) {
-	f, err := ioutil.TempFile("", prefix)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	cleanup := func() {
-		_ = f.Close()
-		err = Remove(f.Name())
-		if err != nil {
-			t.Fatal(err)
-		}
-	}
-
-	return f, cleanup
 }

internal/fs/interface.go

@@ -26,7 +26,6 @@ type FS interface {
 // File is an open file on a file system.
 type File interface {
 	io.Reader
-	io.Writer
 	io.Closer
 
 	Fd() uintptr

internal/restorer/restorer_test.go

@@ -442,7 +442,7 @@ func TestRestorerRelative(t *testing.T) {
 			tempdir, cleanup := rtest.TempDir(t)
 			defer cleanup()
 
-			cleanup = fs.TestChdir(t, tempdir)
+			cleanup = rtest.Chdir(t, tempdir)
 			defer cleanup()
 
 			errors := make(map[string]string)

internal/test/helpers.go

@@ -202,3 +202,29 @@ func TempDir(t testing.TB) (path string, cleanup func()) {
 		RemoveAll(t, tempdir)
 	}
 }
+
+// Chdir changes the current directory to dest.
+// The function back returns to the previous directory.
+func Chdir(t testing.TB, dest string) (back func()) {
+	t.Helper()
+
+	prev, err := os.Getwd()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	t.Logf("chdir to %v", dest)
+	err = os.Chdir(dest)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	return func() {
+		t.Helper()
+		t.Logf("chdir back to %v", prev)
+		err = os.Chdir(prev)
+		if err != nil {
+			t.Fatal(err)
+		}
+	}
+}

internal/textfile/read_test.go

@@ -3,25 +3,44 @@ package textfile
 import (
 	"bytes"
 	"encoding/hex"
+	"io/ioutil"
+	"os"
 	"testing"
-
-	"github.com/restic/restic/internal/fs"
 )
 
-func writeTempfile(t testing.TB, data []byte) (fs.File, func()) {
+// writeTempfile writes data to a new temporary file and returns its name
-	f, removeTempfile := fs.TestTempFile(t, "restic-test-textfile-read-")
+// and a callback that removes it.
+func writeTempfile(t testing.TB, data []byte) (string, func()) {
+	t.Helper()
 
-	_, err := f.Write(data)
+	f, err := ioutil.TempFile("", "restic-test-textfile-read-")
+	if err != nil {
+		t.Fatal(err)
+	}
+	name := f.Name()
+
+	defer func() {
+		closeErr := f.Close()
+		if err == nil && closeErr != nil {
+			err = closeErr
+		}
+		if err != nil {
+			os.Remove(name)
+			t.Fatal(err)
+		}
+	}()
+
+	_, err = f.Write(data)
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	err = f.Close()
+	return name, func() {
-	if err != nil {
+		err := os.Remove(name)
-		t.Fatal(err)
+		if err != nil {
+			t.Fatal(err)
+		}
 	}
-
-	return f, removeTempfile
 }
 
 func dec(s string) []byte {
@@ -60,10 +79,10 @@ func TestRead(t *testing.T) {
 				want = test.data
 			}
 
-			f, cleanup := writeTempfile(t, test.data)
+			tempname, cleanup := writeTempfile(t, test.data)
 			defer cleanup()
 
-			data, err := Read(f.Name())
+			data, err := Read(tempname)
 			if err != nil {
 				t.Fatal(err)
 			}