fs / archiver: convert to handle based interface

The actual implementation still relies on file paths, but with the
abstraction layer in place, an FS implementation can ensure atomic file
accesses in the future.

internal/fs/file.go

@@ -67,7 +67,7 @@ func ResetPermissions(path string) error {
 // Readdirnames returns a list of file in a directory. Flags are passed to fs.OpenFile.
 // O_RDONLY and O_DIRECTORY are implied.
 func Readdirnames(filesystem FS, dir string, flags int) ([]string, error) {
-	f, err := filesystem.OpenFile(dir, O_RDONLY|O_DIRECTORY|flags)
+	f, err := filesystem.OpenFile(dir, O_RDONLY|O_DIRECTORY|flags, false)
 	if err != nil {
 		return nil, fmt.Errorf("openfile for readdirnames failed: %w", err)
 	}

internal/fs/fs_local.go

@@ -20,18 +20,16 @@ func (fs Local) VolumeName(path string) string {
 	return filepath.VolumeName(path)
 }
 
-// OpenFile is the generalized open call; most users will use Open
-// or Create instead.  It opens the named file with specified flag
-// (O_RDONLY etc.) and perm, (0666 etc.) if applicable.  If successful,
-// methods on the returned File can be used for I/O.
-// If there is an error, it will be of type *PathError.
-func (fs Local) OpenFile(name string, flag int) (File, error) {
-	f, err := os.OpenFile(fixpath(name), flag, 0)
-	if err != nil {
-		return nil, err
-	}
-	_ = setFlags(f)
-	return f, nil
+// OpenFile opens a file or directory for reading.
+//
+// If metadataOnly is set, an implementation MUST return a File object for
+// arbitrary file types including symlinks. The implementation may internally use
+// the given file path or a file handle. In particular, an implementation may
+// delay actually accessing the underlying filesystem.
+//
+// Only the O_NOFOLLOW and O_DIRECTORY flags are supported.
+func (fs Local) OpenFile(name string, flag int, metadataOnly bool) (File, error) {
+	return newLocalFile(name, flag, metadataOnly)
 }
 
 // Lstat returns the FileInfo structure describing the named file.
@@ -53,10 +51,6 @@ func (fs Local) ExtendedStat(fi os.FileInfo) ExtendedFileInfo {
 	return ExtendedStat(fi)
 }
 
-func (fs Local) NodeFromFileInfo(path string, fi os.FileInfo, ignoreXattrListError bool) (*restic.Node, error) {
-	return nodeFromFileInfo(path, fi, ignoreXattrListError)
-}
-
 // Join joins any number of path elements into a single path, adding a
 // Separator if necessary. Join calls Clean on the result; in particular, all
 // empty strings are ignored. On Windows, the result is a UNC path if and only
@@ -97,3 +91,87 @@ func (fs Local) Base(path string) string {
 func (fs Local) Dir(path string) string {
 	return filepath.Dir(path)
 }
+
+type localFile struct {
+	name string
+	flag int
+	f    *os.File
+	fi   os.FileInfo
+}
+
+// See the File interface for a description of each method
+var _ File = &localFile{}
+
+func newLocalFile(name string, flag int, metadataOnly bool) (*localFile, error) {
+	var f *os.File
+	if !metadataOnly {
+		var err error
+		f, err = os.OpenFile(fixpath(name), flag, 0)
+		if err != nil {
+			return nil, err
+		}
+		_ = setFlags(f)
+	}
+	return &localFile{
+		name: name,
+		flag: flag,
+		f:    f,
+	}, nil
+}
+
+func (f *localFile) MakeReadable() error {
+	if f.f != nil {
+		panic("file is already readable")
+	}
+
+	newF, err := newLocalFile(f.name, f.flag, false)
+	if err != nil {
+		return err
+	}
+	// replace state and also reset cached FileInfo
+	*f = *newF
+	return nil
+}
+
+func (f *localFile) cacheFI() error {
+	if f.fi != nil {
+		return nil
+	}
+	var err error
+	if f.f != nil {
+		f.fi, err = f.f.Stat()
+	} else if f.flag&O_NOFOLLOW != 0 {
+		f.fi, err = os.Lstat(f.name)
+	} else {
+		f.fi, err = os.Stat(f.name)
+	}
+	return err
+}
+
+func (f *localFile) Stat() (os.FileInfo, error) {
+	err := f.cacheFI()
+	// the call to cacheFI MUST happen before reading from f.fi
+	return f.fi, err
+}
+
+func (f *localFile) ToNode(ignoreXattrListError bool) (*restic.Node, error) {
+	if err := f.cacheFI(); err != nil {
+		return nil, err
+	}
+	return nodeFromFileInfo(f.name, f.fi, ignoreXattrListError)
+}
+
+func (f *localFile) Read(p []byte) (n int, err error) {
+	return f.f.Read(p)
+}
+
+func (f *localFile) Readdirnames(n int) ([]string, error) {
+	return f.f.Readdirnames(n)
+}
+
+func (f *localFile) Close() error {
+	if f.f != nil {
+		return f.f.Close()
+	}
+	return nil
+}

internal/fs/fs_local_vss.go

@@ -10,7 +10,6 @@ import (
 
 	"github.com/restic/restic/internal/errors"
 	"github.com/restic/restic/internal/options"
-	"github.com/restic/restic/internal/restic"
 )
 
 // VSSConfig holds extended options of windows volume shadow copy service.
@@ -126,9 +125,9 @@ func (fs *LocalVss) DeleteSnapshots() {
 	fs.snapshots = activeSnapshots
 }
 
-// OpenFile wraps the Open method of the underlying file system.
-func (fs *LocalVss) OpenFile(name string, flag int) (File, error) {
-	return fs.FS.OpenFile(fs.snapshotPath(name), flag)
+// OpenFile wraps the OpenFile method of the underlying file system.
+func (fs *LocalVss) OpenFile(name string, flag int, metadataOnly bool) (File, error) {
+	return fs.FS.OpenFile(fs.snapshotPath(name), flag, metadataOnly)
 }
 
 // Lstat wraps the Lstat method of the underlying file system.
@@ -136,10 +135,6 @@ func (fs *LocalVss) Lstat(name string) (os.FileInfo, error) {
 	return fs.FS.Lstat(fs.snapshotPath(name))
 }
 
-func (fs *LocalVss) NodeFromFileInfo(path string, fi os.FileInfo, ignoreXattrListError bool) (*restic.Node, error) {
-	return fs.FS.NodeFromFileInfo(fs.snapshotPath(path), fi, ignoreXattrListError)
-}
-
 // isMountPointIncluded  is true if given mountpoint included by user.
 func (fs *LocalVss) isMountPointIncluded(mountPoint string) bool {
 	if fs.excludeVolumes == nil {

internal/fs/fs_local_vss_test.go

@@ -327,7 +327,7 @@ func TestVSSFS(t *testing.T) {
 	rtest.OK(t, err)
 	rtest.Equals(t, origFi.Mode(), lstatFi.Mode())
 
-	f, err := localVss.OpenFile(tempfile, os.O_RDONLY)
+	f, err := localVss.OpenFile(tempfile, os.O_RDONLY, false)
 	rtest.OK(t, err)
 	data, err := io.ReadAll(f)
 	rtest.OK(t, err)

internal/fs/fs_reader.go

@@ -49,12 +49,7 @@ func (fs *Reader) fi() os.FileInfo {
 	}
 }
 
-// OpenFile is the generalized open call; most users will use Open
-// or Create instead.  It opens the named file with specified flag
-// (O_RDONLY etc.) and perm, (0666 etc.) if applicable.  If successful,
-// methods on the returned File can be used for I/O.
-// If there is an error, it will be of type *os.PathError.
-func (fs *Reader) OpenFile(name string, flag int) (f File, err error) {
+func (fs *Reader) OpenFile(name string, flag int, _ bool) (f File, err error) {
 	if flag & ^(O_RDONLY|O_NOFOLLOW) != 0 {
 		return nil, pathError("open", name,
 			fmt.Errorf("invalid combination of flags 0x%x", flag))
@@ -127,17 +122,6 @@ func (fs *Reader) ExtendedStat(fi os.FileInfo) ExtendedFileInfo {
 	}
 }
 
-func (fs *Reader) NodeFromFileInfo(path string, fi os.FileInfo, _ bool) (*restic.Node, error) {
-	node := buildBasicNode(path, fi)
-
-	// fill minimal info with current values for uid, gid
-	node.UID = uint32(os.Getuid())
-	node.GID = uint32(os.Getgid())
-	node.ChangeTime = node.ModTime
-
-	return node, nil
-}
-
 // Join joins any number of path elements into a single path, adding a
 // Separator if necessary. Join calls Clean on the result; in particular, all
 // empty strings are ignored. On Windows, the result is a UNC path if and only
@@ -235,6 +219,10 @@ type fakeFile struct {
 // ensure that fakeFile implements File
 var _ File = fakeFile{}
 
+func (f fakeFile) MakeReadable() error {
+	return nil
+}
+
 func (f fakeFile) Readdirnames(_ int) ([]string, error) {
 	return nil, pathError("readdirnames", f.name, os.ErrInvalid)
 }
@@ -251,6 +239,17 @@ func (f fakeFile) Stat() (os.FileInfo, error) {
 	return f.FileInfo, nil
 }
 
+func (f fakeFile) ToNode(_ bool) (*restic.Node, error) {
+	node := buildBasicNode(f.name, f.FileInfo)
+
+	// fill minimal info with current values for uid, gid
+	node.UID = uint32(os.Getuid())
+	node.GID = uint32(os.Getgid())
+	node.ChangeTime = node.ModTime
+
+	return node, nil
+}
+
 // fakeDir implements Readdirnames and Readdir, everything else is delegated to fakeFile.
 type fakeDir struct {
 	entries []os.FileInfo

internal/fs/fs_reader_test.go

@@ -16,7 +16,7 @@ import (
 )
 
 func verifyFileContentOpenFile(t testing.TB, fs FS, filename string, want []byte) {
-	f, err := fs.OpenFile(filename, O_RDONLY)
+	f, err := fs.OpenFile(filename, O_RDONLY, false)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -37,7 +37,7 @@ func verifyFileContentOpenFile(t testing.TB, fs FS, filename string, want []byte
 }
 
 func verifyDirectoryContents(t testing.TB, fs FS, dir string, want []string) {
-	f, err := fs.OpenFile(dir, os.O_RDONLY)
+	f, err := fs.OpenFile(dir, O_RDONLY, false)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -123,7 +123,7 @@ func TestFSReader(t *testing.T) {
 		{
 			name: "file/Stat",
 			f: func(t *testing.T, fs FS) {
-				f, err := fs.OpenFile(filename, os.O_RDONLY)
+				f, err := fs.OpenFile(filename, O_RDONLY, true)
 				if err != nil {
 					t.Fatal(err)
 				}
@@ -295,7 +295,7 @@ func TestFSReaderMinFileSize(t *testing.T) {
 				AllowEmptyFile: test.allowEmpty,
 			}
 
-			f, err := fs.OpenFile("testfile", os.O_RDONLY)
+			f, err := fs.OpenFile("testfile", O_RDONLY, false)
 			if err != nil {
 				t.Fatal(err)
 			}

internal/fs/fs_track.go

@@ -16,8 +16,8 @@ type Track struct {
 }
 
 // OpenFile wraps the OpenFile method of the underlying file system.
-func (fs Track) OpenFile(name string, flag int) (File, error) {
-	f, err := fs.FS.OpenFile(fixpath(name), flag)
+func (fs Track) OpenFile(name string, flag int, metadataOnly bool) (File, error) {
+	f, err := fs.FS.OpenFile(name, flag, metadataOnly)
 	if err != nil {
 		return nil, err
 	}
@@ -31,7 +31,7 @@ type trackFile struct {
 
 func newTrackFile(stack []byte, filename string, file File) *trackFile {
 	f := &trackFile{file}
-	runtime.SetFinalizer(f, func(_ *trackFile) {
+	runtime.SetFinalizer(f, func(_ any) {
 		fmt.Fprintf(os.Stderr, "file %s not closed\n\nStacktrack:\n%s\n", filename, stack)
 		panic("file " + filename + " not closed")
 	})

internal/fs/interface.go

@@ -9,11 +9,18 @@ import (
 
 // FS bundles all methods needed for a file system.
 type FS interface {
-	OpenFile(name string, flag int) (File, error)
+	// OpenFile opens a file or directory for reading.
+	//
+	// If metadataOnly is set, an implementation MUST return a File object for
+	// arbitrary file types including symlinks. The implementation may internally use
+	// the given file path or a file handle. In particular, an implementation may
+	// delay actually accessing the underlying filesystem.
+	//
+	// Only the O_NOFOLLOW and O_DIRECTORY flags are supported.
+	OpenFile(name string, flag int, metadataOnly bool) (File, error)
 	Lstat(name string) (os.FileInfo, error)
 	DeviceID(fi os.FileInfo) (deviceID uint64, err error)
 	ExtendedStat(fi os.FileInfo) ExtendedFileInfo
-	NodeFromFileInfo(path string, fi os.FileInfo, ignoreXattrListError bool) (*restic.Node, error)
 
 	Join(elem ...string) string
 	Separator() string
@@ -26,11 +33,23 @@ type FS interface {
 	Base(path string) string
 }
 
-// File is an open file on a file system.
+// File is an open file on a file system. When opened as metadataOnly, an
+// implementation may opt to perform filesystem operations using the filepath
+// instead of actually opening the file.
 type File interface {
+	// MakeReadable reopens a File that was opened metadataOnly for reading.
+	// The method must not be called for files that are opened for reading.
+	// If possible, the underlying file should be reopened atomically.
+	// MakeReadable must work for files and directories.
+	MakeReadable() error
+
 	io.Reader
 	io.Closer
 
 	Readdirnames(n int) ([]string, error)
 	Stat() (os.FileInfo, error)
+	// ToNode returns a restic.Node for the File. The internally used os.FileInfo
+	// must be consistent with that returned by Stat(). In particular, the metadata
+	// returned by consecutive calls to Stat() and ToNode() must match.
+	ToNode(ignoreXattrListError bool) (*restic.Node, error)
 }

internal/fs/node_test.go

@@ -17,56 +17,26 @@ import (
 	rtest "github.com/restic/restic/internal/test"
 )
 
-func BenchmarkNodeFillUser(t *testing.B) {
-	tempfile, err := os.CreateTemp("", "restic-test-temp-")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	fi, err := tempfile.Stat()
-	if err != nil {
-		t.Fatal(err)
-	}
-
+func BenchmarkNodeFromFileInfo(t *testing.B) {
+	tempfile, err := os.CreateTemp(t.TempDir(), "restic-test-temp-")
+	rtest.OK(t, err)
 	path := tempfile.Name()
+	rtest.OK(t, tempfile.Close())
+
 	fs := Local{}
+	f, err := fs.OpenFile(path, O_NOFOLLOW, true)
+	rtest.OK(t, err)
+	_, err = f.Stat()
+	rtest.OK(t, err)
 
 	t.ResetTimer()
 
 	for i := 0; i < t.N; i++ {
-		_, err := fs.NodeFromFileInfo(path, fi, false)
+		_, err := f.ToNode(false)
 		rtest.OK(t, err)
 	}
 
-	rtest.OK(t, tempfile.Close())
-	rtest.RemoveAll(t, tempfile.Name())
-}
-
-func BenchmarkNodeFromFileInfo(t *testing.B) {
-	tempfile, err := os.CreateTemp("", "restic-test-temp-")
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	fi, err := tempfile.Stat()
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	path := tempfile.Name()
-	fs := Local{}
-
-	t.ResetTimer()
-
-	for i := 0; i < t.N; i++ {
-		_, err := fs.NodeFromFileInfo(path, fi, false)
-		if err != nil {
-			t.Fatal(err)
-		}
-	}
-
-	rtest.OK(t, tempfile.Close())
-	rtest.RemoveAll(t, tempfile.Name())
+	rtest.OK(t, f.Close())
 }
 
 func parseTime(s string) time.Time {
@@ -249,14 +219,14 @@ func TestNodeRestoreAt(t *testing.T) {
 			rtest.OK(t, NodeCreateAt(&test, nodePath))
 			rtest.OK(t, NodeRestoreMetadata(&test, nodePath, func(msg string) { rtest.OK(t, fmt.Errorf("Warning triggered for path: %s: %s", nodePath, msg)) }))
 
-			fi, err := os.Lstat(nodePath)
-			rtest.OK(t, err)
-
 			fs := &Local{}
-			n2, err := fs.NodeFromFileInfo(nodePath, fi, false)
+			meta, err := fs.OpenFile(nodePath, O_NOFOLLOW, true)
 			rtest.OK(t, err)
-			n3, err := fs.NodeFromFileInfo(nodePath, fi, true)
+			n2, err := meta.ToNode(false)
 			rtest.OK(t, err)
+			n3, err := meta.ToNode(true)
+			rtest.OK(t, err)
+			rtest.OK(t, meta.Close())
 			rtest.Assert(t, n2.Equals(*n3), "unexpected node info mismatch %v", cmp.Diff(n2, n3))
 
 			rtest.Assert(t, test.Name == n2.Name,

internal/fs/node_unix_test.go

@@ -114,16 +114,14 @@ func TestNodeFromFileInfo(t *testing.T) {
 				return
 			}
 
-			if fi.Sys() == nil {
-				t.Skip("fi.Sys() is nil")
-				return
-			}
-
 			fs := &Local{}
-			node, err := fs.NodeFromFileInfo(test.filename, fi, false)
-			if err != nil {
-				t.Fatal(err)
-			}
+			meta, err := fs.OpenFile(test.filename, O_NOFOLLOW, true)
+			rtest.OK(t, err)
+			node, err := meta.ToNode(false)
+			rtest.OK(t, err)
+			rtest.OK(t, meta.Close())
+
+			rtest.OK(t, err)
 
 			switch node.Type {
 			case restic.NodeTypeFile, restic.NodeTypeSymlink:

internal/fs/node_windows_test.go

@@ -222,11 +222,11 @@ func restoreAndGetNode(t *testing.T, tempDir string, testNode *restic.Node, warn
 	test.OK(t, errors.Wrapf(err, "Failed to restore metadata for: %s", testPath))
 
 	fs := &Local{}
-	fi, err := fs.Lstat(testPath)
-	test.OK(t, errors.Wrapf(err, "Could not Lstat for path: %s", testPath))
-
-	nodeFromFileInfo, err := fs.NodeFromFileInfo(testPath, fi, false)
+	meta, err := fs.OpenFile(testPath, O_NOFOLLOW, true)
+	test.OK(t, err)
+	nodeFromFileInfo, err := meta.ToNode(false)
 	test.OK(t, errors.Wrapf(err, "Could not get NodeFromFileInfo for path: %s", testPath))
+	test.OK(t, meta.Close())
 
 	return testPath, nodeFromFileInfo
 }