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

Commit 58dc7c0

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 20c25df commit 58dc7c0

File tree

2 files changed

+23
-48
lines changed

2 files changed

+23
-48
lines changed

‎repository.go

Lines changed: 17 additions & 42 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/v5"
1919
"github.com/go-git/go-billy/v5/osfs"
20-
"github.com/go-git/go-billy/v5/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
@@ -491,21 +491,26 @@ func dotGitCommonDirectory(fs billy.Filesystem) (commonDir billy.Filesystem, err
491491

492492
// PlainClone a repository into the path with the given options, isBare defines
493493
// if the new repository will be bare or normal. If the path is not empty
494-
// ErrRepositoryAlreadyExists is returned.
494+
// ErrCloneDirNotEmpty is returned.
495495
func PlainClone(path string, o *CloneOptions) (*Repository, error) {
496496
return PlainCloneContext(context.Background(), path, o)
497497
}
498498

499499
// PlainCloneContext a repository into the path with the given options, isBare
500500
// defines if the new repository will be bare or normal. If the path is not empty
501-
// ErrRepositoryAlreadyExists is returned.
501+
// ErrCloneDirNotEmpty is returned.
502502
//
503503
// The provided Context must be non-nil. If the context expires before the
504504
// operation is complete, an error is returned. The context only affects the
505505
// transport operations.
506-
//
507-
// TODO(smola): refuse upfront to clone on a non-empty directory in v5, see #1027
508506
func PlainCloneContext(ctx context.Context, path string, o *CloneOptions) (*Repository, error) {
507+
empty, err := checkCloneDirIsEmpty(path)
508+
if err != nil {
509+
return nil, err
510+
}
511+
if !empty {
512+
return nil, fmt.Errorf("%w %s", ErrCloneDirNotEmpty, path)
513+
}
509514
start := time.Now()
510515
defer func() {
511516
url := ""
@@ -515,11 +520,6 @@ func PlainCloneContext(ctx context.Context, path string, o *CloneOptions) (*Repo
515520
trace.Performance.Printf("performance: %.9f s: git command: git clone %s", time.Since(start).Seconds(), url)
516521
}()
517522

518-
cleanup, cleanupParent, err := checkIfCleanupIsNeeded(path)
519-
if err != nil {
520-
return nil, err
521-
}
522-
523523
isBare := o.Bare
524524
if o.Mirror {
525525
isBare = true
@@ -530,12 +530,6 @@ func PlainCloneContext(ctx context.Context, path string, o *CloneOptions) (*Repo
530530
}
531531

532532
err = r.clone(ctx, o)
533-
if err != nil && err != ErrRepositoryAlreadyExists {
534-
if cleanup {
535-
_ = cleanUpDir(path, cleanupParent)
536-
}
537-
}
538-
539533
return r, err
540534
}
541535

@@ -547,49 +541,30 @@ func newRepository(s storage.Storer, worktree billy.Filesystem) *Repository {
547541
}
548542
}
549543

550-
func checkIfCleanupIsNeeded(path string) (cleanup bool, cleanParent bool, err error) {
544+
func checkCloneDirIsEmpty(path string) (empty bool, err error) {
551545
fi, err := osfs.Default.Stat(path)
552546
if err != nil {
553547
if os.IsNotExist(err) {
554-
return true, true, nil
548+
return true, nil
555549
}
556550

557-
return false, false, err
551+
return false, err
558552
}
559553

560554
if !fi.IsDir() {
561-
return false, false, fmt.Errorf("path is not a directory: %s", path)
555+
return false, fmt.Errorf("path is not a directory: %s", path)
562556
}
563557

564558
files, err := osfs.Default.ReadDir(path)
565559
if err != nil {
566-
return false, false, err
560+
return false, err
567561
}
568562

569563
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-
}
564+
return true, nil
590565
}
591566

592-
return err
567+
return false, nil
593568
}
594569

595570
// 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
}
83C7 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