diff --git a/cmd/restic/integration_test.go b/cmd/restic/integration_test.go index 3958d6481..3ba196f5b 100644 --- a/cmd/restic/integration_test.go +++ b/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() diff --git a/internal/archiver/archiver_test.go b/internal/archiver/archiver_test.go index 6fc5ab648..643da5d24 100644 --- a/internal/archiver/archiver_test.go +++ b/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 diff --git a/internal/archiver/scanner_test.go b/internal/archiver/scanner_test.go index a171df5f6..3d4cfacdd 100644 --- a/internal/archiver/scanner_test.go +++ b/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() diff --git a/internal/archiver/testing_test.go b/internal/archiver/testing_test.go index 5d9c43363..e11b86250 100644 --- a/internal/archiver/testing_test.go +++ b/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) diff --git a/internal/archiver/tree_test.go b/internal/archiver/tree_test.go index 00d0b6b82..73ace7bfc 100644 --- a/internal/archiver/tree_test.go +++ b/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) diff --git a/internal/dump/tar_test.go b/internal/dump/tar_test.go index fddbef7f0..a8ce336fd 100644 --- a/internal/dump/tar_test.go +++ b/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{}) diff --git a/internal/fs/file.go b/internal/fs/file.go index 86c519aff..e438857b2 100644 --- a/internal/fs/file.go +++ b/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 { diff --git a/internal/fs/file_unix.go b/internal/fs/file_unix.go index 612465670..f5ea36696 100644 --- a/internal/fs/file_unix.go +++ b/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) { diff --git a/internal/fs/file_windows.go b/internal/fs/file_windows.go index dd53cde5e..8a4d01fb0 100644 --- a/internal/fs/file_windows.go +++ b/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) diff --git a/internal/fs/fs_reader.go b/internal/fs/fs_reader.go index 91fedb263..dac4aac9e 100644 --- a/internal/fs/fs_reader.go +++ b/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 } diff --git a/internal/fs/helpers.go b/internal/fs/helpers.go index 787fe485a..4dd1e0e73 100644 --- a/internal/fs/helpers.go +++ b/internal/fs/helpers.go @@ -1,10 +1,6 @@ package fs -import ( - "io/ioutil" - "os" - "testing" -) +import "os" // 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 -} - -// 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 + return fi.Mode()&os.ModeType == 0 } diff --git a/internal/fs/interface.go b/internal/fs/interface.go index 1c2260215..b26c56944 100644 --- a/internal/fs/interface.go +++ b/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 diff --git a/internal/restorer/restorer_test.go b/internal/restorer/restorer_test.go index 333d4598b..00c56bccd 100644 --- a/internal/restorer/restorer_test.go +++ b/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) diff --git a/internal/test/helpers.go b/internal/test/helpers.go index 785189598..cd2d215cf 100644 --- a/internal/test/helpers.go +++ b/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) + } + } +} diff --git a/internal/textfile/read_test.go b/internal/textfile/read_test.go index 572a33ebe..8e8e659dc 100644 --- a/internal/textfile/read_test.go +++ b/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()) { - f, removeTempfile := fs.TestTempFile(t, "restic-test-textfile-read-") +// writeTempfile writes data to a new temporary file and returns its name +// 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() - if err != nil { - t.Fatal(err) + return name, func() { + err := os.Remove(name) + 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) }