diff --git a/internal/gps/_testdata/badrepo/README.md b/internal/gps/_testdata/badrepo/README.md new file mode 100644 index 0000000000..14232159c3 --- /dev/null +++ b/internal/gps/_testdata/badrepo/README.md @@ -0,0 +1,5 @@ +### Test Data + +This directory contains artifacts that represent malformed repo archives. Its purpose is to ensure `dep` can recover from such corrupted repositories in specific test scenarios. + +- `corrupt_dot_git_directory.tar`: is a repo with a corrupt `.git` directory. Dep can put a directory in such malformed state when a user hits `Ctrl+C` in the middle of a `dep init` process or others. `TestNewCtxRepoRecovery` uses this file to ensure recovery. diff --git a/internal/gps/_testdata/badrepo/corrupt_dot_git_directory.tar b/internal/gps/_testdata/badrepo/corrupt_dot_git_directory.tar new file mode 100644 index 0000000000..7a84731839 Binary files /dev/null and b/internal/gps/_testdata/badrepo/corrupt_dot_git_directory.tar differ diff --git a/internal/gps/maybe_source.go b/internal/gps/maybe_source.go index b892ae0280..db5da21255 100644 --- a/internal/gps/maybe_source.go +++ b/internal/gps/maybe_source.go @@ -84,14 +84,15 @@ func (m maybeGitSource) try(ctx context.Context, cachedir string, c singleSource ustr := m.url.String() path := filepath.Join(cachedir, "sources", sanitizer.Replace(ustr)) - r, err := vcs.NewGitRepo(ustr, path) + r, err := newCtxRepo(vcs.Git, ustr, path) + if err != nil { return nil, 0, unwrapVcsErr(err) } src := &gitSource{ baseVCSSource: baseVCSSource{ - repo: &gitRepo{r}, + repo: r, }, } @@ -139,8 +140,8 @@ func (m maybeGopkginSource) try(ctx context.Context, cachedir string, c singleSo // So, it's OK to just dumb-join the scheme with the path. path := filepath.Join(cachedir, "sources", sanitizer.Replace(m.url.Scheme+"/"+m.opath)) ustr := m.url.String() + r, err := newCtxRepo(vcs.Git, ustr, path) - r, err := vcs.NewGitRepo(ustr, path) if err != nil { return nil, 0, unwrapVcsErr(err) } @@ -148,7 +149,7 @@ func (m maybeGopkginSource) try(ctx context.Context, cachedir string, c singleSo src := &gopkginSource{ gitSource: gitSource{ baseVCSSource: baseVCSSource{ - repo: &gitRepo{r}, + repo: r, }, }, major: m.major, @@ -187,7 +188,8 @@ func (m maybeBzrSource) try(ctx context.Context, cachedir string, c singleSource ustr := m.url.String() path := filepath.Join(cachedir, "sources", sanitizer.Replace(ustr)) - r, err := vcs.NewBzrRepo(ustr, path) + r, err := newCtxRepo(vcs.Bzr, ustr, path) + if err != nil { return nil, 0, unwrapVcsErr(err) } @@ -209,7 +211,7 @@ func (m maybeBzrSource) try(ctx context.Context, cachedir string, c singleSource src := &bzrSource{ baseVCSSource: baseVCSSource{ - repo: &bzrRepo{r}, + repo: r, }, } @@ -228,7 +230,8 @@ func (m maybeHgSource) try(ctx context.Context, cachedir string, c singleSourceC ustr := m.url.String() path := filepath.Join(cachedir, "sources", sanitizer.Replace(ustr)) - r, err := vcs.NewHgRepo(ustr, path) + r, err := newCtxRepo(vcs.Hg, ustr, path) + if err != nil { return nil, 0, unwrapVcsErr(err) } @@ -250,7 +253,7 @@ func (m maybeHgSource) try(ctx context.Context, cachedir string, c singleSourceC src := &hgSource{ baseVCSSource: baseVCSSource{ - repo: &hgRepo{r}, + repo: r, }, } diff --git a/internal/gps/vcs_repo.go b/internal/gps/vcs_repo.go index 10a522ba84..550b4fd43a 100644 --- a/internal/gps/vcs_repo.go +++ b/internal/gps/vcs_repo.go @@ -24,6 +24,40 @@ type ctxRepo interface { //ping(context.Context) (bool, error) } +func newCtxRepo(s vcs.Type, ustr, path string) (r ctxRepo, err error) { + r, err = getVCSRepo(s, ustr, path) + if err != nil { + // if vcs could not initialize the repo due to a local error + // then the local repo is in an incorrect state. Remove and + // treat it as a new not-yet-cloned repo. + + // TODO(marwan-at-work): warn/give progress of the above comment. + os.RemoveAll(path) + r, err = getVCSRepo(s, ustr, path) + } + + return +} + +func getVCSRepo(s vcs.Type, ustr, path string) (r ctxRepo, err error) { + switch s { + case vcs.Git: + var repo *vcs.GitRepo + repo, err = vcs.NewGitRepo(ustr, path) + r = &gitRepo{repo} + case vcs.Bzr: + var repo *vcs.BzrRepo + repo, err = vcs.NewBzrRepo(ustr, path) + r = &bzrRepo{repo} + case vcs.Hg: + var repo *vcs.HgRepo + repo, err = vcs.NewHgRepo(ustr, path) + r = &hgRepo{repo} + } + + return +} + // original implementation of these methods come from // https://github.com/Masterminds/vcs diff --git a/internal/gps/vcs_repo_test.go b/internal/gps/vcs_repo_test.go index 136fba24c5..c5d6b8b557 100644 --- a/internal/gps/vcs_repo_test.go +++ b/internal/gps/vcs_repo_test.go @@ -5,10 +5,14 @@ package gps import ( + "archive/tar" + "compress/gzip" "context" "errors" + "io" "io/ioutil" "os" + "path/filepath" "testing" "time" @@ -18,6 +22,8 @@ import ( // original implementation of these test files come from // https://github.com/Masterminds/vcs test files +const gitRemoteTestRepo = "https://github.com/Masterminds/VCSTestRepo" + func TestErrs(t *testing.T) { err := newVcsLocalErrorOr("", context.Canceled, "") if err != context.Canceled { @@ -46,6 +52,73 @@ func TestErrs(t *testing.T) { } } +func TestNewCtxRepoHappyPath(t *testing.T) { + t.Parallel() + + tempDir, err := ioutil.TempDir("", "go-ctx-repo-happy-test") + if err != nil { + t.Fatal(err) + } + defer func() { + err = os.RemoveAll(tempDir) + if err != nil { + t.Error(err) + } + }() + + _, err = newCtxRepo(vcs.Git, gitRemoteTestRepo, tempDir) + if err != nil { + t.Fatal(err) + } +} + +func TestNewCtxRepoRecovery(t *testing.T) { + t.Parallel() + + tempDir, err := ioutil.TempDir("", "go-ctx-repo-recovery-test") + if err != nil { + t.Fatal(err) + } + defer func() { + err = os.RemoveAll(tempDir) + if err != nil { + t.Error(err) + } + }() + + cwd, err := os.Getwd() + if err != nil { + t.Fatal(err) + } + + src := filepath.Join(cwd, "_testdata", "badrepo", "corrupt_dot_git_directory.tar") + f, err := os.Open(src) + if err != nil { + t.Fatal(err) + } + defer f.Close() + + dest := filepath.Join(tempDir, ".git") + err = untar(dest, f) + if err != nil { + t.Fatalf("could not untar corrupt repo into temp folder: %v\n", err) + } + + _, err = getVCSRepo(vcs.Git, gitRemoteTestRepo, tempDir) + if err != nil { + if _, ok := err.(*vcs.LocalError); !ok { + t.Fatalf("expected a local error but got: %v\n", err) + } + } else { + t.Fatal("expected getVCSRepo to fail when pointing to a corrupt local path. It is possible that vcs.GitNewRepo updated to gracefully handle this test scenario. Check the return of vcs.GitNewRepo.") + } + + _, err = newCtxRepo(vcs.Git, gitRemoteTestRepo, tempDir) + if err != nil { + t.Fatal(err) + } +} + func testSvnRepo(t *testing.T) { t.Parallel() @@ -219,7 +292,7 @@ func testGitRepo(t *testing.T) { } }() - rep, err := vcs.NewGitRepo("https://github.com/Masterminds/VCSTestRepo", tempDir+"/VCSTestRepo") + rep, err := vcs.NewGitRepo(gitRemoteTestRepo, tempDir+"/VCSTestRepo") if err != nil { t.Fatal(err) } @@ -344,3 +417,46 @@ func testBzrRepo(t *testing.T) { t.Fatalf("Current failed to detect Bzr on rev 2 of branch. Got version: %s", v) } } + +func untar(dst string, r io.Reader) error { + gzr, err := gzip.NewReader(r) + if err != nil { + return err + } + defer gzr.Close() + + tr := tar.NewReader(gzr) + + for { + header, err := tr.Next() + + switch { + case err == io.EOF: + return nil + case err != nil: + return err + case header == nil: + continue + } + + target := filepath.Join(dst, header.Name) + switch header.Typeflag { + case tar.TypeDir: + if _, err := os.Stat(target); err != nil { + if err := os.MkdirAll(target, 0755); err != nil { + return err + } + } + case tar.TypeReg: + f, err := os.OpenFile(target, os.O_CREATE|os.O_RDWR, os.FileMode(header.Mode)) + if err != nil { + return err + } + defer f.Close() + + if _, err := io.Copy(f, tr); err != nil { + return err + } + } + } +}