aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorSantiago M. Mola <santi@mola.io>2018-10-30 13:10:00 +0100
committerSantiago M. Mola <santi@mola.io>2018-10-30 13:10:00 +0100
commit3332e8d67dbe810d1607285d43fcd79470cf9961 (patch)
treed737ffe63ecd0059439517ee66a4168c0565807d
parent507681b9a317a91c176460b6aaff1915ba4b9533 (diff)
downloadgo-git-3332e8d67dbe810d1607285d43fcd79470cf9961.tar.gz
improve cleanup implementation, add more tests
Signed-off-by: Santiago M. Mola <santi@mola.io>
-rw-r--r--repository.go110
-rw-r--r--repository_test.go54
2 files changed, 109 insertions, 55 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()
diff --git a/repository_test.go b/repository_test.go
index 887901f..ba2cf1a 100644
--- a/repository_test.go
+++ b/repository_test.go
@@ -593,24 +593,66 @@ func (s *RepositorySuite) TestPlainCloneContextWithProperParameters(c *C) {
c.Assert(err, NotNil)
}
-func (s *RepositorySuite) TestPlainCloneContextWithIncorrectRepo(c *C) {
+func (s *RepositorySuite) TestPlainCloneContextNonExistentWithExistentDir(c *C) {
+ ctx, cancel := context.WithCancel(context.Background())
+ cancel()
+
+ tmpDir := c.MkDir()
+ repoDir := tmpDir
+
+ r, err := PlainCloneContext(ctx, repoDir, false, &CloneOptions{
+ URL: "incorrectOnPurpose",
+ })
+ c.Assert(r, NotNil)
+ c.Assert(err, NotNil)
+
+ _, err = os.Stat(repoDir)
+ c.Assert(os.IsNotExist(err), Equals, false)
+
+ names, err := ioutil.ReadDir(repoDir)
+ c.Assert(err, IsNil)
+ c.Assert(names, HasLen, 0)
+}
+
+func (s *RepositorySuite) TestPlainCloneContextNonExistentWithNonExistentDir(c *C) {
ctx, cancel := context.WithCancel(context.Background())
cancel()
tmpDir := c.MkDir()
repoDir := filepath.Join(tmpDir, "repoDir")
+
r, err := PlainCloneContext(ctx, repoDir, false, &CloneOptions{
URL: "incorrectOnPurpose",
})
- c.Assert(r, IsNil)
+ c.Assert(r, NotNil)
c.Assert(err, NotNil)
_, err = os.Stat(repoDir)
- dirNotExist := os.IsNotExist(err)
- c.Assert(dirNotExist, Equals, true)
+ c.Assert(os.IsNotExist(err), Equals, true)
+}
+
+func (s *RepositorySuite) TestPlainCloneContextNonExistentWithNotDir(c *C) {
+ ctx, cancel := context.WithCancel(context.Background())
+ cancel()
+
+ tmpDir := c.MkDir()
+ repoDir := filepath.Join(tmpDir, "repoDir")
+ f, err := os.Create(repoDir)
+ c.Assert(err, IsNil)
+ c.Assert(f.Close(), IsNil)
+
+ r, err := PlainCloneContext(ctx, repoDir, false, &CloneOptions{
+ URL: "incorrectOnPurpose",
+ })
+ c.Assert(r, IsNil)
+ c.Assert(err, NotNil)
+
+ fi, err := os.Stat(repoDir)
+ c.Assert(err, IsNil)
+ c.Assert(fi.IsDir(), Equals, false)
}
-func (s *RepositorySuite) TestPlainCloneContextWithNotEmptyDir(c *C) {
+func (s *RepositorySuite) TestPlainCloneContextNonExistentWithNotEmptyDir(c *C) {
ctx, cancel := context.WithCancel(context.Background())
cancel()
@@ -627,7 +669,7 @@ func (s *RepositorySuite) TestPlainCloneContextWithNotEmptyDir(c *C) {
URL: "incorrectOnPurpose",
})
c.Assert(r, IsNil)
- c.Assert(err, Equals, ErrDirNotEmpty)
+ c.Assert(err, NotNil)
}
func (s *RepositorySuite) TestPlainCloneWithRecurseSubmodules(c *C) {