From 24f4e780f1019b49fd9f0c6644d3925a1e754056 Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 21 Jul 2024 15:22:21 +0200 Subject: [PATCH] backend: consistently use os package for filesystem access The go std library should be good enough to manage the files in the backend and cache folders. --- cmd/restic/cmd_cache.go | 3 +-- cmd/restic/cmd_check.go | 3 +-- cmd/restic/global.go | 3 +-- internal/backend/cache/cache.go | 13 ++++++------- internal/backend/cache/file.go | 21 ++++++++++----------- internal/backend/cache/file_test.go | 3 +-- internal/backend/local/local.go | 24 ++++++++++++------------ internal/backend/local/local_unix.go | 4 +--- internal/fs/file.go | 9 --------- 9 files changed, 33 insertions(+), 50 deletions(-) diff --git a/cmd/restic/cmd_cache.go b/cmd/restic/cmd_cache.go index e54c73451..cd970b699 100644 --- a/cmd/restic/cmd_cache.go +++ b/cmd/restic/cmd_cache.go @@ -10,7 +10,6 @@ import ( "github.com/restic/restic/internal/backend/cache" "github.com/restic/restic/internal/errors" - "github.com/restic/restic/internal/fs" "github.com/restic/restic/internal/ui" "github.com/restic/restic/internal/ui/table" "github.com/spf13/cobra" @@ -89,7 +88,7 @@ func runCache(opts CacheOptions, gopts GlobalOptions, args []string) error { for _, item := range oldDirs { dir := filepath.Join(cachedir, item.Name()) - err = fs.RemoveAll(dir) + err = os.RemoveAll(dir) if err != nil { Warnf("unable to remove %v: %v\n", dir, err) } diff --git a/cmd/restic/cmd_check.go b/cmd/restic/cmd_check.go index fc460e39e..8788b0caf 100644 --- a/cmd/restic/cmd_check.go +++ b/cmd/restic/cmd_check.go @@ -14,7 +14,6 @@ import ( "github.com/restic/restic/internal/backend/cache" "github.com/restic/restic/internal/checker" "github.com/restic/restic/internal/errors" - "github.com/restic/restic/internal/fs" "github.com/restic/restic/internal/repository" "github.com/restic/restic/internal/restic" "github.com/restic/restic/internal/ui" @@ -202,7 +201,7 @@ func prepareCheckCache(opts CheckOptions, gopts *GlobalOptions, printer progress printer.P("using temporary cache in %v\n", tempdir) cleanup = func() { - err := fs.RemoveAll(tempdir) + err := os.RemoveAll(tempdir) if err != nil { printer.E("error removing temporary cache directory: %v\n", err) } diff --git a/cmd/restic/global.go b/cmd/restic/global.go index 375b57f98..5fb37f9f2 100644 --- a/cmd/restic/global.go +++ b/cmd/restic/global.go @@ -29,7 +29,6 @@ import ( "github.com/restic/restic/internal/backend/sftp" "github.com/restic/restic/internal/backend/swift" "github.com/restic/restic/internal/debug" - "github.com/restic/restic/internal/fs" "github.com/restic/restic/internal/options" "github.com/restic/restic/internal/repository" "github.com/restic/restic/internal/restic" @@ -548,7 +547,7 @@ func OpenRepository(ctx context.Context, opts GlobalOptions) (*repository.Reposi } for _, item := range oldCacheDirs { dir := filepath.Join(c.Base, item.Name()) - err = fs.RemoveAll(dir) + err = os.RemoveAll(dir) if err != nil { Warnf("unable to remove %v: %v\n", dir, err) } diff --git a/internal/backend/cache/cache.go b/internal/backend/cache/cache.go index a55b51c70..2893df501 100644 --- a/internal/backend/cache/cache.go +++ b/internal/backend/cache/cache.go @@ -12,7 +12,6 @@ import ( "github.com/pkg/errors" "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/debug" - "github.com/restic/restic/internal/fs" "github.com/restic/restic/internal/restic" ) @@ -54,7 +53,7 @@ const cachedirTagSignature = "Signature: 8a477f597d28d172789f06886806bc55\n" func writeCachedirTag(dir string) error { tagfile := filepath.Join(dir, "CACHEDIR.TAG") - f, err := fs.OpenFile(tagfile, os.O_CREATE|os.O_EXCL|os.O_WRONLY, fileMode) + f, err := os.OpenFile(tagfile, os.O_CREATE|os.O_EXCL|os.O_WRONLY, fileMode) if err != nil { if errors.Is(err, os.ErrExist) { return nil @@ -85,7 +84,7 @@ func New(id string, basedir string) (c *Cache, err error) { } } - err = fs.MkdirAll(basedir, dirMode) + err = os.MkdirAll(basedir, dirMode) if err != nil { return nil, errors.WithStack(err) } @@ -113,7 +112,7 @@ func New(id string, basedir string) (c *Cache, err error) { case errors.Is(err, os.ErrNotExist): // Create the repo cache dir. The parent exists, so Mkdir suffices. - err := fs.Mkdir(cachedir, dirMode) + err := os.Mkdir(cachedir, dirMode) switch { case err == nil: created = true @@ -134,7 +133,7 @@ func New(id string, basedir string) (c *Cache, err error) { } for _, p := range cacheLayoutPaths { - if err = fs.MkdirAll(filepath.Join(cachedir, p), dirMode); err != nil { + if err = os.MkdirAll(filepath.Join(cachedir, p), dirMode); err != nil { return nil, errors.WithStack(err) } } @@ -152,7 +151,7 @@ func New(id string, basedir string) (c *Cache, err error) { // directory d to the current time. func updateTimestamp(d string) error { t := time.Now() - return fs.Chtimes(d, t, t) + return os.Chtimes(d, t, t) } // MaxCacheAge is the default age (30 days) after which cache directories are considered old. @@ -165,7 +164,7 @@ func validCacheDirName(s string) bool { // listCacheDirs returns the list of cache directories. func listCacheDirs(basedir string) ([]os.FileInfo, error) { - f, err := fs.Open(basedir) + f, err := os.Open(basedir) if err != nil { if errors.Is(err, os.ErrNotExist) { err = nil diff --git a/internal/backend/cache/file.go b/internal/backend/cache/file.go index 12f5f23c5..41fd0b49b 100644 --- a/internal/backend/cache/file.go +++ b/internal/backend/cache/file.go @@ -12,7 +12,6 @@ import ( "github.com/restic/restic/internal/backend/util" "github.com/restic/restic/internal/crypto" "github.com/restic/restic/internal/debug" - "github.com/restic/restic/internal/fs" "github.com/restic/restic/internal/restic" ) @@ -44,7 +43,7 @@ func (c *Cache) load(h backend.Handle, length int, offset int64) (io.ReadCloser, return nil, false, errors.New("cannot be cached") } - f, err := fs.Open(c.filename(h)) + f, err := os.Open(c.filename(h)) if err != nil { return nil, false, errors.WithStack(err) } @@ -91,7 +90,7 @@ func (c *Cache) save(h backend.Handle, rd io.Reader) error { finalname := c.filename(h) dir := filepath.Dir(finalname) - err := fs.Mkdir(dir, 0700) + err := os.Mkdir(dir, 0700) if err != nil && !errors.Is(err, os.ErrExist) { return err } @@ -106,26 +105,26 @@ func (c *Cache) save(h backend.Handle, rd io.Reader) error { n, err := io.Copy(f, rd) if err != nil { _ = f.Close() - _ = fs.Remove(f.Name()) + _ = os.Remove(f.Name()) return errors.Wrap(err, "Copy") } if n <= int64(crypto.CiphertextLength(0)) { _ = f.Close() - _ = fs.Remove(f.Name()) + _ = os.Remove(f.Name()) debug.Log("trying to cache truncated file %v, removing", h) return nil } // Close, then rename. Windows doesn't like the reverse order. if err = f.Close(); err != nil { - _ = fs.Remove(f.Name()) + _ = os.Remove(f.Name()) return errors.WithStack(err) } - err = fs.Rename(f.Name(), finalname) + err = os.Rename(f.Name(), finalname) if err != nil { - _ = fs.Remove(f.Name()) + _ = os.Remove(f.Name()) } if runtime.GOOS == "windows" && errors.Is(err, os.ErrPermission) { // On Windows, renaming over an existing file is ok @@ -162,7 +161,7 @@ func (c *Cache) remove(h backend.Handle) (bool, error) { return false, nil } - err := fs.Remove(c.filename(h)) + err := os.Remove(c.filename(h)) removed := err == nil if errors.Is(err, os.ErrNotExist) { err = nil @@ -189,7 +188,7 @@ func (c *Cache) Clear(t restic.FileType, valid restic.IDSet) error { } // ignore ErrNotExist to gracefully handle multiple processes running Clear() concurrently - if err = fs.Remove(c.filename(backend.Handle{Type: t, Name: id.String()})); err != nil && !errors.Is(err, os.ErrNotExist) { + if err = os.Remove(c.filename(backend.Handle{Type: t, Name: id.String()})); err != nil && !errors.Is(err, os.ErrNotExist) { return err } } @@ -236,6 +235,6 @@ func (c *Cache) Has(h backend.Handle) bool { return false } - _, err := fs.Stat(c.filename(h)) + _, err := os.Stat(c.filename(h)) return err == nil } diff --git a/internal/backend/cache/file_test.go b/internal/backend/cache/file_test.go index ed2cd295a..942f71f91 100644 --- a/internal/backend/cache/file_test.go +++ b/internal/backend/cache/file_test.go @@ -12,7 +12,6 @@ import ( "github.com/restic/restic/internal/backend" "github.com/restic/restic/internal/errors" - "github.com/restic/restic/internal/fs" "github.com/restic/restic/internal/restic" rtest "github.com/restic/restic/internal/test" @@ -278,7 +277,7 @@ func TestFileSaveConcurrent(t *testing.T) { func TestFileSaveAfterDamage(t *testing.T) { c := TestNewCache(t) - rtest.OK(t, fs.RemoveAll(c.path)) + rtest.OK(t, os.RemoveAll(c.path)) // save a few bytes of data in the cache data := rtest.Random(123456789, 42) diff --git a/internal/backend/local/local.go b/internal/backend/local/local.go index 8985ef4c4..ee87ae5d6 100644 --- a/internal/backend/local/local.go +++ b/internal/backend/local/local.go @@ -40,7 +40,7 @@ func NewFactory() location.Factory { func open(cfg Config) (*Local, error) { l := layout.NewDefaultLayout(cfg.Path, filepath.Join) - fi, err := fs.Stat(l.Filename(backend.Handle{Type: backend.ConfigFile})) + fi, err := os.Stat(l.Filename(backend.Handle{Type: backend.ConfigFile})) m := util.DeriveModesFromFileInfo(fi, err) debug.Log("using (%03O file, %03O dir) permissions", m.File, m.Dir) @@ -68,14 +68,14 @@ func Create(_ context.Context, cfg Config) (*Local, error) { } // test if config file already exists - _, err = fs.Lstat(be.Filename(backend.Handle{Type: backend.ConfigFile})) + _, err = os.Lstat(be.Filename(backend.Handle{Type: backend.ConfigFile})) if err == nil { return nil, errors.New("config file already exists") } // create paths for data and refs for _, d := range be.Paths() { - err := fs.MkdirAll(d, be.Modes.Dir) + err := os.MkdirAll(d, be.Modes.Dir) if err != nil { return nil, errors.WithStack(err) } @@ -127,7 +127,7 @@ func (b *Local) Save(_ context.Context, h backend.Handle, rd backend.RewindReade debug.Log("error %v: creating dir", err) // error is caused by a missing directory, try to create it - mkdirErr := fs.MkdirAll(dir, b.Modes.Dir) + mkdirErr := os.MkdirAll(dir, b.Modes.Dir) if mkdirErr != nil { debug.Log("error creating dir %v: %v", dir, mkdirErr) } else { @@ -147,7 +147,7 @@ func (b *Local) Save(_ context.Context, h backend.Handle, rd backend.RewindReade // temporary's name and no other goroutine will get the same data to // Save, so the temporary name should never be reused by another // goroutine. - _ = fs.Remove(f.Name()) + _ = os.Remove(f.Name()) } }(f) @@ -211,7 +211,7 @@ func (b *Local) Load(ctx context.Context, h backend.Handle, length int, offset i } func (b *Local) openReader(_ context.Context, h backend.Handle, length int, offset int64) (io.ReadCloser, error) { - f, err := fs.Open(b.Filename(h)) + f, err := os.Open(b.Filename(h)) if err != nil { return nil, err } @@ -245,7 +245,7 @@ func (b *Local) openReader(_ context.Context, h backend.Handle, length int, offs // Stat returns information about a blob. func (b *Local) Stat(_ context.Context, h backend.Handle) (backend.FileInfo, error) { - fi, err := fs.Stat(b.Filename(h)) + fi, err := os.Stat(b.Filename(h)) if err != nil { return backend.FileInfo{}, errors.WithStack(err) } @@ -258,12 +258,12 @@ func (b *Local) Remove(_ context.Context, h backend.Handle) error { fn := b.Filename(h) // reset read-only flag - err := fs.Chmod(fn, 0666) + err := os.Chmod(fn, 0666) if err != nil && !os.IsPermission(err) { return errors.WithStack(err) } - return fs.Remove(fn) + return os.Remove(fn) } // List runs fn for each file in the backend which has the type t. When an @@ -289,7 +289,7 @@ func (b *Local) List(ctx context.Context, t backend.FileType, fn func(backend.Fi // Also, visitDirs assumes it sees a directory full of directories, while // visitFiles wants a directory full or regular files. func visitDirs(ctx context.Context, dir string, fn func(backend.FileInfo) error) error { - d, err := fs.Open(dir) + d, err := os.Open(dir) if err != nil { return err } @@ -316,7 +316,7 @@ func visitDirs(ctx context.Context, dir string, fn func(backend.FileInfo) error) } func visitFiles(ctx context.Context, dir string, fn func(backend.FileInfo) error, ignoreNotADirectory bool) error { - d, err := fs.Open(dir) + d, err := os.Open(dir) if err != nil { return err } @@ -362,7 +362,7 @@ func visitFiles(ctx context.Context, dir string, fn func(backend.FileInfo) error // Delete removes the repository and all files. func (b *Local) Delete(_ context.Context) error { - return fs.RemoveAll(b.Path) + return os.RemoveAll(b.Path) } // Close closes all open files. diff --git a/internal/backend/local/local_unix.go b/internal/backend/local/local_unix.go index e3256ed7a..e52587456 100644 --- a/internal/backend/local/local_unix.go +++ b/internal/backend/local/local_unix.go @@ -8,8 +8,6 @@ import ( "os" "runtime" "syscall" - - "github.com/restic/restic/internal/fs" ) // fsyncDir flushes changes to the directory dir. @@ -45,5 +43,5 @@ func isMacENOTTY(err error) bool { // set file to readonly func setFileReadonly(f string, mode os.FileMode) error { - return fs.Chmod(f, mode&^0222) + return os.Chmod(f, mode&^0222) } diff --git a/internal/fs/file.go b/internal/fs/file.go index 356b466c3..b727df79c 100644 --- a/internal/fs/file.go +++ b/internal/fs/file.go @@ -88,15 +88,6 @@ func OpenFile(name string, flag int, perm os.FileMode) (*os.File, error) { return os.OpenFile(fixpath(name), flag, perm) } -// Chtimes changes the access and modification times of the named file, -// similar to the Unix utime() or utimes() functions. -// -// The underlying filesystem may truncate or round the values to a less -// precise time unit. If there is an error, it will be of type *PathError. -func Chtimes(name string, atime time.Time, mtime time.Time) error { - return os.Chtimes(fixpath(name), atime, mtime) -} - // IsAccessDenied checks if the error is due to permission error. func IsAccessDenied(err error) bool { return os.IsPermission(err)