cmd/go/internal/modfetch/codehost: fix pseudoversions for non-semver tags and tags...

Pseudoversion determination depends in part on the results from gitRepo.RecentTag, which currently invokes:

git describe --first-parent --always --abbrev=0 --match <prefix>v[0-9]*.[0-9]*.[0-9]* --tags <rev>

The comment at https://github.com/golang/go/issues/27171#issuecomment-470134255 describes some problems with the current approach.

One problem is Docker and other repos can have tags that are not valid semver tags but that still match a glob pattern of v[0-9]*.[0-9]*.[0-9]* which are found by 'git describe' but then rejected by cmd/go, and hence those repos currently can end up with v0.0.0 pseudoversions instead of finding a proper semver tag to use as input to building a pseudoversion  (when then causes problems when the v0.0.0 pseudoversion is fed into MVS). An example problematic tag is a date-based tag such as 'v18.06.16', which matches the glob pattern, but is not a valid semver tag (due to the leading 0 in '06').

Issues #31673, #31287, and #27171 also describe problems where the '--first-parent' argument to 'git describe' cause the current approach to miss relevant semver tags that were created on a separate branch and then subsequently merged to master.

In #27171, Bryan described the base tag that is supposed to be used for pseudoversions as:

"It is intended to be the semantically-latest tag that appears on any commit that is a (transitive) parent of the commit with the given hash, regardless of branches. (The pseudo-version is supposed to sort after every version — tagged or otherwise — that came before it, but before the next tag that a human might plausibly want to apply to the branch.)"

This CL solves the glob problem and tags-on-other-branches problem more directly than the current approach: this CL gets the full list of tags that have been merged into the specific revision of interest, and then sorts and filters the results in cmd/go to select the semantically-latest valid semver tag.

Fixes #31673
Fixes #31287
Updates #27171

Change-Id: I7c3e6b46b2b21dd60562cf2893b6bd2afaae61d5
Reviewed-on: https://go-review.googlesource.com/c/go/+/174061
Run-TryBot: Jay Conrod <jayconrod@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>

diff --git a/src/cmd/go/internal/modfetch/codehost/git.go b/src/cmd/go/internal/modfetch/codehost/git.go
index 5273e633b5c6c5b70f75491b65ce77f8a79a1da8..55627cb72ac7041183adfb432643fc4f4ae3e3ca 100644 (file)
--- a/src/cmd/go/internal/modfetch/codehost/git.go
+++ b/src/cmd/go/internal/modfetch/codehost/git.go
@@ -19,6 +19,7 @@ import (
 
        "cmd/go/internal/lockedfile"
        "cmd/go/internal/par"
+       "cmd/go/internal/semver"
 )
 
 // GitRepo returns the code repository at the given Git remote reference.
@@ -652,16 +653,40 @@ func (r *gitRepo) RecentTag(rev, prefix string) (tag string, err error) {
        }
        rev = info.Name // expand hash prefixes
 
-       // describe sets tag and err using 'git describe' and reports whether the
+       // describe sets tag and err using 'git for-each-ref' and reports whether the
        // result is definitive.
        describe := func() (definitive bool) {
                var out []byte
-               out, err = Run(r.dir, "git", "describe", "--first-parent", "--always", "--abbrev=0", "--match", prefix+"v[0-9]*.[0-9]*.[0-9]*", "--tags", rev)
+               out, err = Run(r.dir, "git", "for-each-ref", "--format", "%(refname)", "refs/tags", "--merged", rev)
                if err != nil {
-                       return true // Because we use "--always", describe should never fail.
+                       return true
+               }
+
+               // prefixed tags aren't valid semver tags so compare without prefix, but only tags with correct prefix
+               var highest string
+               for _, line := range strings.Split(string(out), "\n") {
+                       line = strings.TrimSpace(line)
+                       // git do support lstrip in for-each-ref format, but it was added in v2.13.0. Stripping here
+                       // instead gives support for git v2.7.0.
+                       if !strings.HasPrefix(line, "refs/tags/") {
+                               continue
+                       }
+                       line = line[len("refs/tags/"):]
+
+                       if !strings.HasPrefix(line, prefix) {
+                               continue
+                       }
+
+                       semtag := line[len(prefix):]
+                       if semver.IsValid(semtag) {
+                               highest = semver.Max(highest, semtag)
+                       }
+               }
+
+               if highest != "" {
+                       tag = prefix + highest
                }
 
-               tag = string(bytes.TrimSpace(out))
                return tag != "" && !AllHex(tag)
        }
 

diff --git a/src/cmd/go/testdata/script/mod_get_pseudo_other_branch.txt b/src/cmd/go/testdata/script/mod_get_pseudo_other_branch.txt
new file mode 100644 (file)
index 0000000..fa1398e
--- /dev/null
+++ b/src/cmd/go/testdata/script/mod_get_pseudo_other_branch.txt
@@ -0,0 +1,31 @@
+env GO111MODULE=on
+env GOPROXY=
+
+# TODO(jayconrod): get test repo onto vcs-test.golang.org
+
+# Testing that a pseudo-version is based on the semantically-latest
+# tag that appears in any commit that is a (transitive) parent of the commit
+# supplied to 'go get', regardless of branches
+
+[!net] skip
+[!exec:git] skip
+
+# For this test repository:
+#  tag v0.2.1 is most recent tag on master itself
+#  tag v0.2.2 is on branch2, which was then merged to master
+#  commit 5aaa858 is on master at a later point
+#
+# The pseudo-version hence sorts immediately after v0.2.2 rather
+# than v0.2.1, even though the v0.2.2 tag is not on master.
+go get -m github.com/leitzler/tagtests@5aaa858
+go list -m all
+stdout '^github.com/leitzler/tagtests v0.2.3-0.20190424071028-5aaa858a59e2$'
+
+-- go.mod --
+module x
+
+go 1.12
+-- x.go --
+package x
+
+import _ "github.com/leitzler/tagtests"

diff --git a/src/cmd/go/testdata/script/mod_get_pseudo_prefix.txt b/src/cmd/go/testdata/script/mod_get_pseudo_prefix.txt
new file mode 100644 (file)
index 0000000..f46444e
--- /dev/null
+++ b/src/cmd/go/testdata/script/mod_get_pseudo_prefix.txt
@@ -0,0 +1,35 @@
+env GO111MODULE=on
+env GOPROXY=
+
+# TODO(jayconrod): get test repo onto vcs-test.golang.org
+
+# Testing that a pseudo-version is based on the semantically-latest
+# prefixed tag in any commit that is a parent of the commit supplied
+# to 'go get', when using a repo with go.mod in a sub directory.
+
+[!net] skip
+[!exec:git] skip
+
+# For this test repository go.mod resides in sub/ (only):
+#  master (372cb6e) is not tagged
+#  tag v0.2.0 is most recent tag before master
+#  tag sub/v0.0.10 is most recent tag before v0.2.0
+#
+# The pseudo-version is based on sub/v0.0.10, since v0.2.0 doesn't
+# contain the prefix.
+go get -m github.com/leitzler/prefixtagtests/sub
+go list -m all
+stdout '^github.com/leitzler/prefixtagtests/sub v0.0.10$'
+
+go get -u -m github.com/leitzler/prefixtagtests/sub@372cb6e
+go list -m all
+stdout '^github.com/leitzler/prefixtagtests/sub v0.0.11-0.20190427183112-372cb6ea3fb5$'
+
+-- go.mod --
+module x
+
+go 1.12
+-- x.go --
+package x
+
+import _ "github.com/leitzler/prefixtagtests/sub"