[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 <rsc@golang.org>
Reviewed-by: Chris Broadfoot <cbro@golang.org>

diff --git a/src/cmd/go/get.go b/src/cmd/go/get.go
index 6fb4235068ec944a94b49c8fe07a972e9824b004..14da0bdb0bd95b2c1cf1c05018b60ee503672a2e 100644 (file)
--- 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 fa78578d1215142efbe3a31ee451e12888340c57..00fd746e2e4a6a44d71fa7ab66bb2028f4cedecd 100644 (file)
--- 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 fcdce220a7c478af19e21616891f54ed91f5fb3d..2b9f52f7ec3d7ed27fca7ebf3740567f45e4cf97 100644 (file)
--- 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 {