From 06a23b5455d9a050558dd3d6800f93352caa2734 Mon Sep 17 00:00:00 2001 From: Nicolas Simonds Date: Thu, 24 Apr 2025 18:05:37 +1000 Subject: [PATCH 1/3] osfs: add `file.Sync()` support Adds a call to `file.Sync()` to `util.WriteFile()` (right before closing the file, so it should be functionally equivalent) to ensure test coverage. Fixes: Issue #86 --- embedfs/embed.go | 5 +++++ fs.go | 2 ++ internal/test/mock.go | 4 ++++ memfs/file.go | 5 +++++ util/util.go | 4 ++++ 5 files changed, 20 insertions(+) diff --git a/embedfs/embed.go b/embedfs/embed.go index d0b7c8c..c665b4e 100644 --- a/embedfs/embed.go +++ b/embedfs/embed.go @@ -236,6 +236,11 @@ func (f *file) Truncate(_ int64) error { return billy.ErrReadOnly } +// Sync for embedfs file is a no-op. +func (f *file) Sync() error { + return nil +} + // Write is not supported. // // Calls will always return billy.ErrReadOnly. diff --git a/fs.go b/fs.go index fe66099..d00b0e5 100644 --- a/fs.go +++ b/fs.go @@ -178,6 +178,8 @@ type File interface { Unlock() error // Truncate the file. Truncate(size int64) error + // Commit the current contents of the file to stable storage. + Sync() error } // Capable interface can return the available features of a filesystem. diff --git a/internal/test/mock.go b/internal/test/mock.go index a7bd7c0..f672cc8 100644 --- a/internal/test/mock.go +++ b/internal/test/mock.go @@ -140,6 +140,10 @@ func (*FileMock) Stat() (fs.FileInfo, error) { return nil, nil } +func (*FileMock) Sync() error { + return nil +} + func (*FileMock) Truncate(_ int64) error { return nil } diff --git a/memfs/file.go b/memfs/file.go index 7ebe76b..033cdae 100644 --- a/memfs/file.go +++ b/memfs/file.go @@ -146,6 +146,11 @@ func (f *file) Unlock() error { return nil } +// Sync is a no-op in memfs. +func (f *file) Sync() error { + return nil +} + type fileInfo struct { name string size int diff --git a/util/util.go b/util/util.go index 24b84fa..e41cff6 100644 --- a/util/util.go +++ b/util/util.go @@ -116,6 +116,10 @@ func WriteFile(fs billy.Basic, filename string, data []byte, perm fs.FileMode) ( if err == nil && n < len(data) { err = io.ErrShortWrite } + err = f.Sync() + if err != nil { + return err + } return nil } From a2666e48b7af2bf67ac7460fd90b4750cd1b71d3 Mon Sep 17 00:00:00 2001 From: Georg Friedrich Date: Tue, 14 Oct 2025 18:18:24 +1100 Subject: [PATCH 2/3] add SyncCapability and Syncer interface Also add CallLogger to mock file system to record file system calls. The current implementation only logs sync calls but can be extended as needed. --- embedfs/embed.go | 5 ----- fs.go | 10 ++++++++-- internal/test/mock.go | 15 +++++++++++++-- memfs/file.go | 5 ----- memfs/memory_test.go | 2 +- osfs/os_plan9.go | 4 ++++ osfs/os_posix.go | 4 ++++ osfs/os_windows.go | 4 ++++ util/util.go | 8 +++++--- util/util_test.go | 13 +++++++++++++ 10 files changed, 52 insertions(+), 18 deletions(-) diff --git a/embedfs/embed.go b/embedfs/embed.go index c665b4e..d0b7c8c 100644 --- a/embedfs/embed.go +++ b/embedfs/embed.go @@ -236,11 +236,6 @@ func (f *file) Truncate(_ int64) error { return billy.ErrReadOnly } -// Sync for embedfs file is a no-op. -func (f *file) Sync() error { - return nil -} - // Write is not supported. // // Calls will always return billy.ErrReadOnly. diff --git a/fs.go b/fs.go index d00b0e5..43c5cc5 100644 --- a/fs.go +++ b/fs.go @@ -34,18 +34,20 @@ const ( TruncateCapability // LockCapability is the ability to lock a file. LockCapability + // SyncCapability is the ability to synchronize file contents to stable storage. + SyncCapability // DefaultCapabilities lists all capable features supported by filesystems // without Capability interface. This list should not be changed until a // major version is released. DefaultCapabilities Capability = WriteCapability | ReadCapability | ReadAndWriteCapability | SeekCapability | TruncateCapability | - LockCapability + LockCapability | SyncCapability // AllCapabilities lists all capable features. AllCapabilities Capability = WriteCapability | ReadCapability | ReadAndWriteCapability | SeekCapability | TruncateCapability | - LockCapability + LockCapability | SyncCapability ) // Filesystem abstract the operations in a storage-agnostic interface. @@ -178,6 +180,10 @@ type File interface { Unlock() error // Truncate the file. Truncate(size int64) error +} + +// Syncer interface can be implemented by filesystems that support syncing. +type Syncer interface { // Commit the current contents of the file to stable storage. Sync() error } diff --git a/internal/test/mock.go b/internal/test/mock.go index f672cc8..586cd98 100644 --- a/internal/test/mock.go +++ b/internal/test/mock.go @@ -10,6 +10,14 @@ import ( "github.com/go-git/go-billy/v6" ) +type CallLogger struct { + Calls []string +} + +func (l *CallLogger) Log(call string, args string) { + l.Calls = append(l.Calls, call+" "+args) +} + type BasicMock struct { CreateArgs []string OpenArgs []string @@ -18,6 +26,7 @@ type BasicMock struct { RenameArgs [][2]string RemoveArgs []string JoinArgs [][]string + CallLogger CallLogger } func (fs *BasicMock) Create(filename string) (billy.File, error) { @@ -32,7 +41,7 @@ func (fs *BasicMock) Open(filename string) (billy.File, error) { func (fs *BasicMock) OpenFile(filename string, flag int, mode fs.FileMode) (billy.File, error) { fs.OpenFileArgs = append(fs.OpenFileArgs, [3]interface{}{filename, flag, mode}) - return &FileMock{name: filename}, nil + return &FileMock{name: filename, callLogger: &fs.CallLogger}, nil } func (fs *BasicMock) Stat(filename string) (os.FileInfo, error) { @@ -106,6 +115,7 @@ func (fs *SymlinkMock) Readlink(link string) (string, error) { type FileMock struct { name string bytes.Buffer + callLogger *CallLogger } func (f *FileMock) Name() string { @@ -140,7 +150,8 @@ func (*FileMock) Stat() (fs.FileInfo, error) { return nil, nil } -func (*FileMock) Sync() error { +func (f *FileMock) Sync() error { + f.callLogger.Log("Sync", f.name) return nil } diff --git a/memfs/file.go b/memfs/file.go index 033cdae..7ebe76b 100644 --- a/memfs/file.go +++ b/memfs/file.go @@ -146,11 +146,6 @@ func (f *file) Unlock() error { return nil } -// Sync is a no-op in memfs. -func (f *file) Sync() error { - return nil -} - type fileInfo struct { name string size int diff --git a/memfs/memory_test.go b/memfs/memory_test.go index 96391c7..8882a50 100644 --- a/memfs/memory_test.go +++ b/memfs/memory_test.go @@ -31,7 +31,7 @@ func TestCapabilities(t *testing.T) { assert.True(t, ok) caps := billy.Capabilities(fs) - assert.Equal(t, billy.DefaultCapabilities&^billy.LockCapability, caps) + assert.Equal(t, billy.DefaultCapabilities&^billy.LockCapability&^billy.SyncCapability, caps) } func TestModTime(t *testing.T) { diff --git a/osfs/os_plan9.go b/osfs/os_plan9.go index 84020b5..aa366d9 100644 --- a/osfs/os_plan9.go +++ b/osfs/os_plan9.go @@ -27,6 +27,10 @@ func (f *file) Unlock() error { return nil } +func (f *file) Sync() error { + return f.File.Sync() +} + func rename(from, to string) error { // If from and to are in different directories, copy the file // since Plan 9 does not support cross-directory rename. diff --git a/osfs/os_posix.go b/osfs/os_posix.go index de511ed..9df0e5e 100644 --- a/osfs/os_posix.go +++ b/osfs/os_posix.go @@ -24,6 +24,10 @@ func (f *file) Unlock() error { return unix.Flock(int(f.File.Fd()), unix.LOCK_UN) } +func (f *file) Sync() error { + return f.File.Sync() +} + func rename(from, to string) error { return os.Rename(from, to) } diff --git a/osfs/os_windows.go b/osfs/os_windows.go index e23cede..eee689d 100644 --- a/osfs/os_windows.go +++ b/osfs/os_windows.go @@ -48,6 +48,10 @@ func (f *file) Unlock() error { return nil } +func (f *file) Sync() error { + return f.File.Sync() +} + func rename(from, to string) error { return os.Rename(from, to) } diff --git a/util/util.go b/util/util.go index e41cff6..e657a76 100644 --- a/util/util.go +++ b/util/util.go @@ -116,9 +116,11 @@ func WriteFile(fs billy.Basic, filename string, data []byte, perm fs.FileMode) ( if err == nil && n < len(data) { err = io.ErrShortWrite } - err = f.Sync() - if err != nil { - return err + if sf, ok := f.(billy.Syncer); ok { + err = sf.Sync() + if err != nil { + return err + } } return nil diff --git a/util/util_test.go b/util/util_test.go index d142179..429fb44 100644 --- a/util/util_test.go +++ b/util/util_test.go @@ -6,8 +6,10 @@ import ( "regexp" "testing" + "github.com/go-git/go-billy/v6/internal/test" "github.com/go-git/go-billy/v6/memfs" "github.com/go-git/go-billy/v6/util" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -90,3 +92,14 @@ func TestTempDir_WithNonRoot(t *testing.T) { t.Errorf(`TempDir(fs, "", "") = %s, should not be relative to os.TempDir on not root filesystem`, f) } } + +func TestWriteFile_Sync(t *testing.T) { + fs := &test.BasicMock{} + filename := "TestWriteFile.txt" + data := []byte("hello world") + err := util.WriteFile(fs, filename, data, 0644) + require.NoError(t, err) + + assert.Len(t, fs.CallLogger.Calls, 1) + assert.Equal(t, "Sync TestWriteFile.txt", fs.CallLogger.Calls[0]) +} From a0ca0d8be3f482d29eb6582cd8d9b8b9400724da Mon Sep 17 00:00:00 2001 From: Georg Friedrich Date: Wed, 15 Oct 2025 09:53:52 +1100 Subject: [PATCH 3/3] SyncCapability is not a DefaultCapability --- fs.go | 4 ++-- memfs/memory_test.go | 2 +- osfs/os_bound.go | 4 ++++ osfs/os_bound_test.go | 10 ++++++++++ osfs/os_chroot_test.go | 2 +- util/util.go | 5 +---- 6 files changed, 19 insertions(+), 8 deletions(-) diff --git a/fs.go b/fs.go index 43c5cc5..0c9f22e 100644 --- a/fs.go +++ b/fs.go @@ -42,7 +42,7 @@ const ( // major version is released. DefaultCapabilities Capability = WriteCapability | ReadCapability | ReadAndWriteCapability | SeekCapability | TruncateCapability | - LockCapability | SyncCapability + LockCapability // AllCapabilities lists all capable features. AllCapabilities Capability = WriteCapability | ReadCapability | @@ -184,7 +184,7 @@ type File interface { // Syncer interface can be implemented by filesystems that support syncing. type Syncer interface { - // Commit the current contents of the file to stable storage. + // Sync commits the current contents of the file to stable storage. Sync() error } diff --git a/memfs/memory_test.go b/memfs/memory_test.go index 01aef20..1c7d1e9 100644 --- a/memfs/memory_test.go +++ b/memfs/memory_test.go @@ -31,7 +31,7 @@ func TestCapabilities(t *testing.T) { assert.True(t, ok) caps := billy.Capabilities(fs) - assert.Equal(t, billy.DefaultCapabilities&^billy.LockCapability&^billy.SyncCapability, caps) + assert.Equal(t, billy.DefaultCapabilities&^billy.LockCapability, caps) } func TestModTime(t *testing.T) { diff --git a/osfs/os_bound.go b/osfs/os_bound.go index 953cafc..333018d 100644 --- a/osfs/os_bound.go +++ b/osfs/os_bound.go @@ -54,6 +54,10 @@ func newBoundOS(d string, deduplicatePath bool) billy.Filesystem { return &BoundOS{baseDir: d, deduplicatePath: deduplicatePath} } +func (fs *BoundOS) Capabilities() billy.Capability { + return billy.DefaultCapabilities & billy.SyncCapability +} + func (fs *BoundOS) Create(filename string) (billy.File, error) { return fs.OpenFile(filename, os.O_RDWR|os.O_CREATE|os.O_TRUNC, defaultCreateMode) } diff --git a/osfs/os_bound_test.go b/osfs/os_bound_test.go index 78273d3..259232b 100644 --- a/osfs/os_bound_test.go +++ b/osfs/os_bound_test.go @@ -32,6 +32,16 @@ import ( "github.com/stretchr/testify/require" ) +func TestBoundOSCapabilities(t *testing.T) { + dir := t.TempDir() + fs := newBoundOS(dir, true) + _, ok := fs.(billy.Capable) + assert.True(t, ok) + + caps := billy.Capabilities(fs) + assert.Equal(t, billy.DefaultCapabilities&billy.SyncCapability, caps) +} + func TestOpen(t *testing.T) { tests := []struct { name string diff --git a/osfs/os_chroot_test.go b/osfs/os_chroot_test.go index ff3fecf..011ec41 100644 --- a/osfs/os_chroot_test.go +++ b/osfs/os_chroot_test.go @@ -45,5 +45,5 @@ func TestCapabilities(t *testing.T) { assert.True(t, ok) caps := billy.Capabilities(fs) - assert.Equal(t, billy.AllCapabilities, caps) + assert.Equal(t, billy.DefaultCapabilities, caps) } diff --git a/util/util.go b/util/util.go index e657a76..49637c7 100644 --- a/util/util.go +++ b/util/util.go @@ -117,10 +117,7 @@ func WriteFile(fs billy.Basic, filename string, data []byte, perm fs.FileMode) ( err = io.ErrShortWrite } if sf, ok := f.(billy.Syncer); ok { - err = sf.Sync() - if err != nil { - return err - } + return sf.Sync() } return nil