From 5e9f8ecaa4af2dddc638b1a8a4a58834f54878aa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B8rn=20Erik=20Pedersen?= Date: Sun, 16 Dec 2018 11:57:17 +0100 Subject: [PATCH] Return os.ErrExist from CopyOnWriteFs.Mkdir/MkdirAll Before this commit, `CopyOnWriteFs` would return `syscall.EEXIST` in `Mkdir` and `MkdirAll` when a directory already exists. The main problem with this is that `os.IsExist` returns `false` for that error on Windows. These methods now return `os.ErrExist`, which is in line with how the other file systems behave. Fixes #189 --- copyOnWriteFs.go | 4 ++-- copyOnWriteFs_test.go | 51 +++++++++++++++++++++++++++++++------------ go.mod | 2 ++ go.sum | 2 ++ 4 files changed, 43 insertions(+), 16 deletions(-) create mode 100644 go.sum diff --git a/copyOnWriteFs.go b/copyOnWriteFs.go index 9aef397..f5dd237 100644 --- a/copyOnWriteFs.go +++ b/copyOnWriteFs.go @@ -267,7 +267,7 @@ func (u *CopyOnWriteFs) Mkdir(name string, perm os.FileMode) error { return u.layer.MkdirAll(name, perm) } if dir { - return syscall.EEXIST + return ErrFileExists } return u.layer.MkdirAll(name, perm) } @@ -282,7 +282,7 @@ func (u *CopyOnWriteFs) MkdirAll(name string, perm os.FileMode) error { return u.layer.MkdirAll(name, perm) } if dir { - return syscall.EEXIST + return ErrFileExists } return u.layer.MkdirAll(name, perm) } diff --git a/copyOnWriteFs_test.go b/copyOnWriteFs_test.go index 0f199dc..a6cbc00 100644 --- a/copyOnWriteFs_test.go +++ b/copyOnWriteFs_test.go @@ -1,26 +1,49 @@ package afero -import "testing" +import ( + "os" + "path/filepath" + "testing" +) func TestCopyOnWrite(t *testing.T) { - var fs Fs - var err error - base := NewOsFs() - roBase := NewReadOnlyFs(base) - ufs := NewCopyOnWriteFs(roBase, NewMemMapFs()) - - fs = ufs - err = fs.MkdirAll("nonexistent/directory/", 0744) + osFs := NewOsFs() + writeDir, err := TempDir(osFs, "", "copy-on-write-test") if err != nil { - t.Error(err) - return + t.Fatal("error creating tempDir", err) } - _, err = fs.Create("nonexistent/directory/newfile") + defer osFs.RemoveAll(writeDir) + + compositeFs := NewCopyOnWriteFs(NewReadOnlyFs(NewOsFs()), osFs) + + var dir = filepath.Join(writeDir, "some/path") + + err = compositeFs.MkdirAll(dir, 0744) if err != nil { - t.Error(err) - return + t.Fatal(err) + } + _, err = compositeFs.Create(filepath.Join(dir, "newfile")) + if err != nil { + t.Fatal(err) } + // https://github.com/spf13/afero/issues/189 + // We want the composite file system to behave like the OS file system + // on Mkdir and MkdirAll + for _, fs := range []Fs{osFs, compositeFs} { + err = fs.Mkdir(dir, 0744) + if err == nil || !os.IsExist(err) { + t.Errorf("Mkdir: Got %q for %T", err, fs) + } + + // https://github.com/spf13/afero/issues/191 + if _, ok := fs.(*OsFs); !ok { + err = fs.MkdirAll(dir, 0744) + if err == nil || !os.IsExist(err) { + t.Errorf("MkdirAll: Got %q for %T", err, fs) + } + } + } } func TestCopyOnWriteFileInMemMapBase(t *testing.T) { diff --git a/go.mod b/go.mod index 9eff4fe..0868550 100644 --- a/go.mod +++ b/go.mod @@ -1 +1,3 @@ module github.com/spf13/afero + +require golang.org/x/text v0.3.0 diff --git a/go.sum b/go.sum new file mode 100644 index 0000000..6bad37b --- /dev/null +++ b/go.sum @@ -0,0 +1,2 @@ +golang.org/x/text v0.3.0 h1:g61tztE5qeGQ89tm6NTjjM9VPIm088od1l6aSorWRWg= +golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=