Make OsFs.Open interoperable with others (with tests confirming)

composite_test.go

@@ -1,15 +1,52 @@
 package afero
 
 import (
+	"fmt"
 	"io/ioutil"
 	"os"
 	"testing"
 	"time"
-	"fmt"
 )
 
 var tempDirs []string
 
+func NewTempOsBaseFs(t *testing.T) Fs {
+	name, err := TempDir(NewOsFs(), "", "")
+	if err != nil {
+		t.Error("error creating tempDir", err)
+	}
+
+	tempDirs = append(tempDirs, name)
+
+	return NewBasePathFs(NewOsFs(), name)
+}
+
+func CleanupTempDirs(t *testing.T) {
+	osfs := NewOsFs()
+	type ev struct{
+		path string
+		e error
+	}
+
+	errs := []ev{}
+
+	for _, x := range tempDirs {
+		err := osfs.RemoveAll(x)
+		if err != nil {
+			errs = append(errs, ev{path:x,e: err})
+		}
+	}
+
+	for _, e := range errs {
+		fmt.Println("error removing tempDir", e.path, e.e)
+	}
+
+	if len(errs) > 0 {
+		t.Error("error cleaning up tempDirs")
+	}
+	tempDirs = []string{}
+}
+
 func TestUnionCreateExisting(t *testing.T) {
 	base := &MemMapFs{}
 	roBase := &ReadOnlyFs{source: base}
@@ -125,31 +162,110 @@ func TestExistingDirectoryCollisionReaddir(t *testing.T) {
 	}
 }
 
-func NewTempOsBaseFs(t *testing.T) Fs {
-	name, err := TempDir(NewOsFs(), "", "")
+func TestNestedDirBaseReaddir(t *testing.T) {
+	base := &MemMapFs{}
+	roBase := &ReadOnlyFs{source: base}
+	overlay := &MemMapFs{}
+
+	ufs := &CopyOnWriteFs{base: roBase, layer: overlay}
+
+	base.MkdirAll("/home/test/foo/bar", 0777)
+	fh, _ := base.Create("/home/test/file.txt")
+	fh.WriteString("This is a test")
+	fh.Close()
+
+	fh, _ = base.Create("/home/test/foo/file2.txt")
+	fh.WriteString("This is a test")
+	fh.Close()
+	fh, _ = base.Create("/home/test/foo/bar/file3.txt")
+	fh.WriteString("This is a test")
+	fh.Close()
+
+	overlay.MkdirAll("/", 0777)
+
+	// Opening something only in the base
+	fh, _ = ufs.Open("/home/test/foo")
+	list, err := fh.Readdir(-1)
 	if err != nil {
-		t.Error("error creating tempDir", err)
+		t.Errorf("Readdir failed", err)
+	}
+	if len(list) != 2 {
+		for _, x := range list {
+			fmt.Println(x.Name())
+		}
+		t.Errorf("Got wrong number of files in union: %v", len(list))
 	}
-
-	fmt.Println("created tempdir", name)
-	tempDirs = append(tempDirs, name)
-
-	return NewBasePathFs(NewOsFs(), name)
 }
 
-func CleanupTempDirs() {
-	osfs := NewOsFs()
-	for _, x := range tempDirs {
-		err := osfs.RemoveAll(x)
-		if err != nil {
-			fmt.Println("error removing tempDir", x, err)
-		}
+func TestNestedDirOverlayReaddir(t *testing.T) {
+	base := &MemMapFs{}
+	roBase := &ReadOnlyFs{source: base}
+	overlay := &MemMapFs{}
+
+	ufs := &CopyOnWriteFs{base: roBase, layer: overlay}
+
+	base.MkdirAll("/", 0777)
+	overlay.MkdirAll("/home/test/foo/bar", 0777)
+	fh, _ := overlay.Create("/home/test/file.txt")
+	fh.WriteString("This is a test")
+	fh.Close()
+	fh, _ = overlay.Create("/home/test/foo/file2.txt")
+	fh.WriteString("This is a test")
+	fh.Close()
+	fh, _ = overlay.Create("/home/test/foo/bar/file3.txt")
+	fh.WriteString("This is a test")
+	fh.Close()
+
+	// Opening nested dir only in the overlay
+	fh, _ = ufs.Open("/home/test/foo")
+	list, err := fh.Readdir(-1)
+	if err != nil {
+		t.Errorf("Readdir failed", err)
+	}
+	if len(list) != 2 {
+		for _, x := range list {
+			fmt.Println(x.Name())
+		}
+		t.Errorf("Got wrong number of files in union: %v", len(list))
+	}
+}
+
+func TestNestedDirOverlayOsFsReaddir(t *testing.T) {
+	defer CleanupTempDirs(t)
+	base := NewTempOsBaseFs(t)
+	roBase := &ReadOnlyFs{source: base}
+	overlay := NewTempOsBaseFs(t)
+
+	ufs := &CopyOnWriteFs{base: roBase, layer: overlay}
+
+	base.MkdirAll("/", 0777)
+	overlay.MkdirAll("/home/test/foo/bar", 0777)
+	fh, _ := overlay.Create("/home/test/file.txt")
+	fh.WriteString("This is a test")
+	fh.Close()
+	fh, _ = overlay.Create("/home/test/foo/file2.txt")
+	fh.WriteString("This is a test")
+	fh.Close()
+	fh, _ = overlay.Create("/home/test/foo/bar/file3.txt")
+	fh.WriteString("This is a test")
+	fh.Close()
+
+	// Opening nested dir only in the overlay
+	fh, _ = ufs.Open("/home/test/foo")
+	list, err := fh.Readdir(-1)
+	if err != nil {
+		t.Errorf("Readdir failed", err)
+	}
+	if len(list) != 2 {
+		for _, x := range list {
+			fmt.Println(x.Name())
+		}
+		t.Errorf("Got wrong number of files in union: %v", len(list))
 	}
-	tempDirs = []string{}
 }
 
 func TestCopyOnWriteFsWithOsFs(t *testing.T) {
-	defer CleanupTempDirs()
+	defer CleanupTempDirs(t)
 	base := NewTempOsBaseFs(t)
 	roBase := &ReadOnlyFs{source: base}
 	overlay := NewTempOsBaseFs(t)

copyOnWriteFs.go

@@ -26,6 +26,8 @@ func NewCopyOnWriteFs(base Fs, layer Fs) Fs {
 	return &CopyOnWriteFs{base: base, layer: layer}
 }
 
+// isBaseFile Returns true if the given file is only found in the base layer
+// will return true if file is not found in either layer
 func (u *CopyOnWriteFs) isBaseFile(name string) (bool, error) {
 	if _, err := u.layer.Stat(name); err == nil {
 		return false, nil
@@ -144,7 +146,9 @@ func (u *CopyOnWriteFs) Open(name string) (File, error) {
 	if err != nil {
 		return nil, err
 	}
+
 	if b {
+		// If it's only in the base (not overlay) return that File
 		return u.base.Open(name)
 	}
 
@@ -153,6 +157,7 @@ func (u *CopyOnWriteFs) Open(name string) (File, error) {
 		return nil, err
 	}
 	if !dir {
+		// If it's in the overlay and not a directory, return that file
 		return u.layer.Open(name)
 	}
 
@@ -161,6 +166,7 @@ func (u *CopyOnWriteFs) Open(name string) (File, error) {
 	if err != nil && bfile == nil {
 		return nil, err
 	}
+	// If it's a directory in both, return a unionFile
 	return &UnionFile{base: bfile, layer: lfile}, nil
 }
 

os.go

@@ -32,7 +32,11 @@ func NewOsFs() Fs {
 func (OsFs) Name() string { return "OsFs" }
 
 func (OsFs) Create(name string) (File, error) {
-	return os.Create(name)
+	f, e := os.Create(name)
+	if f == nil {
+		return nil, e
+	}
+	return f, e
 }
 
 func (OsFs) Mkdir(name string, perm os.FileMode) error {
@@ -44,11 +48,20 @@ func (OsFs) MkdirAll(path string, perm os.FileMode) error {
 }
 
 func (OsFs) Open(name string) (File, error) {
-	return os.Open(name)
+	f, e := os.Open(name)
+
+	if f == nil {
+		return nil, e
+	}
+	return f, e
 }
 
 func (OsFs) OpenFile(name string, flag int, perm os.FileMode) (File, error) {
-	return os.OpenFile(name, flag, perm)
+	f, e := os.OpenFile(name, flag, perm)
+	if f == nil {
+		return nil, e
+	}
+	return f, e
 }
 
 func (OsFs) Remove(name string) error {

unionFile.go

@@ -123,6 +123,8 @@ func (f *UnionFile) Name() string {
 	return f.base.Name()
 }
 
+// Readdir will weave the two directories together and
+// return a single view of the overlayed directories
 func (f *UnionFile) Readdir(c int) (ofi []os.FileInfo, err error) {
 	if f.off == 0 {
 		var files = make(map[string]os.FileInfo)
@@ -136,6 +138,7 @@ func (f *UnionFile) Readdir(c int) (ofi []os.FileInfo, err error) {
 				files[fi.Name()] = fi
 			}
 		}
+
 		if f.base != nil {
 			rfi, err = f.base.Readdir(-1)
 			if err != nil {