8000 git: repository, add empty dir check to clone · go-git/go-git@2db07ff · GitHub
[go: up one dir, main page]
Skip to content

Commit 2db07ff

Browse files
CLBRITTON2CLBRITTON2
committed
git: repository, add empty dir check to clone
add check to return early from clone if the destination dir is not empty, remove cleanup logic that removed files from errored clones - git doesn't have this logic, update repository_test to test against new error type
1 parent 5d6af40 commit 2db07ff

File tree

2 files changed

+23
-49
lines changed

2 files changed

+23
-49
lines changed

‎repository.go

Lines changed: 17 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717
"github.com/ProtonMail/go-crypto/openpgp"
1818
"github.com/go-git/go-billy/v6"
1919
"github.com/go-git/go-billy/v6/osfs"
20-
"github.com/go-git/go-billy/v6/util"
2120
"github.com/go-git/go-git/v6/config"
2221
"github.com/go-git/go-git/v6/internal/path_util"
2322
"github.com/go-git/go-git/v6/internal/revision"
@@ -65,6 +64,7 @@ var (
6564
ErrAlternatePathNotSupported = errors.New("alternate path must use the file scheme")
6665
ErrUnsupportedMergeStrategy = errors.New("unsupported merge strategy")
6766
ErrFastForwardMergeNotPossible = errors.New("not possible to fast-forward merge changes")
67+
ErrCloneDirNotEmpty = errors.New("clone destination path already exists and is not empty")
6868
)
6969

7070
// Repository represents a git repository
@@ -306,7 +306,6 @@ func PlainInit(path string, isBare bool, options ...InitOption) (*Repository, er
306306
}
307307
s := filesystem.NewStorage(dot, cache.NewObjectLRUDefault())
308308
r, err := initFn(s)
309-
310309
if err != nil {
311310
return nil, err
312311
}
@@ -491,21 +490,26 @@ func dotGitCommonDirectory(fs billy.Filesystem) (commonDir billy.Filesystem, err
491490

492491
// PlainClone a repository into the path with the given options, isBare defines
493492
// if the new repository will be bare or normal. If the path is not empty
494-
// ErrRepositoryAlreadyExists is returned.
493+
// ErrCloneDirNotEmpty is returned.
495494
func PlainClone(path string, o *CloneOptions) (*Repository, error) {
496495
return PlainCloneContext(context.Background(), path, o)
497496
}
498497

499498
// PlainCloneContext a repository into the path with the given options, isBare
500499
// defines if the new repository will be bare or normal. If the path is not empty
501-
// ErrRepositoryAlreadyExists is returned.
500+
// ErrCloneDirNotEmpty is returned.
502501
//
503502
// The provided Context must be non-nil. If the context expires before the
504503
// operation is complete, an error is returned. The context only affects the
505504
// transport operations.
506-
//
507-
// TODO(smola): refuse upfront to clone on a non-empty directory in v5, see #1027
508505
func PlainCloneContext(ctx context.Context, path string, o *CloneOptions) (*Repository, error) {
506+
empty, err := checkCloneDirIsEmpty(path)
507+
if err != nil {
508+
return nil, err
509+
}
510+
if !empty {
511+
return nil, fmt.Errorf("%w %s", ErrCloneDirNotEmpty, path)
512+
}
509513
start := time.Now()
510514
defer func() {
511515
url := ""
@@ -515,11 +519,6 @@ func PlainCloneContext(ctx context.Context, path string, o *CloneOptions) (*Repo
515519
trace.Performance.Printf("performance: %.9f s: git command: git clone %s", time.Since(start).Seconds(), url)
516520
}()
517521

518-
cleanup, cleanupParent, err := checkIfCleanupIsNeeded(path)
519-
if err != nil {
520-
return nil, err
521-
}
522-
523522
isBare := o.Bare
524523
if o.Mirror {
525524
isBare = true
@@ -530,12 +529,6 @@ func PlainCloneContext(ctx context.Context, path string, o *CloneOptions) (*Repo
530529
}
531530

532531
err = r.clone(ctx, o)
533-
if err != nil && err != ErrRepositoryAlreadyExists {
534-
if cleanup {
535-
_ = cleanUpDir(path, cleanupParent)
536-
}
537-
}
538-
539532
return r, err
540533
}
541534

@@ -547,49 +540,30 @@ func newRepository(s storage.Storer, worktree billy.Filesystem) *Repository {
547540
}
548541
}
549542

550-
func checkIfCleanupIsNeeded(path string) (cleanup bool, cleanParent bool, err error) {
543+
func checkCloneDirIsEmpty(path string) (empty bool, err error) {
551544
fi, err := osfs.Default.Stat(path)
552545
if err != nil {
553546
if os.IsNotExist(err) {
554-
return true, true, nil
547+
return true, nil
555548
}
556549

557-
return false, false, err
550+
return false, err
558551
}
559552

560553
if !fi.IsDir() {
561-
return false, false, fmt.Errorf("path is not a directory: %s", path)
554+
return false, fmt.Errorf("path is not a directory: %s", path)
562555
}
563556

564557
files, err := osfs.Default.ReadDir(path)
565558
if err != nil {
566-
return false, false, err
559+
return false, err
567560
}
568561

569562
if len(files) == 0 {
570-
return true, false, nil
571-
}
572-
573-
return false, false, nil
574-
}
575-
576-
func cleanUpDir(path string, all bool) error {
577-
if all {
578-
return util.RemoveAll(osfs.Default, path)
579-
}
580-
581-
files, err := osfs.Default.ReadDir(path)
582-
if err != nil {
583-
return err
584-
}
585-
586-
for _, fi := range files {
587-
if err := util.RemoveAll(osfs.Default, osfs.Default.Join(path, fi.Name())); err != nil {
588-
return err
589-
}
563+
return true, nil
590564
}
591565

592-
return err
566+
return false, nil
593567
}
594568

595569
// Config return the repository config. In a filesystem backed repository this

‎repository_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1044,7 +1044,7 @@ func (s *RepositorySuite) TestPlainCloneOverExistingGitDirectory() {
10441044
URL: s.GetBasicLocalRepositoryURL(),
10451045
})
10461046
s.Nil(r)
1047-
s.ErrorIs(err, ErrRepositoryAlreadyExists)
1047+
s.ErrorIs(err, ErrCloneDirNotEmpty)
10481048
}
10491049

10501050
func (s *RepositorySuite) TestPlainCloneContextCancel() {
@@ -1152,8 +1152,8 @@ func (s *RepositorySuite) TestPlainCloneContextNonExistentWithNotEmptyDir() {
11521152
r, err := PlainCloneContext(ctx, fs.Join(fs.Root(), repoDir), &CloneOptions{
11531153
URL: "incorrectOnPurpose",
11541154
})
1155-
s.NotNil(r)
1156-
s.ErrorIs(err, transport.ErrRepositoryNotFound)
1155+
s.Nil(r)
1156+
s.ErrorIs(err, ErrCloneDirNotEmpty)
11571157

11581158
_, err = fs.Stat(dummyFile)
11591159
s.NoError(err)
@@ -1174,7 +1174,7 @@ func (s *RepositorySuite) TestPlainCloneContextNonExistingOverExistingGitDirecto
11741174
URL: "incorrectOnPurpose",
11751175
})
11761176
s.Nil(r)
1177-
s.ErrorIs(err, ErrRepositoryAlreadyExists)
1177+
s.ErrorIs(err, ErrCloneDirNotEmpty)
11781178
}
11791179

11801180
func (s *RepositorySuite) TestPlainCloneWithRecurseSubmodules() {
@@ -1301,8 +1301,8 @@ func (s *RepositorySuite) TestFetchWithFilters() {
13011301
Filter: packp.FilterBlobNone(),
13021302
})
13031303
s.ErrorIs(err, transport.ErrFilterNotSupported)
1304-
13051304
}
1305+
13061306
func (s *RepositorySuite) TestFetchWithFiltersReal() {
13071307
r, _ := Init(memory.NewStorage())
13081308
_, err := r.CreateRemote(&config.RemoteConfig{
@@ -1317,8 +1317,8 @@ func (s *RepositorySuite) TestFetchWithFiltersReal() {
13171317
blob, err := r.BlobObject(plumbing.NewHash("9a48f23120e880dfbe41f7c9b7b708e9ee62a492"))
13181318
s.NotNil(err)
13191319
s.Nil(blob)
1320-
13211320
}
1321+
13221322
func (s *RepositorySuite) TestCloneWithProgress() {
13231323
s.T().Skip("Currently, go-git server-side implementation does not support writing" +
13241324
"progress and sideband messages to the client. This means any tests that" +

0 commit comments

Comments
 (0)
0