From ec71ee078fd3243b78c0d404c8634bd97e38d7eb Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Fri, 22 Sep 2017 12:17:21 -0400 Subject: [PATCH] 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 TryBot-Result: Gobot Gobot Reviewed-by: David Crawshaw Reviewed-by: Ian Lance Taylor Reviewed-by: Simon Rawet --- src/cmd/go/go_test.go | 19 +++++++++++ src/cmd/go/internal/get/get.go | 5 +++ src/cmd/go/internal/get/vcs.go | 58 +++++++++++++++++++++++++++++++++- 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/src/cmd/go/go_test.go b/src/cmd/go/go_test.go index 4460dca156..625b4eea9a 100644 --- 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 e1c90181fe..d62adffb46 100644 --- 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 91aad9a3a7..6b32bb3177 100644 --- 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 { -- 2.48.1