From 3bea90d84107889aaaaa0089f615d7070951a832 Mon Sep 17 00:00:00 2001 From: "Bryan C. Mills" Date: Wed, 13 Nov 2019 17:18:23 -0500 Subject: [PATCH] =?utf8?q?cmd/go:=20allow=20a=20fork=20with=20path=20[?= =?utf8?q?=E2=80=A6]/vN=20to=20replace=20gopkg.in/[=E2=80=A6].vN?= MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Fixes #34254 Change-Id: Ib4e476d31264342538c2cf381177823183cba890 Reviewed-on: https://go-review.googlesource.com/c/go/+/206761 Run-TryBot: Bryan C. Mills TryBot-Result: Gobot Gobot Reviewed-by: Jay Conrod --- src/cmd/go/internal/modfetch/coderepo.go | 49 +++++++++++++------ .../testdata/script/mod_replace_gopkgin.txt | 28 +++++++++++ 2 files changed, 62 insertions(+), 15 deletions(-) create mode 100644 src/cmd/go/testdata/script/mod_replace_gopkgin.txt diff --git a/src/cmd/go/internal/modfetch/coderepo.go b/src/cmd/go/internal/modfetch/coderepo.go index 03dd4b076d..849e8c7ca1 100644 --- a/src/cmd/go/internal/modfetch/coderepo.go +++ b/src/cmd/go/internal/modfetch/coderepo.go @@ -719,9 +719,6 @@ func (r *codeRepo) findDir(version string) (rev, dir string, gomod []byte, err e // because of replacement modules. This might be a fork of // the real module, found at a different path, usable only in // a replace directive. - // - // TODO(bcmills): This doesn't seem right. Investigate further. - // (Notably: why can't we replace foo/v2 with fork-of-foo/v3?) dir2 := path.Join(r.codeDir, r.pathMajor[1:]) file2 = path.Join(dir2, "go.mod") gomod2, err2 := r.code.ReadFile(rev, file2, codehost.MaxGoMod) @@ -747,11 +744,11 @@ func (r *codeRepo) findDir(version string) (rev, dir string, gomod []byte, err e // Not v2/go.mod, so it's either go.mod or nothing. Which is it? if found1 { - // Explicit go.mod with matching module path OK. + // Explicit go.mod with matching major version ok. return rev, r.codeDir, gomod1, nil } if err1 == nil { - // Explicit go.mod with non-matching module path disallowed. + // Explicit go.mod with non-matching major version disallowed. suffix := "" if file2 != "" { suffix = fmt.Sprintf(" (and ...%s/go.mod does not exist)", r.pathMajor) @@ -762,6 +759,9 @@ func (r *codeRepo) findDir(version string) (rev, dir string, gomod []byte, err e if r.pathMajor != "" { // ".v1", ".v2" for gopkg.in return "", "", nil, fmt.Errorf("%s has non-...%s module path %q%s at revision %s", file1, r.pathMajor, mpath1, suffix, rev) } + if _, _, ok := module.SplitPathVersion(mpath1); !ok { + return "", "", nil, fmt.Errorf("%s has malformed module path %q%s at revision %s", file1, mpath1, suffix, rev) + } return "", "", nil, fmt.Errorf("%s has post-%s module path %q%s at revision %s", file1, semver.Major(version), mpath1, suffix, rev) } @@ -778,24 +778,43 @@ func (r *codeRepo) findDir(version string) (rev, dir string, gomod []byte, err e return "", "", nil, fmt.Errorf("missing %s/go.mod at revision %s", r.pathPrefix, rev) } +// isMajor reports whether the versions allowed for mpath are compatible with +// the major version(s) implied by pathMajor, or false if mpath has an invalid +// version suffix. func isMajor(mpath, pathMajor string) bool { if mpath == "" { + // If we don't have a path, we don't know what version(s) it is compatible with. + return false + } + _, mpathMajor, ok := module.SplitPathVersion(mpath) + if !ok { + // An invalid module path is not compatible with any version. return false } if pathMajor == "" { - // mpath must NOT have version suffix. - i := len(mpath) - for i > 0 && '0' <= mpath[i-1] && mpath[i-1] <= '9' { - i-- - } - if i < len(mpath) && i >= 2 && mpath[i-1] == 'v' && mpath[i-2] == '/' { - // Found valid suffix. + // All of the valid versions for a gopkg.in module that requires major + // version v0 or v1 are compatible with the "v0 or v1" implied by an empty + // pathMajor. + switch module.PathMajorPrefix(mpathMajor) { + case "", "v0", "v1": + return true + default: return false } - return true } - // Otherwise pathMajor is ".v1", ".v2" (gopkg.in), or "/v2", "/v3" etc. - return strings.HasSuffix(mpath, pathMajor) + if mpathMajor == "" { + // Even if pathMajor is ".v0" or ".v1", we can't be sure that a module + // without a suffix is tagged appropriately. Besides, we don't expect clones + // of non-gopkg.in modules to have gopkg.in paths, so a non-empty, + // non-gopkg.in mpath is probably the wrong module for any such pathMajor + // anyway. + return false + } + // If both pathMajor and mpathMajor are non-empty, then we only care that they + // have the same major-version validation rules. A clone fetched via a /v2 + // path might replace a module with path gopkg.in/foo.v2-unstable, and that's + // ok. + return pathMajor[1:] == mpathMajor[1:] } func (r *codeRepo) GoMod(version string) (data []byte, err error) { diff --git a/src/cmd/go/testdata/script/mod_replace_gopkgin.txt b/src/cmd/go/testdata/script/mod_replace_gopkgin.txt new file mode 100644 index 0000000000..6608fb1b80 --- /dev/null +++ b/src/cmd/go/testdata/script/mod_replace_gopkgin.txt @@ -0,0 +1,28 @@ +# Regression test for golang.org/issue/34254: +# a clone of gopkg.in/[…].vN should be replaceable by +# a fork hosted at corp.example.com/[…]/vN, +# even if there is an explicit go.mod file containing the +# gopkg.in path. + +[short] skip +[!net] skip +[!exec:git] skip + +env GO111MODULE=on +env GOPROXY=direct +env GOSUMDB=off + +# Replacing gopkg.in/[…].vN with a repository with a root go.mod file +# specifying […].vN and a compatible version should succeed, even if +# the replacement path is not a gopkg.in path. +cd dot-to-dot +go list gopkg.in/src-d/go-git.v4 + +-- dot-to-dot/go.mod -- +module golang.org/issue/34254 + +go 1.13 + +require gopkg.in/src-d/go-git.v4 v4.13.1 + +replace gopkg.in/src-d/go-git.v4 v4.13.1 => github.com/src-d/go-git/v4 v4.13.1 -- 2.50.0