Переглянути джерело

chore(fs): put the caseFS as the outermost layer again (#9716)

Reasoning in comments. The main motivation is to avoid all the case
checks when walking the filesystem.

"again" as we already tried once, but it caused a major issue ragarding
mtimefs layer. The root of this problem has been fixed in the meantime
in ac8b3342a
Simon Frei 1 рік тому
батько
коміт
cbe1220680
2 змінених файлів з 59 додано та 34 видалено
  1. 24 9
      lib/fs/filesystem.go
  2. 35 25
      lib/fs/logfs.go

+ 24 - 9
lib/fs/filesystem.go

@@ -261,11 +261,6 @@ func NewFilesystem(fsType FilesystemType, uri string, opts ...Option) Filesystem
 		}
 	}
 
-	// Case handling is the innermost, as any filesystem calls by wrappers should be case-resolved
-	if caseOpt != nil {
-		fs = caseOpt.apply(fs)
-	}
-
 	// mtime handling should happen inside walking, as filesystem calls while
 	// walking should be mtime-resolved too
 	if mtimeOpt != nil {
@@ -274,15 +269,35 @@ func NewFilesystem(fsType FilesystemType, uri string, opts ...Option) Filesystem
 
 	fs = &metricsFS{next: fs}
 
+	layersAboveWalkFilesystem := 0
+	if caseOpt != nil {
+		// DirNames calls made to check the case of a name will also be
+		// attributed to the calling function.
+		layersAboveWalkFilesystem++
+	}
 	if l.ShouldDebug("walkfs") {
-		return NewWalkFilesystem(&logFilesystem{fs})
+		// A walkFilesystem is not a layer to skip, it embeds the underlying
+		// filesystem, passing calls directly trough. Except for calls made
+		// during walking, however those are truly originating in the walk
+		// filesystem.
+		fs = NewWalkFilesystem(newLogFilesystem(fs, layersAboveWalkFilesystem))
+	} else if l.ShouldDebug("fs") {
+		fs = newLogFilesystem(NewWalkFilesystem(fs), layersAboveWalkFilesystem)
+	} else {
+		fs = NewWalkFilesystem(fs)
 	}
 
-	if l.ShouldDebug("fs") {
-		return &logFilesystem{NewWalkFilesystem(fs)}
+	// Case handling is at the outermost layer to resolve all input names.
+	// Reason being is that the only names/paths that are potentially "wrong"
+	// come from outside the fs package. Any paths that result from filesystem
+	// operations itself already have the correct case. Thus there's e.g. no
+	// point to check the case on all the stating the walk filesystem does, it
+	// just adds overhead.
+	if caseOpt != nil {
+		fs = caseOpt.apply(fs)
 	}
 
-	return NewWalkFilesystem(fs)
+	return fs
 }
 
 // IsInternal returns true if the file, as a path relative to the folder

+ 35 - 25
lib/fs/logfs.go

@@ -16,10 +16,20 @@ import (
 
 type logFilesystem struct {
 	Filesystem
+	// Number of filesystem layers on top of logFilesystem to skip when looking
+	// for the true caller of the filesystem
+	layers int
 }
 
-func getCaller() string {
-	_, file, line, ok := runtime.Caller(2)
+func newLogFilesystem(fs Filesystem, layers int) *logFilesystem {
+	return &logFilesystem{
+		Filesystem: fs,
+		layers:     layers,
+	}
+}
+
+func (fs *logFilesystem) getCaller() string {
+	_, file, line, ok := runtime.Caller(fs.layers + 1)
 	if !ok {
 		return "unknown"
 	}
@@ -28,139 +38,139 @@ func getCaller() string {
 
 func (fs *logFilesystem) Chmod(name string, mode FileMode) error {
 	err := fs.Filesystem.Chmod(name, mode)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "Chmod", name, mode, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "Chmod", name, mode, err)
 	return err
 }
 
 func (fs *logFilesystem) Chtimes(name string, atime time.Time, mtime time.Time) error {
 	err := fs.Filesystem.Chtimes(name, atime, mtime)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "Chtimes", name, atime, mtime, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "Chtimes", name, atime, mtime, err)
 	return err
 }
 
 func (fs *logFilesystem) Create(name string) (File, error) {
 	file, err := fs.Filesystem.Create(name)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "Create", name, file, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "Create", name, file, err)
 	return file, err
 }
 
 func (fs *logFilesystem) CreateSymlink(target, name string) error {
 	err := fs.Filesystem.CreateSymlink(target, name)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "CreateSymlink", target, name, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "CreateSymlink", target, name, err)
 	return err
 }
 
 func (fs *logFilesystem) DirNames(name string) ([]string, error) {
 	names, err := fs.Filesystem.DirNames(name)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "DirNames", name, names, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "DirNames", name, names, err)
 	return names, err
 }
 
 func (fs *logFilesystem) Lstat(name string) (FileInfo, error) {
 	info, err := fs.Filesystem.Lstat(name)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "Lstat", name, info, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "Lstat", name, info, err)
 	return info, err
 }
 
 func (fs *logFilesystem) Mkdir(name string, perm FileMode) error {
 	err := fs.Filesystem.Mkdir(name, perm)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "Mkdir", name, perm, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "Mkdir", name, perm, err)
 	return err
 }
 
 func (fs *logFilesystem) MkdirAll(name string, perm FileMode) error {
 	err := fs.Filesystem.MkdirAll(name, perm)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "MkdirAll", name, perm, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "MkdirAll", name, perm, err)
 	return err
 }
 
 func (fs *logFilesystem) Open(name string) (File, error) {
 	file, err := fs.Filesystem.Open(name)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "Open", name, file, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "Open", name, file, err)
 	return file, err
 }
 
 func (fs *logFilesystem) OpenFile(name string, flags int, mode FileMode) (File, error) {
 	file, err := fs.Filesystem.OpenFile(name, flags, mode)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "OpenFile", name, flags, mode, file, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "OpenFile", name, flags, mode, file, err)
 	return file, err
 }
 
 func (fs *logFilesystem) ReadSymlink(name string) (string, error) {
 	target, err := fs.Filesystem.ReadSymlink(name)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "ReadSymlink", name, target, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "ReadSymlink", name, target, err)
 	return target, err
 }
 
 func (fs *logFilesystem) Remove(name string) error {
 	err := fs.Filesystem.Remove(name)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "Remove", name, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "Remove", name, err)
 	return err
 }
 
 func (fs *logFilesystem) RemoveAll(name string) error {
 	err := fs.Filesystem.RemoveAll(name)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "RemoveAll", name, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "RemoveAll", name, err)
 	return err
 }
 
 func (fs *logFilesystem) Rename(oldname, newname string) error {
 	err := fs.Filesystem.Rename(oldname, newname)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "Rename", oldname, newname, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "Rename", oldname, newname, err)
 	return err
 }
 
 func (fs *logFilesystem) Stat(name string) (FileInfo, error) {
 	info, err := fs.Filesystem.Stat(name)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "Stat", name, info, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "Stat", name, info, err)
 	return info, err
 }
 
 func (fs *logFilesystem) SymlinksSupported() bool {
 	supported := fs.Filesystem.SymlinksSupported()
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "SymlinksSupported", supported)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "SymlinksSupported", supported)
 	return supported
 }
 
 func (fs *logFilesystem) Walk(root string, walkFn WalkFunc) error {
 	err := fs.Filesystem.Walk(root, walkFn)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "Walk", root, walkFn, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "Walk", root, walkFn, err)
 	return err
 }
 
 func (fs *logFilesystem) Watch(path string, ignore Matcher, ctx context.Context, ignorePerms bool) (<-chan Event, <-chan error, error) {
 	evChan, errChan, err := fs.Filesystem.Watch(path, ignore, ctx, ignorePerms)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "Watch", path, ignore, ignorePerms, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "Watch", path, ignore, ignorePerms, err)
 	return evChan, errChan, err
 }
 
 func (fs *logFilesystem) Unhide(name string) error {
 	err := fs.Filesystem.Unhide(name)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "Unhide", name, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "Unhide", name, err)
 	return err
 }
 
 func (fs *logFilesystem) Hide(name string) error {
 	err := fs.Filesystem.Hide(name)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "Hide", name, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "Hide", name, err)
 	return err
 }
 
 func (fs *logFilesystem) Glob(name string) ([]string, error) {
 	names, err := fs.Filesystem.Glob(name)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "Glob", name, names, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "Glob", name, names, err)
 	return names, err
 }
 
 func (fs *logFilesystem) Roots() ([]string, error) {
 	roots, err := fs.Filesystem.Roots()
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "Roots", roots, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "Roots", roots, err)
 	return roots, err
 }
 
 func (fs *logFilesystem) Usage(name string) (Usage, error) {
 	usage, err := fs.Filesystem.Usage(name)
-	l.Debugln(getCaller(), fs.Type(), fs.URI(), "Usage", name, usage, err)
+	l.Debugln(fs.getCaller(), fs.Type(), fs.URI(), "Usage", name, usage, err)
 	return usage, err
 }