From b80c56fef3dac9f5c7032b7bacd72083b8dddeae Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Fri, 13 Sep 2024 13:23:18 -0700 Subject: [PATCH 1/5] Add wrapper for io/fs Signed-off-by: Evan Anderson --- helper/iofs/iofs.go | 142 +++++++++++++++++++++++++++++++++++++++ helper/iofs/iofs_test.go | 102 ++++++++++++++++++++++++++++ 2 files changed, 244 insertions(+) create mode 100644 helper/iofs/iofs.go create mode 100644 helper/iofs/iofs_test.go diff --git a/helper/iofs/iofs.go b/helper/iofs/iofs.go new file mode 100644 index 0000000..6c71078 --- /dev/null +++ b/helper/iofs/iofs.go @@ -0,0 +1,142 @@ +// Package iofs provides an adapter from billy.Filesystem to a the +// standard library io.fs.FS interface. +package iofs + +import ( + "io" + "io/fs" + "path/filepath" + + billyfs "github.com/go-git/go-billy/v5" + "github.com/go-git/go-billy/v5/helper/polyfill" +) + +// Wrap adapts a billy.Filesystem to a io.fs.FS. +func Wrap(fs billyfs.Basic) fs.FS { + return &adapterFs{fs: polyfill.New(fs)} +} + +type adapterFs struct { + fs billyfs.Filesystem +} + +var _ fs.FS = (*adapterFs)(nil) +var _ fs.ReadDirFS = (*adapterFs)(nil) +var _ fs.StatFS = (*adapterFs)(nil) +var _ fs.ReadFileFS = (*adapterFs)(nil) + +// GlobFS would be harder, we don't implement for now. + +// Open implements fs.FS. +func (a *adapterFs) Open(name string) (fs.File, error) { + if name[0] == '/' || name != filepath.Clean(name) { + // fstest.TestFS explicitly checks that these should return error + // MemFS is performs the clean internally, so we need to block that here for testing. + return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrInvalid} + } + stat, err := a.fs.Stat(name) + if err != nil { + return nil, err + } + if stat.IsDir() { + entries, err := a.ReadDir(name) + if err != nil { + return nil, err + } + return makeDir(stat, entries), nil + } + file, err := a.fs.Open(name) + return &adapterFile{file: file, info: stat}, err +} + +// ReadDir implements fs.ReadDirFS. +func (a *adapterFs) ReadDir(name string) ([]fs.DirEntry, error) { + items, err := a.fs.ReadDir(name) + if err != nil { + return nil, err + } + entries := make([]fs.DirEntry, len(items)) + for i, item := range items { + entries[i] = fs.FileInfoToDirEntry(item) + } + return entries, nil +} + +// Stat implements fs.StatFS. +func (a *adapterFs) Stat(name string) (fs.FileInfo, error) { + return a.fs.Stat(name) +} + +// ReadFile implements fs.ReadFileFS. +func (a *adapterFs) ReadFile(name string) ([]byte, error) { + stat, err := a.fs.Stat(name) + if err != nil { + return nil, err + } + b := make([]byte, stat.Size()) + file, err := a.Open(name) + if err != nil { + return nil, err + } + defer file.Close() + _, err = file.Read(b) + return b, err +} + +type adapterFile struct { + file billyfs.File + info fs.FileInfo +} + +var _ fs.File = (*adapterFile)(nil) + +// Close implements fs.File. +func (a *adapterFile) Close() error { + return a.file.Close() +} + +// Read implements fs.File. +func (a *adapterFile) Read(b []byte) (int, error) { + return a.file.Read(b) +} + +// Stat implements fs.File. +func (a *adapterFile) Stat() (fs.FileInfo, error) { + return a.info, nil +} + +type adapterDirFile struct { + adapterFile + entries []fs.DirEntry +} + +var _ fs.ReadDirFile = (*adapterDirFile)(nil) + +func makeDir(stat fs.FileInfo, entries []fs.DirEntry) *adapterDirFile { + return &adapterDirFile{ + adapterFile: adapterFile{info: stat}, + entries: entries, + } +} + +// Close implements fs.File. +// Subtle: note that this is shadowing adapterFile.Close. +func (a *adapterDirFile) Close() error { + return nil +} + +// ReadDir implements fs.ReadDirFile. +func (a *adapterDirFile) ReadDir(n int) ([]fs.DirEntry, error) { + if len(a.entries) == 0 && n > 0 { + return nil, io.EOF + } + if n <= 0 { + n = len(a.entries) + } + if n > len(a.entries) { + n = len(a.entries) + } + entries := a.entries[:n] + a.entries = a.entries[n:] + return entries, nil +} \ No newline at end of file diff --git a/helper/iofs/iofs_test.go b/helper/iofs/iofs_test.go new file mode 100644 index 0000000..65d62fb --- /dev/null +++ b/helper/iofs/iofs_test.go @@ -0,0 +1,102 @@ +package iofs + +import ( + "errors" + "io/fs" + "strings" + "testing" + "testing/fstest" + + billyfs "github.com/go-git/go-billy/v5" + "github.com/go-git/go-billy/v5/memfs" +) + +type errorList interface { + Unwrap() []error +} + +type wrappedError interface { + Unwrap() error +} + +// TestWithFSTest leverages the packaged Go fstest package, which seems comprehensive +func TestWithFSTest(t *testing.T) { + t.Parallel() + memfs := memfs.New() + iofs := Wrap(memfs) + + files := map[string]string{ + "foo.txt": "hello, world", + "bar.txt": "goodbye, world", + "dir/baz.txt": "こんにちわ, world", + } + created_files := make([]string, 0, len(files)) + for filename, contents := range files { + makeFile(memfs, t, filename, contents) + created_files = append(created_files, filename) + } + + err := fstest.TestFS(iofs, created_files...) + if err != nil { + if unwrapped := errors.Unwrap(err); unwrapped != nil { + err = unwrapped + } + if errs, ok := err.(errorList); ok { + for _, e := range errs.Unwrap() { + + if strings.Contains(e.Error(), "ModTime") { + // Memfs returns the current time for Stat().ModTime(), which triggers + // a diff complaint in fstest. We can ignore this, or store modtimes + // for every file in Memfs (at a cost of 16 bytes / file). + t.Log("Skipping ModTime error (ok).") + } else { + t.Errorf("Unexpected fstest error: %v", e) + } + } + } else { + t.Fatalf("Failed to test fs:\n%v", err) + } + } +} + +func TestDeletes(t *testing.T) { + t.Parallel() + memfs := memfs.New() + iofs := Wrap(memfs).(fs.ReadFileFS) + + makeFile(memfs, t, "foo.txt", "hello, world") + makeFile(memfs, t, "deleted", "nothing to see") + + if _, err := iofs.ReadFile("nonexistent"); err == nil { + t.Errorf("expected error for nonexistent file") + } + + data, err := iofs.ReadFile("deleted") + if err != nil { + t.Fatalf("failed to read file before delete: %v", err) + } + if string(data) != "nothing to see" { + t.Errorf("unexpected contents before delete: %v", data) + } + + if err := memfs.Remove("deleted"); err != nil { + t.Fatalf("failed to remove file: %v", err) + } + + if _, err = iofs.ReadFile("deleted"); err == nil { + t.Errorf("file existed after delete!") + } +} + +func makeFile(fs billyfs.Basic, t *testing.T, filename string, contents string) { + t.Helper() + file, err := fs.Create(filename) + if err != nil { + t.Fatalf("failed to create file %s: %v", filename, err) + } + defer file.Close() + _, err = file.Write([]byte(contents)) + if err != nil { + t.Fatalf("failed to write to file %s: %v", filename, err) + } +} From db71dcbabb49e9dc874efeb4fec0501b3fefbdb5 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Tue, 17 Sep 2024 11:39:35 -0700 Subject: [PATCH 2/5] Add support for fstest on go < 1.23 --- helper/iofs/iofs_test.go | 82 +++++++++++++++++++++++++++++++--------- 1 file changed, 64 insertions(+), 18 deletions(-) diff --git a/helper/iofs/iofs_test.go b/helper/iofs/iofs_test.go index 65d62fb..db27f7c 100644 --- a/helper/iofs/iofs_test.go +++ b/helper/iofs/iofs_test.go @@ -3,6 +3,8 @@ package iofs import ( "errors" "io/fs" + "path/filepath" + "runtime" "strings" "testing" "testing/fstest" @@ -38,24 +40,7 @@ func TestWithFSTest(t *testing.T) { err := fstest.TestFS(iofs, created_files...) if err != nil { - if unwrapped := errors.Unwrap(err); unwrapped != nil { - err = unwrapped - } - if errs, ok := err.(errorList); ok { - for _, e := range errs.Unwrap() { - - if strings.Contains(e.Error(), "ModTime") { - // Memfs returns the current time for Stat().ModTime(), which triggers - // a diff complaint in fstest. We can ignore this, or store modtimes - // for every file in Memfs (at a cost of 16 bytes / file). - t.Log("Skipping ModTime error (ok).") - } else { - t.Errorf("Unexpected fstest error: %v", e) - } - } - } else { - t.Fatalf("Failed to test fs:\n%v", err) - } + checkFsTestError(t, err, files) } } @@ -100,3 +85,64 @@ func makeFile(fs billyfs.Basic, t *testing.T, filename string, contents string) t.Fatalf("failed to write to file %s: %v", filename, err) } } + +func checkFsTestError(t *testing.T, err error, files map[string]string) { + t.Helper() + + if unwrapped := errors.Unwrap(err); unwrapped != nil { + err = unwrapped + } + + // Go >= 1.23 (after https://cs.opensource.google/go/go/+/74cce866f865c3188a34309e4ebc7a5c9ed0683d) + // has nicely-Joined wrapped errors. Try that first. + if errs, ok := err.(errorList); ok { + for _, e := range errs.Unwrap() { + + if strings.Contains(e.Error(), "ModTime") { + // Memfs returns the current time for Stat().ModTime(), which triggers + // a diff complaint in fstest. We can ignore this, or store modtimes + // for every file in Memfs (at a cost of 16 bytes / file). + t.Log("Skipping ModTime error (ok).") + } else { + t.Errorf("Unexpected fstest error: %v", e) + } + } + } else { + if runtime.Version() >= "go1.23" { + t.Fatalf("Failed to test fs:\n%v", err) + } + // filter lines from the error text corresponding to the above errors; + // output looks like: + // bar.txt: mismatch: + // entry.Info() = bar.txt IsDir=false Mode=-rw-rw-rw- Size=14 ModTime=2024-09-17 10:09:00.377023639 +0000 UTC m=+0.002625548 + // file.Stat() = bar.txt IsDir=false Mode=-rw-rw-rw- Size=14 ModTime=2024-09-17 10:09:00.376907011 +0000 UTC m=+0.002508970 + // + // bar.txt: fs.Stat(...) = bar.txt IsDir=false Mode=-rw-rw-rw- Size=14 ModTime=2024-09-17 10:09:00.381356651 +0000 UTC m=+0.006959191 + // want bar.txt IsDir=false Mode=-rw-rw-rw- Size=14 ModTime=2024-09-17 10:09:00.376907011 +0000 UTC m=+0.002508970 + // bar.txt: fsys.Stat(...) = bar.txt IsDir=false Mode=-rw-rw-rw- Size=14 ModTime=2024-09-17 10:09:00.381488617 +0000 UTC m=+0.007090346 + // want bar.txt IsDir=false Mode=-rw-rw-rw- Size=14 ModTime=2024-09-17 10:09:00.376907011 +0000 UTC m=+0.002508970 + // We filter on "empty line" or "ModTime" or "$filename: mismatch" to ignore these. + lines := strings.Split(err.Error(), "\n") + filtered := make([]string, 0, len(lines)) + filename_mismatches := make(map[string]struct{}, len(files) * 2) + for name, _ := range files { + for dirname := name; dirname != "."; dirname = filepath.Dir(dirname) { + filename_mismatches[dirname + ": mismatch:"] = struct{}{} + } + } + for _, line := range lines { + line = strings.TrimSpace(line) + if line == "" || strings.Contains(line, "ModTime=") { + continue + } + + if _, ok := filename_mismatches[line]; ok { + continue + } + filtered = append(filtered, line) + } + if len(filtered) > 0 { + t.Fatalf("Failed to test fs:\n%s", strings.Join(filtered, "\n")) + } + } +} From 28f6c4986d403626559b92aa275cac0ceeda6439 Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Tue, 17 Sep 2024 16:35:49 -0700 Subject: [PATCH 3/5] Fix test handling on go < 1.23 --- helper/iofs/iofs_test.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/helper/iofs/iofs_test.go b/helper/iofs/iofs_test.go index db27f7c..4f14765 100644 --- a/helper/iofs/iofs_test.go +++ b/helper/iofs/iofs_test.go @@ -113,6 +113,7 @@ func checkFsTestError(t *testing.T, err error, files map[string]string) { } // filter lines from the error text corresponding to the above errors; // output looks like: + // TestFS found errors: // bar.txt: mismatch: // entry.Info() = bar.txt IsDir=false Mode=-rw-rw-rw- Size=14 ModTime=2024-09-17 10:09:00.377023639 +0000 UTC m=+0.002625548 // file.Stat() = bar.txt IsDir=false Mode=-rw-rw-rw- Size=14 ModTime=2024-09-17 10:09:00.376907011 +0000 UTC m=+0.002508970 @@ -124,19 +125,22 @@ func checkFsTestError(t *testing.T, err error, files map[string]string) { // We filter on "empty line" or "ModTime" or "$filename: mismatch" to ignore these. lines := strings.Split(err.Error(), "\n") filtered := make([]string, 0, len(lines)) - filename_mismatches := make(map[string]struct{}, len(files) * 2) - for name, _ := range files { + filename_mismatches := make(map[string]struct{}, len(files)*2) + for name := range files { for dirname := name; dirname != "."; dirname = filepath.Dir(dirname) { - filename_mismatches[dirname + ": mismatch:"] = struct{}{} + filename_mismatches[dirname+": mismatch:"] = struct{}{} } } + if strings.TrimSpace(lines[0]) == "TestFS found errors:" { + lines = lines[1:] + } for _, line := range lines { - line = strings.TrimSpace(line) - if line == "" || strings.Contains(line, "ModTime=") { + trimmed := strings.TrimSpace(line) + if trimmed == "" || strings.Contains(trimmed, "ModTime=") { continue } - if _, ok := filename_mismatches[line]; ok { + if _, ok := filename_mismatches[trimmed]; ok { continue } filtered = append(filtered, line) From b8c5b1bd59024b0644aaef23ed43177ca8301f7f Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Sat, 21 Sep 2024 22:53:54 -0700 Subject: [PATCH 4/5] Prevent test failures on Windows, address feedback from pjbgf --- helper/iofs/iofs.go | 32 +++++++++++++++----------------- helper/iofs/iofs_test.go | 12 ++++++++---- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/helper/iofs/iofs.go b/helper/iofs/iofs.go index 6c71078..eb5a4b4 100644 --- a/helper/iofs/iofs.go +++ b/helper/iofs/iofs.go @@ -20,18 +20,19 @@ type adapterFs struct { fs billyfs.Filesystem } +// Type assertion that adapterFS implements the following interfaces: var _ fs.FS = (*adapterFs)(nil) var _ fs.ReadDirFS = (*adapterFs)(nil) var _ fs.StatFS = (*adapterFs)(nil) var _ fs.ReadFileFS = (*adapterFs)(nil) -// GlobFS would be harder, we don't implement for now. +// TODO: implement fs.GlobFS, which will be a fair bit more code. -// Open implements fs.FS. +// Open opens the named file on the underlying FS, implementing fs.FS (returning a file or error). func (a *adapterFs) Open(name string) (fs.File, error) { if name[0] == '/' || name != filepath.Clean(name) { - // fstest.TestFS explicitly checks that these should return error - // MemFS is performs the clean internally, so we need to block that here for testing. + // fstest.TestFS explicitly checks that these should return error. + // MemFS performs the clean internally, so we need to block that here for testing purposes. return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrInvalid} } stat, err := a.fs.Stat(name) @@ -49,7 +50,7 @@ func (a *adapterFs) Open(name string) (fs.File, error) { return &adapterFile{file: file, info: stat}, err } -// ReadDir implements fs.ReadDirFS. +// ReadDir reads the named directory, implementing fs.ReadDirFS (returning a listing or error). func (a *adapterFs) ReadDir(name string) ([]fs.DirEntry, error) { items, err := a.fs.ReadDir(name) if err != nil { @@ -62,12 +63,12 @@ func (a *adapterFs) ReadDir(name string) ([]fs.DirEntry, error) { return entries, nil } -// Stat implements fs.StatFS. +// Stat returns information on the named file, implementing fs.StatFS (returning FileInfo or error). func (a *adapterFs) Stat(name string) (fs.FileInfo, error) { return a.fs.Stat(name) } -// ReadFile implements fs.ReadFileFS. +// ReadFile reads the named file and returns its contents, implementing fs.ReadFileFS (returning contents or error). func (a *adapterFs) ReadFile(name string) ([]byte, error) { stat, err := a.fs.Stat(name) if err != nil { @@ -90,17 +91,17 @@ type adapterFile struct { var _ fs.File = (*adapterFile)(nil) -// Close implements fs.File. +// Close closes the file, implementing fs.File (and io.Closer). func (a *adapterFile) Close() error { return a.file.Close() } -// Read implements fs.File. +// Read reads bytes from the file, implementing fs.File (and io.Reader). func (a *adapterFile) Read(b []byte) (int, error) { return a.file.Read(b) } -// Stat implements fs.File. +// Stat returns file information, implementing fs.File (returning FileInfo or error). func (a *adapterFile) Stat() (fs.FileInfo, error) { return a.info, nil } @@ -119,24 +120,21 @@ func makeDir(stat fs.FileInfo, entries []fs.DirEntry) *adapterDirFile { } } -// Close implements fs.File. +// Close closes the directory, implementing fs.File (and io.Closer). // Subtle: note that this is shadowing adapterFile.Close. func (a *adapterDirFile) Close() error { return nil } -// ReadDir implements fs.ReadDirFile. +// ReadDir reads the directory contents, implementing fs.ReadDirFile (returning directory listing or error). func (a *adapterDirFile) ReadDir(n int) ([]fs.DirEntry, error) { if len(a.entries) == 0 && n > 0 { return nil, io.EOF } - if n <= 0 { - n = len(a.entries) - } - if n > len(a.entries) { + if n <= 0 || n > len(a.entries) { n = len(a.entries) } entries := a.entries[:n] a.entries = a.entries[n:] return entries, nil -} \ No newline at end of file +} diff --git a/helper/iofs/iofs_test.go b/helper/iofs/iofs_test.go index 4f14765..e9bfd8c 100644 --- a/helper/iofs/iofs_test.go +++ b/helper/iofs/iofs_test.go @@ -21,16 +21,16 @@ type wrappedError interface { Unwrap() error } -// TestWithFSTest leverages the packaged Go fstest package, which seems comprehensive +// TestWithFSTest leverages the packaged Go fstest package, which seems comprehensive. func TestWithFSTest(t *testing.T) { t.Parallel() memfs := memfs.New() iofs := Wrap(memfs) files := map[string]string{ - "foo.txt": "hello, world", - "bar.txt": "goodbye, world", - "dir/baz.txt": "こんにちわ, world", + "foo.txt": "hello, world", + "bar.txt": "goodbye, world", + filepath.Join("dir", "baz.txt"): "こんにちわ, world", } created_files := make([]string, 0, len(files)) for filename, contents := range files { @@ -38,6 +38,10 @@ func TestWithFSTest(t *testing.T) { created_files = append(created_files, filename) } + if runtime.GOOS == "windows" { + t.Skip("fstest.TestFS is not yet windows path aware") + } + err := fstest.TestFS(iofs, created_files...) if err != nil { checkFsTestError(t, err, files) From b50bc97051ea4bdfef26dd45fdc00d6611483f0b Mon Sep 17 00:00:00 2001 From: Evan Anderson Date: Fri, 27 Sep 2024 09:32:57 +0200 Subject: [PATCH 5/5] Rename Wrap to New --- helper/iofs/iofs.go | 2 +- helper/iofs/iofs_test.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/helper/iofs/iofs.go b/helper/iofs/iofs.go index eb5a4b4..be8f348 100644 --- a/helper/iofs/iofs.go +++ b/helper/iofs/iofs.go @@ -12,7 +12,7 @@ import ( ) // Wrap adapts a billy.Filesystem to a io.fs.FS. -func Wrap(fs billyfs.Basic) fs.FS { +func New(fs billyfs.Basic) fs.FS { return &adapterFs{fs: polyfill.New(fs)} } diff --git a/helper/iofs/iofs_test.go b/helper/iofs/iofs_test.go index e9bfd8c..02d25de 100644 --- a/helper/iofs/iofs_test.go +++ b/helper/iofs/iofs_test.go @@ -25,7 +25,7 @@ type wrappedError interface { func TestWithFSTest(t *testing.T) { t.Parallel() memfs := memfs.New() - iofs := Wrap(memfs) + iofs := New(memfs) files := map[string]string{ "foo.txt": "hello, world", @@ -51,7 +51,7 @@ func TestWithFSTest(t *testing.T) { func TestDeletes(t *testing.T) { t.Parallel() memfs := memfs.New() - iofs := Wrap(memfs).(fs.ReadFileFS) + iofs := New(memfs).(fs.ReadFileFS) makeFile(memfs, t, "foo.txt", "hello, world") makeFile(memfs, t, "deleted", "nothing to see")