cmd/go: reject update of VCS inside VCS

This can only lead to confusion.

Change-Id: Iae84c6404ab5eeb6950faa2364f97a017c67c506
Reviewed-on: https://go-review.googlesource.com/68110
Run-TryBot: Russ Cox <rsc@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Simon Rawet <Simon@rawet.se>

diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go
index 4460dca156989a5a17160ffaa7102cd5faabc36e..625b4eea9a024203f2818be5aef7db98d7ece705 100644 (file)
--- a/src/cmd/go/go_test.go
+++ b/src/cmd/go/go_test.go
@@ -1398,6 +1398,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/internal/get/get.go b/src/cmd/go/internal/get/get.go
index e1c90181fe3e82ccc322031b955ab696c4f99890..d62adffb46d70113f514bd79fdaf2c9e81bc9af7 100644 (file)
--- a/src/cmd/go/internal/get/get.go
+++ b/src/cmd/go/internal/get/get.go
@@ -439,6 +439,11 @@ func downloadPackage(p *load.Package) error {
                p.Internal.Build.PkgRoot = filepath.Join(list[0], "pkg")
        }
        root := filepath.Join(p.Internal.Build.SrcRoot, filepath.FromSlash(rootPath))
+
+       if err := checkNestedVCS(vcs, root, p.Internal.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/internal/get/vcs.go b/src/cmd/go/internal/get/vcs.go
index 91aad9a3a77d75bd007fadac32268b13f7c3930f..6b32bb31778b6fffe9fefaa5252baec7862994d3 100644 (file)
--- a/src/cmd/go/internal/get/vcs.go
+++ b/src/cmd/go/internal/get/vcs.go
@@ -548,11 +548,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)
                        }
                }
 
@@ -565,9 +582,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 {