improve cleanup implementation, add more tests

Signed-off-by: Santiago M. Mola <santi@mola.io>

1 files changed, 61 insertions, 49 deletions

diff --git a/repository.go b/repository.go
index bb89b28..9420af9 100644
--- a/repository.go
+++ b/repository.go
@@ -51,7 +51,6 @@ var (
 	ErrIsBareRepository          = errors.New("worktree not available in a bare repository")
 	ErrUnableToResolveCommit     = errors.New("unable to resolve commit")
 	ErrPackedObjectsNotSupported = errors.New("Packed objects not supported")
-	ErrDirNotEmpty               = errors.New("directory is not empty")
 )
 
 // Repository represents a git repository
@@ -344,36 +343,11 @@ func PlainClone(path string, isBare bool, o *CloneOptions) (*Repository, error)
 //
 // TODO(mcuadros): move isBare to CloneOptions in v5
 func PlainCloneContext(ctx context.Context, path string, isBare bool, o *CloneOptions) (*Repository, error) {
-	dirEmpty := false
-	dirExist := false
-
-	file, err := os.Stat(path)
+	dirExists, err := checkExistsAndIsEmptyDir(path)
 	if err != nil {
 		return nil, err
 	}
 
-	if !os.IsNotExist(err) {
-		dirExist = file.IsDir()
-	}
-
-	if dirExist {
-		fh, err := os.Open(path)
-		if err != nil {
-			return nil, err
-		}
-		defer fh.Close()
-
-		names, err := fh.Readdirnames(1)
-		if err != io.EOF && err != nil {
-			return nil, err
-		}
-		if len(names) == 0 {
-			dirEmpty = true
-		} else {
-			return nil, ErrDirNotEmpty
-		}
-	}
-
 	r, err := PlainInit(path, isBare)
 	if err != nil {
 		return nil, err
@@ -381,28 +355,7 @@ func PlainCloneContext(ctx context.Context, path string, isBare bool, o *CloneOp
 
 	err = r.clone(ctx, o)
 	if err != nil && err != ErrRepositoryAlreadyExists {
-		if dirEmpty {
-			fh, err := os.Open(path)
-			if err != nil {
-				return nil, err
-			}
-			defer fh.Close()
-
-			names, err := fh.Readdirnames(-1)
-			if err != nil && err != io.EOF {
-				return nil, err
-			}
-
-			for _, name := range names {
-				err = os.RemoveAll(filepath.Join(path, name))
-				if err != nil {
-					return nil, err
-				}
-			}
-		} else if !dirExist {
-			os.RemoveAll(path)
-			return nil, err
-		}
+		cleanUpDir(path, !dirExists)
 	}
 
 	return r, err
@@ -416,6 +369,65 @@ func newRepository(s storage.Storer, worktree billy.Filesystem) *Repository {
 	}
 }
 
+func checkExistsAndIsEmptyDir(path string) (exists bool, err error) {
+	fi, err := os.Stat(path)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return false, nil
+		}
+
+		return false, err
+	}
+
+	if !fi.IsDir() {
+		return false, fmt.Errorf("path is not a directory: %s", path)
+	}
+
+	f, err := os.Open(path)
+	if err != nil {
+		return false, err
+	}
+
+	defer ioutil.CheckClose(f, &err)
+
+	_, err = f.Readdirnames(1)
+	if err == io.EOF {
+		return true, nil
+	}
+
+	if err != nil {
+		return true, err
+	}
+
+	return true, fmt.Errorf("directory is not empty: %s", path)
+}
+
+func cleanUpDir(path string, all bool) error {
+	if all {
+		return os.RemoveAll(path)
+	}
+
+	f, err := os.Open(path)
+	if err != nil {
+		return err
+	}
+
+	defer ioutil.CheckClose(f, &err)
+
+	names, err := f.Readdirnames(-1)
+	if err != nil {
+		return err
+	}
+
+	for _, name := range names {
+		if err := os.RemoveAll(filepath.Join(path, name)); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 // Config return the repository config
 func (r *Repository) Config() (*config.Config, error) {
 	return r.Storer.Config()