From a4544a0f8af001d1fb6df0e70750f570ec49ccf9 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 22 Sep 2017 12:17:21 -0400 Subject: [release-branch.go1.8] cmd/go: reject update of VCS inside VCS Cherry-pick of CL 68110. Change-Id: Iae84c6404ab5eeb6950faa2364f97a017c67c506 Reviewed-on: https://go-review.googlesource.com/68190 Run-TryBot: Russ Cox Reviewed-by: Chris Broadfoot --- src/cmd/go/get.go | 5 +++++ src/cmd/go/go_test.go | 19 +++++++++++++++++ src/cmd/go/vcs.go | 58 ++++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/src/cmd/go/get.go b/src/cmd/go/get.go index 6fb4235068..14da0bdb0b 100644 --- a/src/cmd/go/get.go +++ b/src/cmd/go/get.go @@ -439,6 +439,11 @@ func downloadPackage(p *Package) error { p.build.PkgRoot = filepath.Join(list[0], "pkg") } root := filepath.Join(p.build.SrcRoot, filepath.FromSlash(rootPath)) + + if err := checkNestedVCS(vcs, root, p.build.SrcRoot); err != nil { + return err + } + // If we've considered this repository already, don't do it again. if downloadRootCache[root] { return nil diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index fa78578d12..00fd746e2e 100644 --- a/src/cmd/go/go_test.go +++ b/src/cmd/go/go_test.go @@ -1273,6 +1273,25 @@ func TestGetGitDefaultBranch(t *testing.T) { tg.grepStdout(`\* another-branch`, "not on correct default branch") } +func TestAccidentalGitCheckout(t *testing.T) { + testenv.MustHaveExternalNetwork(t) + if _, err := exec.LookPath("git"); err != nil { + t.Skip("skipping because git binary not found") + } + + tg := testgo(t) + defer tg.cleanup() + tg.parallel() + tg.tempDir("src") + tg.setenv("GOPATH", tg.path(".")) + + tg.runFail("get", "-u", "vcs-test.golang.org/go/test1-svn-git") + tg.grepStderr("src[\\\\/]vcs-test.* uses git, but parent .*src[\\\\/]vcs-test.* uses svn", "get did not fail for right reason") + + tg.runFail("get", "-u", "vcs-test.golang.org/go/test2-svn-git/test2main") + tg.grepStderr("src[\\\\/]vcs-test.* uses git, but parent .*src[\\\\/]vcs-test.* uses svn", "get did not fail for right reason") +} + func TestErrorMessageForSyntaxErrorInTestGoFileSaysFAIL(t *testing.T) { tg := testgo(t) defer tg.cleanup() diff --git a/src/cmd/go/vcs.go b/src/cmd/go/vcs.go index fcdce220a7..2b9f52f7ec 100644 --- a/src/cmd/go/vcs.go +++ b/src/cmd/go/vcs.go @@ -497,11 +497,28 @@ func vcsFromDir(dir, srcRoot string) (vcs *vcsCmd, root string, err error) { return nil, "", fmt.Errorf("directory %q is outside source root %q", dir, srcRoot) } + var vcsRet *vcsCmd + var rootRet string + origDir := dir for len(dir) > len(srcRoot) { for _, vcs := range vcsList { if _, err := os.Stat(filepath.Join(dir, "."+vcs.cmd)); err == nil { - return vcs, filepath.ToSlash(dir[len(srcRoot)+1:]), nil + root := filepath.ToSlash(dir[len(srcRoot)+1:]) + // Record first VCS we find, but keep looking, + // to detect mistakes like one kind of VCS inside another. + if vcsRet == nil { + vcsRet = vcs + rootRet = root + continue + } + // Allow .git inside .git, which can arise due to submodules. + if vcsRet == vcs && vcs.cmd == "git" { + continue + } + // Otherwise, we have one VCS inside a different VCS. + return nil, "", fmt.Errorf("directory %q uses %s, but parent %q uses %s", + filepath.Join(srcRoot, rootRet), vcsRet.cmd, filepath.Join(srcRoot, root), vcs.cmd) } } @@ -514,9 +531,48 @@ func vcsFromDir(dir, srcRoot string) (vcs *vcsCmd, root string, err error) { dir = ndir } + if vcsRet != nil { + return vcsRet, rootRet, nil + } + return nil, "", fmt.Errorf("directory %q is not using a known version control system", origDir) } +// checkNestedVCS checks for an incorrectly-nested VCS-inside-VCS +// situation for dir, checking parents up until srcRoot. +func checkNestedVCS(vcs *vcsCmd, dir, srcRoot string) error { + if len(dir) <= len(srcRoot) || dir[len(srcRoot)] != filepath.Separator { + return fmt.Errorf("directory %q is outside source root %q", dir, srcRoot) + } + + otherDir := dir + for len(otherDir) > len(srcRoot) { + for _, otherVCS := range vcsList { + if _, err := os.Stat(filepath.Join(dir, "."+otherVCS.cmd)); err == nil { + // Allow expected vcs in original dir. + if otherDir == dir && otherVCS == vcs { + continue + } + // Allow .git inside .git, which can arise due to submodules. + if otherVCS == vcs && vcs.cmd == "git" { + continue + } + // Otherwise, we have one VCS inside a different VCS. + return fmt.Errorf("directory %q uses %s, but parent %q uses %s", dir, vcs.cmd, otherDir, otherVCS.cmd) + } + } + // Move to parent. + newDir := filepath.Dir(otherDir) + if len(newDir) >= len(otherDir) { + // Shouldn't happen, but just in case, stop. + break + } + otherDir = newDir + } + + return nil +} + // repoRoot represents a version control system, a repo, and a root of // where to put it on disk. type repoRoot struct { -- cgit v1.2.3-54-g00ecf