Fixed a bug in memfs renaming.

fuseops/ops.go

@@ -360,7 +360,8 @@ func (o *CreateSymlinkOp) toBazilfuseResponse() (bfResp interface{}) {
 
 // TODO(jacobsa): Comments for struct and fields, in particular covering
 // renames across mount points. Mention that you'll still get a forget, like
-// RmDirOp. Also that an existing destination name should be atomically replaced.
+// RmDirOp. Also that an existing destination name should be atomically
+// replaced. Also that the new directory must be empty if it exists.
 type RenameOp struct {
 	commonOp
 

samples/memfs/inode.go

@@ -278,7 +278,7 @@ func (in *inode) RemoveChild(name string) {
 // Serve a ReadDir request.
 //
 // REQUIRES: in.isDir()
-func (in *inode) ReadDir(offset int, size int) (data []byte, err error) {
+func (in *inode) ReadDir(offset int, size int) (data []byte) {
 	if !in.isDir() {
 		panic("ReadDir called on non-directory.")
 	}

samples/memfs/memfs.go

@@ -410,14 +410,21 @@ func (fs *memFS) Rename(
 		return
 	}
 
-	// If the new name exists in the new parent, delete it first. Then link in
-	// the child.
+	// If the new name exists already in the new parent, make sure it's not a
+	// non-empty directory, then delete it.
 	newParent := fs.getInodeOrDie(op.NewParent)
-	_, _, ok = newParent.LookUpChild(op.NewName)
+	existingID, _, ok := newParent.LookUpChild(op.NewName)
 	if ok {
+		existing := fs.getInodeOrDie(existingID)
+		if existing.isDir() && len(existing.ReadDir(0, 1024)) > 0 {
+			err = fuse.ENOTEMPTY
+			return
+		}
+
 		newParent.RemoveChild(op.NewName)
 	}
 
+	// Link the new name.
 	newParent.AddChild(
 		childID,
 		op.NewName,
@@ -515,11 +522,7 @@ func (fs *memFS) ReadDir(
 	inode := fs.getInodeOrDie(op.Inode)
 
 	// Serve the request.
-	op.Data, err = inode.ReadDir(int(op.Offset), op.Size)
-	if err != nil {
-		err = fmt.Errorf("inode.ReadDir: %v", err)
-		return
-	}
+	op.Data = inode.ReadDir(int(op.Offset), op.Size)
 
 	return
 }

samples/memfs/memfs_test.go

@@ -1598,7 +1598,7 @@ func (t *MemFSTest) RenameOverExistingDirectory() {
 
 	// Renaming over the non-empty one shouldn't work.
 	err = os.Rename(newPath, oldPath)
-	ExpectThat(err, Error(HasSubstr("TODO")))
+	ExpectThat(err, Error(HasSubstr("not empty")))
 
 	// But the other way around should.
 	err = os.Rename(oldPath, newPath)