cmd/go/internal/modload: classify "invalid pseudo-version" errors in Query

If we encounter an unclassified error in modload.Query, it takes
precedence even if the package is found in some other module.
(That is intentional, so that if a package exists in both a parent
and a nested module the outcome is deterministic, and does not shift
if a temporary error causes one of the modules to be unavailable.)

A pseudo-version is formed from a base version and a commit hash.
Each version tag is specific to the module in a particular directory
of the repo (often the root directory), whereas the commit hash is
the same for all subdirectories. When we go to check a particular
subdirectory for the requested package, we may find that that version
is not valid for that combination of <subdirectory, commit hash>,
but we should keep looking to see whether it is valid for a module
in some other subdirectory.

Fixes #47650.

Change-Id: Id48f590ce906a3d4cf4e82fc66137bf67613277d
Cq-Include-Trybots: luci.golang.try:gotip-linux-amd64-longtest,gotip-windows-amd64-longtest
Reviewed-on: https://go-review.googlesource.com/c/go/+/548475
Reviewed-by: Michael Matloob <matloob@golang.org>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Bryan Mills <bcmills@google.com>

diff --git a/src/cmd/go/internal/modfetch/coderepo.go b/src/cmd/go/internal/modfetch/coderepo.go
index 7d83ac6971d2ad25c93f74db4011dded2180afb5..75c34e9fcb538f652f2915c328f3d9132ffa1d28 100644 (file)
--- a/src/cmd/go/internal/modfetch/coderepo.go
+++ b/src/cmd/go/internal/modfetch/coderepo.go
@@ -514,10 +514,20 @@ func (r *codeRepo) convert(ctx context.Context, info *codehost.RevInfo, statVers
        // Determine version.
 
        if module.IsPseudoVersion(statVers) {
+               // Validate the go.mod location and major version before
+               // we check for an ancestor tagged with the pseude-version base.
+               //
+               // We can rule out an invalid subdirectory or major version with only
+               // shallow commit information, but checking the pseudo-version base may
+               // require downloading a (potentially more expensive) full history.
+               revInfo, err = checkCanonical(statVers)
+               if err != nil {
+                       return revInfo, err
+               }
                if err := r.validatePseudoVersion(ctx, info, statVers); err != nil {
                        return nil, err
                }
-               return checkCanonical(statVers)
+               return revInfo, nil
        }
 
        // statVers is not a pseudo-version, so we need to either resolve it to a

diff --git a/src/cmd/go/internal/modload/query.go b/src/cmd/go/internal/modload/query.go
index 895c6c003274ce61ad1e0df0fa880ab193337e77..c4cf55442ba69be0cd10fe777cb086252ba74919 100644 (file)
--- a/src/cmd/go/internal/modload/query.go
+++ b/src/cmd/go/internal/modload/query.go
@@ -891,11 +891,12 @@ func queryPrefixModules(ctx context.Context, candidateModules []string, queryMod
        // is most likely to find helpful: the most useful class of error at the
        // longest matching path.
        var (
-               noPackage   *PackageNotInModuleError
-               noVersion   *NoMatchingVersionError
-               noPatchBase *NoPatchBaseError
-               invalidPath *module.InvalidPathError // see comment in case below
-               notExistErr error
+               noPackage      *PackageNotInModuleError
+               noVersion      *NoMatchingVersionError
+               noPatchBase    *NoPatchBaseError
+               invalidPath    *module.InvalidPathError // see comment in case below
+               invalidVersion error
+               notExistErr    error
        )
        for _, r := range results {
                switch rErr := r.err.(type) {
@@ -931,6 +932,10 @@ func queryPrefixModules(ctx context.Context, candidateModules []string, queryMod
                                if notExistErr == nil {
                                        notExistErr = rErr
                                }
+                       } else if iv := (*module.InvalidVersionError)(nil); errors.As(rErr, &iv) {
+                               if invalidVersion == nil {
+                                       invalidVersion = rErr
+                               }
                        } else if err == nil {
                                if len(found) > 0 || noPackage != nil {
                                        // golang.org/issue/34094: If we have already found a module that
@@ -961,6 +966,8 @@ func queryPrefixModules(ctx context.Context, candidateModules []string, queryMod
                        err = noPatchBase
                case invalidPath != nil:
                        err = invalidPath
+               case invalidVersion != nil:
+                       err = invalidVersion
                case notExistErr != nil:
                        err = notExistErr
                default:

diff --git a/src/cmd/go/testdata/script/mod_get_issue47650.txt b/src/cmd/go/testdata/script/mod_get_issue47650.txt
new file mode 100644 (file)
index 0000000..8561b21
--- /dev/null
+++ b/src/cmd/go/testdata/script/mod_get_issue47650.txt
@@ -0,0 +1,29 @@
+# Regression test for https://go.dev/issue/47650:
+# 'go get' with a pseudo-version of a non-root package within a module
+# erroneously rejected the pseudo-version as invalid, because it did not fetch
+# enough commit history to validate the pseudo-version base.
+
+[short] skip 'creates and uses a git repository'
+[!git] skip
+
+env GOPRIVATE=vcs-test.golang.org
+
+# If we request a package in a subdirectory of a module by commit hash, we
+# successfully resolve it to a pseudo-version derived from a tag on the parent
+# commit.
+cp go.mod go.mod.orig
+go get -x vcs-test.golang.org/git/issue47650.git/cmd/issue47650@21535ef346c3
+stderr '^go: added vcs-test.golang.org/git/issue47650.git v0.1.1-0.20210811175200-21535ef346c3$'
+
+# Explicitly requesting that same version should succeed, fetching additional
+# history for the requested commit as needed in order to validate the
+# pseudo-version base.
+go clean -modcache
+cp go.mod.orig go.mod
+go get -x vcs-test.golang.org/git/issue47650.git/cmd/issue47650@v0.1.1-0.20210811175200-21535ef346c3
+stderr '^go: added vcs-test.golang.org/git/issue47650.git v0.1.1-0.20210811175200-21535ef346c3$'
+
+-- go.mod --
+module example
+
+go 1.20

diff --git a/src/cmd/go/testdata/vcstest/git/issue47650.txt b/src/cmd/go/testdata/vcstest/git/issue47650.txt
new file mode 100644 (file)
index 0000000..fe037ce
--- /dev/null
+++ b/src/cmd/go/testdata/vcstest/git/issue47650.txt
@@ -0,0 +1,41 @@
+handle git
+
+env GIT_AUTHOR_NAME='Bryan C. Mills'
+env GIT_AUTHOR_EMAIL='bcmills@google.com'
+env GIT_COMMITTER_NAME=$GIT_AUTHOR_NAME
+env GIT_COMMITTER_EMAIL=$GIT_AUTHOR_EMAIL
+
+git init
+
+at 2021-08-11T13:52:00-04:00
+git add cmd
+git commit -m 'add cmd/issue47650'
+git tag v0.1.0
+
+git add go.mod
+git commit -m 'add go.mod'
+
+git show-ref --tags --heads
+cmp stdout .git-refs
+
+git log --oneline --decorate=short
+cmp stdout .git-log
+
+-- .git-refs --
+21535ef346c3e79fd09edd75bd4725f06c828e43 refs/heads/main
+4d237df2dbfc8a443af2f5e84be774f08a2aed0c refs/tags/v0.1.0
+-- .git-log --
+21535ef (HEAD -> main) add go.mod
+4d237df (tag: v0.1.0) add cmd/issue47650
+-- go.mod --
+module vcs-test.golang.org/git/issue47650.git
+
+go 1.17
+-- cmd/issue47650/main.go --
+package main
+
+import "os"
+
+func main() {
+       os.Stdout.WriteString("Hello, world!")
+}