diff options
| -rw-r--r-- | repository.go | 25 | ||||
| -rw-r--r-- | repository_test.go | 51 |
2 files changed, 57 insertions, 19 deletions
diff --git a/repository.go b/repository.go index f548e9a..97134ec 100644 --- a/repository.go +++ b/repository.go @@ -342,8 +342,9 @@ func PlainClone(path string, isBare bool, o *CloneOptions) (*Repository, error) // transport operations. // // TODO(mcuadros): move isBare to CloneOptions in v5 +// TODO(smola): refuse upfront to clone on a non-empty directory in v5, see #1027 func PlainCloneContext(ctx context.Context, path string, isBare bool, o *CloneOptions) (*Repository, error) { - dirExists, err := checkExistsAndIsEmptyDir(path) + cleanup, cleanupParent, err := checkIfCleanupIsNeeded(path) if err != nil { return nil, err } @@ -355,7 +356,9 @@ func PlainCloneContext(ctx context.Context, path string, isBare bool, o *CloneOp err = r.clone(ctx, o) if err != nil && err != ErrRepositoryAlreadyExists { - cleanUpDir(path, !dirExists) + if cleanup { + cleanUpDir(path, cleanupParent) + } } return r, err @@ -369,37 +372,37 @@ func newRepository(s storage.Storer, worktree billy.Filesystem) *Repository { } } -func checkExistsAndIsEmptyDir(path string) (exists bool, err error) { +func checkIfCleanupIsNeeded(path string) (cleanup bool, cleanParent bool, err error) { fi, err := os.Stat(path) if err != nil { if os.IsNotExist(err) { - return false, nil + return true, true, nil } - return false, err + return false, false, err } if !fi.IsDir() { - return false, fmt.Errorf("path is not a directory: %s", path) + return false, false, fmt.Errorf("path is not a directory: %s", path) } f, err := os.Open(path) if err != nil { - return false, err + return false, false, err } defer ioutil.CheckClose(f, &err) _, err = f.Readdirnames(1) if err == io.EOF { - return true, nil + return true, false, nil } if err != nil { - return true, err + return false, false, err } - return true, fmt.Errorf("directory is not empty: %s", path) + return false, false, nil } func cleanUpDir(path string, all bool) error { @@ -425,7 +428,7 @@ func cleanUpDir(path string, all bool) error { } } - return nil + return err } // Config return the repository config diff --git a/repository_test.go b/repository_test.go index 59ab89e..70e344e 100644 --- a/repository_test.go +++ b/repository_test.go @@ -21,6 +21,7 @@ import ( "gopkg.in/src-d/go-git.v4/plumbing/cache" "gopkg.in/src-d/go-git.v4/plumbing/object" "gopkg.in/src-d/go-git.v4/plumbing/storer" + "gopkg.in/src-d/go-git.v4/plumbing/transport" "gopkg.in/src-d/go-git.v4/storage" "gopkg.in/src-d/go-git.v4/storage/filesystem" "gopkg.in/src-d/go-git.v4/storage/memory" @@ -177,11 +178,12 @@ func (s *RepositorySuite) TestCloneContext(c *C) { ctx, cancel := context.WithCancel(context.Background()) cancel() - _, err := CloneContext(ctx, memory.NewStorage(), nil, &CloneOptions{ + r, err := CloneContext(ctx, memory.NewStorage(), nil, &CloneOptions{ URL: s.GetBasicLocalRepositoryURL(), }) - c.Assert(err, NotNil) + c.Assert(r, NotNil) + c.Assert(err, ErrorMatches, ".* context canceled") } func (s *RepositorySuite) TestCloneWithTags(c *C) { @@ -581,7 +583,20 @@ func (s *RepositorySuite) TestPlainCloneWithRemoteName(c *C) { c.Assert(remote, NotNil) } -func (s *RepositorySuite) TestPlainCloneContextWithProperParameters(c *C) { +func (s *RepositorySuite) TestPlainCloneOverExistingGitDirectory(c *C) { + tmpDir := c.MkDir() + r, err := PlainInit(tmpDir, false) + c.Assert(r, NotNil) + c.Assert(err, IsNil) + + r, err = PlainClone(tmpDir, false, &CloneOptions{ + URL: s.GetBasicLocalRepositoryURL(), + }) + c.Assert(r, IsNil) + c.Assert(err, Equals, ErrRepositoryAlreadyExists) +} + +func (s *RepositorySuite) TestPlainCloneContextCancel(c *C) { ctx, cancel := context.WithCancel(context.Background()) cancel() @@ -590,7 +605,7 @@ func (s *RepositorySuite) TestPlainCloneContextWithProperParameters(c *C) { }) c.Assert(r, NotNil) - c.Assert(err, NotNil) + c.Assert(err, ErrorMatches, ".* context canceled") } func (s *RepositorySuite) TestPlainCloneContextNonExistentWithExistentDir(c *C) { @@ -604,7 +619,7 @@ func (s *RepositorySuite) TestPlainCloneContextNonExistentWithExistentDir(c *C) URL: "incorrectOnPurpose", }) c.Assert(r, NotNil) - c.Assert(err, NotNil) + c.Assert(err, Equals, transport.ErrRepositoryNotFound) _, err = os.Stat(repoDir) c.Assert(os.IsNotExist(err), Equals, false) @@ -625,7 +640,7 @@ func (s *RepositorySuite) TestPlainCloneContextNonExistentWithNonExistentDir(c * URL: "incorrectOnPurpose", }) c.Assert(r, NotNil) - c.Assert(err, NotNil) + c.Assert(err, Equals, transport.ErrRepositoryNotFound) _, err = os.Stat(repoDir) c.Assert(os.IsNotExist(err), Equals, true) @@ -645,7 +660,7 @@ func (s *RepositorySuite) TestPlainCloneContextNonExistentWithNotDir(c *C) { URL: "incorrectOnPurpose", }) c.Assert(r, IsNil) - c.Assert(err, NotNil) + c.Assert(err, ErrorMatches, ".*not a directory.*") fi, err := os.Stat(repoDir) c.Assert(err, IsNil) @@ -668,8 +683,28 @@ func (s *RepositorySuite) TestPlainCloneContextNonExistentWithNotEmptyDir(c *C) r, err := PlainCloneContext(ctx, repoDirPath, false, &CloneOptions{ URL: "incorrectOnPurpose", }) + c.Assert(r, NotNil) + c.Assert(err, Equals, transport.ErrRepositoryNotFound) + + _, err = os.Stat(dummyFile) + c.Assert(err, IsNil) + +} + +func (s *RepositorySuite) TestPlainCloneContextNonExistingOverExistingGitDirectory(c *C) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + tmpDir := c.MkDir() + r, err := PlainInit(tmpDir, false) + c.Assert(r, NotNil) + c.Assert(err, IsNil) + + r, err = PlainCloneContext(ctx, tmpDir, false, &CloneOptions{ + URL: "incorrectOnPurpose", + }) c.Assert(r, IsNil) - c.Assert(err, NotNil) + c.Assert(err, Equals, ErrRepositoryAlreadyExists) } func (s *RepositorySuite) TestPlainCloneWithRecurseSubmodules(c *C) { |